-
Notifications
You must be signed in to change notification settings - Fork 13.2k
allow calling goToDef on modifiers of named declarations
#60384
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
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,7 +64,9 @@ import { | |
| isJSDocOverrideTag, | ||
| isJsxOpeningLikeElement, | ||
| isJumpStatementTarget, | ||
| isModifier, | ||
| isModuleSpecifierLike, | ||
| isNamedDeclaration, | ||
| isNameOfFunctionDeclaration, | ||
| isNewExpressionTarget, | ||
| isObjectBindingPattern, | ||
|
|
@@ -128,7 +130,10 @@ export function getDefinitionAtPosition(program: Program, sourceFile: SourceFile | |
| const typeChecker = program.getTypeChecker(); | ||
|
|
||
| if (node.kind === SyntaxKind.OverrideKeyword || (isIdentifier(node) && isJSDocOverrideTag(parent) && parent.tagName === node)) { | ||
| return getDefinitionFromOverriddenMember(typeChecker, node) || emptyArray; | ||
| const def = getDefinitionFromOverriddenMember(typeChecker, node); | ||
| if (def !== undefined || node.kind !== SyntaxKind.OverrideKeyword) { | ||
| return def || emptyArray; | ||
| } | ||
| } | ||
|
|
||
| // Labels | ||
|
|
@@ -137,15 +142,8 @@ export function getDefinitionAtPosition(program: Program, sourceFile: SourceFile | |
| return label ? [createDefinitionInfoFromName(typeChecker, label, ScriptElementKind.label, node.text, /*containerName*/ undefined!)] : undefined; // TODO: GH#18217 | ||
| } | ||
|
|
||
| // for switch statments | ||
| switch (node.kind) { | ||
| case SyntaxKind.ReturnKeyword: | ||
| const functionDeclaration = findAncestor(node.parent, n => | ||
| isClassStaticBlockDeclaration(n) | ||
| ? "quit" | ||
| : isFunctionLikeDeclaration(n)) as FunctionLikeDeclaration | undefined; | ||
| return functionDeclaration | ||
| ? [createDefinitionFromSignatureDeclaration(typeChecker, functionDeclaration)] | ||
| : undefined; | ||
| case SyntaxKind.DefaultKeyword: | ||
| if (!isDefaultClause(node.parent)) { | ||
| break; | ||
|
|
@@ -159,16 +157,30 @@ export function getDefinitionAtPosition(program: Program, sourceFile: SourceFile | |
| break; | ||
| } | ||
|
|
||
| if (node.kind === SyntaxKind.AwaitKeyword) { | ||
| const functionDeclaration = findAncestor(node, n => isFunctionLikeDeclaration(n)); | ||
| const isAsyncFunction = functionDeclaration && some(functionDeclaration.modifiers, node => node.kind === SyntaxKind.AsyncKeyword); | ||
| return isAsyncFunction ? [createDefinitionFromSignatureDeclaration(typeChecker, functionDeclaration)] : undefined; | ||
| // for keywords related to function or method definitions | ||
| let findFunctionDecl: ((n: Node) => boolean | "quit") | undefined; | ||
| let checkFunctionDeclaration: ((decl: FunctionLikeDeclaration) => boolean) | undefined; | ||
| switch (node.kind) { | ||
| case SyntaxKind.ReturnKeyword: | ||
| findFunctionDecl = n => { | ||
| return isClassStaticBlockDeclaration(n) | ||
| ? "quit" | ||
| : isFunctionLikeDeclaration(n); | ||
| }; | ||
| break; | ||
| case SyntaxKind.AwaitKeyword: | ||
| checkFunctionDeclaration = (functionDeclaration: FunctionLikeDeclaration) => some(functionDeclaration.modifiers, node => node.kind === SyntaxKind.AsyncKeyword); | ||
| findFunctionDecl = isFunctionLikeDeclaration; | ||
| break; | ||
| case SyntaxKind.YieldKeyword: | ||
| checkFunctionDeclaration = functionDeclaration => !!functionDeclaration.asteriskToken; | ||
| findFunctionDecl = isFunctionLikeDeclaration; | ||
| break; | ||
| } | ||
|
|
||
| if (node.kind === SyntaxKind.YieldKeyword) { | ||
| const functionDeclaration = findAncestor(node, n => isFunctionLikeDeclaration(n)); | ||
| const isGeneratorFunction = functionDeclaration && functionDeclaration.asteriskToken; | ||
| return isGeneratorFunction ? [createDefinitionFromSignatureDeclaration(typeChecker, functionDeclaration)] : undefined; | ||
| if (findFunctionDecl) { | ||
| const functionDeclaration = findAncestor(node, findFunctionDecl) as FunctionLikeDeclaration | undefined; | ||
| const isCorrectDeclaration = functionDeclaration && (!checkFunctionDeclaration || checkFunctionDeclaration(functionDeclaration)); | ||
| return isCorrectDeclaration ? [createDefinitionFromSignatureDeclaration(typeChecker, functionDeclaration)] : undefined; | ||
|
||
| } | ||
|
|
||
| if (isStaticModifier(node) && isClassStaticBlockDeclaration(node.parent)) { | ||
|
|
@@ -217,6 +229,10 @@ export function getDefinitionAtPosition(program: Program, sourceFile: SourceFile | |
| } | ||
| } | ||
|
|
||
| if (isModifier(node) && (isClassElement(parent) || isNamedDeclaration(parent))) { | ||
| symbol = parent.symbol; | ||
| } | ||
|
|
||
| // Could not find a symbol e.g. node is string or number keyword, | ||
| // or the symbol was an internal symbol and does not have a declaration e.g. undefined symbol | ||
| if (!symbol) { | ||
|
|
@@ -457,7 +473,11 @@ export function getTypeDefinitionAtPosition(typeChecker: TypeChecker, sourceFile | |
| if (isImportMeta(node.parent) && node.parent.name === node) { | ||
| return definitionFromType(typeChecker.getTypeAtLocation(node.parent), typeChecker, node.parent, /*failedAliasResolution*/ false); | ||
| } | ||
| const { symbol, failedAliasResolution } = getSymbol(node, typeChecker, /*stopAtAlias*/ false); | ||
| let { symbol, failedAliasResolution } = getSymbol(node, typeChecker, /*stopAtAlias*/ false); | ||
| if (isModifier(node) && (isClassElement(node.parent) || isNamedDeclaration(node.parent))) { | ||
| symbol = node.parent.symbol; | ||
| failedAliasResolution = false; | ||
| } | ||
| if (!symbol) return undefined; | ||
|
|
||
| const typeAtLocation = typeChecker.getTypeOfSymbolAtLocation(symbol, node); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| // === goToDefinition === | ||
| // === /a.ts === | ||
| // class A { | ||
| // <|private [|{| textSpan: true |}z|]/*GOTO DEF*/: string;|> | ||
| // } | ||
|
|
||
| // === Details === | ||
| [ | ||
| { | ||
| "kind": "property", | ||
| "name": "z", | ||
| "containerName": "A", | ||
| "isLocal": true, | ||
| "isAmbient": false, | ||
| "unverified": false, | ||
| "failedAliasResolution": false | ||
| } | ||
| ] |
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 feels like it's very much a special-case. Are you trying to just not break some sort of test case?
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.
It was existing code, so I'm not sure if there are more cases that would be an issue, but it breaks https://github.com/microsoft/TypeScript/blob/main/tests/cases/fourslash/goToDefinitionReturn5.ts
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.
It's probably just fine to update the baseline there.