-
Notifications
You must be signed in to change notification settings - Fork 12.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(51225): Go-to-definition on case or default should jump to the containing switch statement if available. #51236
Merged
Merged
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
7721e83
fix(51225): Go-to-definition on case or default should jump to the co…
sviat9440 1c1599f
Merge branch 'main' into fix/51225
sviat9440 1995f33
fix(51225): Fix context span
sviat9440 f09b5b6
Merge branch 'main' into fix/51225
sandersn 03d4b40
port tests to baselineGoToDefinition
sandersn d692bbf
dprint fmt
sandersn a97f704
Revert context span change
sandersn 77c28cb
Revert context span again, add close paren this time
sandersn e11f00e
Better way of including close paren
sandersn ffe7d40
Merge branch 'main' into fix/51225
sandersn 279ce44
Move context span calculation to central location
sandersn dd9d9f1
move createDefinitionInfoFromStatement closer to others
sandersn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 17 additions & 0 deletions
17
tests/baselines/reference/goToDefinitionSwitchCase1.baseline.jsonc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// === goToDefinition === | ||
// === /tests/cases/fourslash/goToDefinitionSwitchCase1.ts === | ||
// <|[|switch|] (null )|> { | ||
// /*GOTO DEF*/case null: break; | ||
// } | ||
|
||
// === Details === | ||
[ | ||
{ | ||
"kind": "keyword", | ||
"name": "switch", | ||
"containerName": "", | ||
"isLocal": true, | ||
"isAmbient": false, | ||
"unverified": false | ||
} | ||
] |
17 changes: 17 additions & 0 deletions
17
tests/baselines/reference/goToDefinitionSwitchCase2.baseline.jsonc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// === goToDefinition === | ||
// === /tests/cases/fourslash/goToDefinitionSwitchCase2.ts === | ||
// <|[|switch|] (null)|> { | ||
// /*GOTO DEF*/default: break; | ||
// } | ||
|
||
// === Details === | ||
[ | ||
{ | ||
"kind": "keyword", | ||
"name": "switch", | ||
"containerName": "", | ||
"isLocal": true, | ||
"isAmbient": false, | ||
"unverified": false | ||
} | ||
] |
45 changes: 45 additions & 0 deletions
45
tests/baselines/reference/goToDefinitionSwitchCase3.baseline.jsonc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
// === goToDefinition === | ||
// === /tests/cases/fourslash/goToDefinitionSwitchCase3.ts === | ||
// <|[|switch|] (null)|> { | ||
// /*GOTO DEF*/default: { | ||
// switch (null) { | ||
// default: break; | ||
// } | ||
// }; | ||
// } | ||
|
||
// === Details === | ||
[ | ||
{ | ||
"kind": "keyword", | ||
"name": "switch", | ||
"containerName": "", | ||
"isLocal": true, | ||
"isAmbient": false, | ||
"unverified": false | ||
} | ||
] | ||
|
||
|
||
|
||
// === goToDefinition === | ||
// === /tests/cases/fourslash/goToDefinitionSwitchCase3.ts === | ||
// switch (null) { | ||
// default: { | ||
// <|[|switch|] (null)|> { | ||
// /*GOTO DEF*/default: break; | ||
// } | ||
// }; | ||
// } | ||
|
||
// === Details === | ||
[ | ||
{ | ||
"kind": "keyword", | ||
"name": "switch", | ||
"containerName": "", | ||
"isLocal": true, | ||
"isAmbient": false, | ||
"unverified": false | ||
} | ||
] |
21 changes: 21 additions & 0 deletions
21
tests/baselines/reference/goToDefinitionSwitchCase4.baseline.jsonc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
// === goToDefinition === | ||
// === /tests/cases/fourslash/goToDefinitionSwitchCase4.ts === | ||
// switch (null) { | ||
// case null: break; | ||
// } | ||
// | ||
// <|[|switch|] (null)|> { | ||
// /*GOTO DEF*/case null: break; | ||
// } | ||
|
||
// === Details === | ||
[ | ||
{ | ||
"kind": "keyword", | ||
"name": "switch", | ||
"containerName": "", | ||
"isLocal": true, | ||
"isAmbient": false, | ||
"unverified": false | ||
} | ||
] |
16 changes: 16 additions & 0 deletions
16
tests/baselines/reference/goToDefinitionSwitchCase5.baseline.jsonc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// === goToDefinition === | ||
// === /tests/cases/fourslash/goToDefinitionSwitchCase5.ts === | ||
// [|export /*GOTO DEF*/default {}|] | ||
|
||
// === Details === | ||
[ | ||
{ | ||
"kind": "property", | ||
"name": "default", | ||
"containerName": "\"/tests/cases/fourslash/goToDefinitionSwitchCase5\"", | ||
"isLocal": true, | ||
"isAmbient": false, | ||
"unverified": false, | ||
"failedAliasResolution": false | ||
} | ||
] |
47 changes: 47 additions & 0 deletions
47
tests/baselines/reference/goToDefinitionSwitchCase6.baseline.jsonc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
// === goToDefinition === | ||
// === /tests/cases/fourslash/goToDefinitionSwitchCase6.ts === | ||
// export default { /*GOTO DEF*/[|{| textSpan: true |}case|] }; | ||
// default; | ||
// case 42; | ||
|
||
// === Details === | ||
[ | ||
{ | ||
"kind": "property", | ||
"name": "case", | ||
"containerName": "__object", | ||
"isLocal": true, | ||
"isAmbient": false, | ||
"unverified": false, | ||
"failedAliasResolution": false | ||
} | ||
] | ||
|
||
|
||
|
||
// === goToDefinition === | ||
// === /tests/cases/fourslash/goToDefinitionSwitchCase6.ts === | ||
// [|export default { case }; | ||
// /*GOTO DEF*/default; | ||
// case 42;|] | ||
|
||
// === Details === | ||
[ | ||
{ | ||
"kind": "module", | ||
"name": "\"/tests/cases/fourslash/goToDefinitionSwitchCase6\"", | ||
"containerName": "", | ||
"isLocal": false, | ||
"isAmbient": false, | ||
"unverified": false, | ||
"failedAliasResolution": false | ||
} | ||
] | ||
|
||
|
||
|
||
// === goToDefinition === | ||
// === /tests/cases/fourslash/goToDefinitionSwitchCase6.ts === | ||
// export default { case }; | ||
// default; | ||
// /*GOTO DEF*/case 42; |
18 changes: 18 additions & 0 deletions
18
tests/baselines/reference/goToDefinitionSwitchCase7.baseline.jsonc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
// === goToDefinition === | ||
// === /tests/cases/fourslash/goToDefinitionSwitchCase7.ts === | ||
// switch (null) { | ||
// case null: | ||
// [|export /*GOTO DEF*/default 123;|] | ||
|
||
// === Details === | ||
[ | ||
{ | ||
"kind": "var", | ||
"name": "default", | ||
"containerName": "", | ||
"isLocal": true, | ||
"isAmbient": false, | ||
"unverified": false, | ||
"failedAliasResolution": false | ||
} | ||
] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
/// <reference path="fourslash.ts" /> | ||
|
||
////switch (null ) { | ||
//// [|/*start*/case|] null: break; | ||
////} | ||
|
||
verify.baselineGoToDefinition("start"); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
/// <reference path="fourslash.ts" /> | ||
|
||
////switch (null) { | ||
//// [|/*start*/default|]: break; | ||
////} | ||
|
||
verify.baselineGoToDefinition("start"); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
/// <reference path="fourslash.ts" /> | ||
|
||
////switch (null) { | ||
//// [|/*start1*/default|]: { | ||
//// switch (null) { | ||
//// [|/*start2*/default|]: break; | ||
//// } | ||
//// }; | ||
////} | ||
|
||
verify.baselineGoToDefinition("start1", "start2"); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
/// <reference path="fourslash.ts" /> | ||
|
||
//// switch (null) { | ||
//// case null: break; | ||
//// } | ||
//// | ||
//// switch (null) { | ||
//// [|/*start*/case|] null: break; | ||
//// } | ||
|
||
verify.baselineGoToDefinition("start"); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
/// <reference path="fourslash.ts" /> | ||
|
||
////export [|/*start*/default|] {} | ||
|
||
verify.baselineGoToDefinition("start"); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
/// <reference path="fourslash.ts" /> | ||
|
||
////export default { [|/*a*/case|] }; | ||
////[|/*b*/default|]; | ||
////[|/*c*/case|] 42; | ||
|
||
verify.baselineGoToDefinition("a", "b", "c"); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
/// <reference path="fourslash.ts" /> | ||
|
||
////switch (null) { | ||
//// case null: | ||
//// export [|/*start*/default|] 123; | ||
|
||
verify.baselineGoToDefinition("start"); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be moved to
getContextNode
which handledSwitchStatement
then...FindAllReferences.getContextSpan()
so that all places where we will use context span are in single place.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at createDefinitionInfoFromName above, it looks like the right way to use this is first to call getContextNode to get the start/end tokens, then to use toContextSpan to turn it into a context span (see code below). Unfortunately, my
getFullStart
hack (which grabs the closing paren) gets lost since getContextNode returns a node pair, not a location pair. And toContextSpan currently always calls getEnd, but my hack uses getFullStart.Any ideas? I'm going to try hacking toContextSpan to special case switches to see how bad that is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, the hack isn't too bad; there's a special case for strin gliterals there already, so I added it after that.