also return classes when caling goToDef on a constructor call (micros…

…oft#59421)
iisaduan · Aug 16, 2024 · 26c4320 · 26c4320
1 parent f025a5b
commit 26c4320
Show file tree

Hide file tree

Showing 9 changed files with 601 additions and 26 deletions.
diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts
@@ -46,6 +46,7 @@ import {
     isBindingElement,
     isCallLikeExpression,
     isCallOrNewExpressionTarget,
+    isClassDeclaration,
     isClassElement,
     isClassExpression,
     isClassLike,
@@ -228,16 +229,20 @@ export function getDefinitionAtPosition(program: Program, sourceFile: SourceFile
     // Don't go to the component constructor definition for a JSX element, just go to the component definition.
     if (calledDeclaration && !(isJsxOpeningLikeElement(node.parent) && isJsxConstructorLike(calledDeclaration))) {
         const sigInfo = createDefinitionFromSignatureDeclaration(typeChecker, calledDeclaration, failedAliasResolution);
+
         // For a function, if this is the original function definition, return just sigInfo.
         // If this is the original constructor definition, parent is the class.
+        // Here, we filter declarations to not duplicate returned definitions.
+        let declarationFilter: (d: Declaration) => boolean = d => d !== calledDeclaration;
         if (typeChecker.getRootSymbols(symbol).some(s => symbolMatchesSignature(s, calledDeclaration))) {
-            return [sigInfo];
-        }
-        else {
-            const defs = getDefinitionFromSymbol(typeChecker, symbol, node, failedAliasResolution, calledDeclaration) || emptyArray;
-            // For a 'super()' call, put the signature first, else put the variable first.
-            return node.kind === SyntaxKind.SuperKeyword ? [sigInfo, ...defs] : [...defs, sigInfo];
+            if (!isConstructorDeclaration(calledDeclaration)) return [sigInfo];
+
+            // If we found a constructor declaration, we also look for class declarations as definitions
+            declarationFilter = (d: Declaration) => d !== calledDeclaration && (isClassDeclaration(d) || isClassExpression(d));
         }
+        const defs = getDefinitionFromSymbol(typeChecker, symbol, node, failedAliasResolution, declarationFilter) || emptyArray;
+        // For a 'super()' call, put the signature first, else put the variable first.
+        return node.kind === SyntaxKind.SuperKeyword ? [sigInfo, ...defs] : [...defs, sigInfo];
     }
 
     // Because name in short-hand property assignment has two different meanings: property name and property value,
@@ -584,9 +589,10 @@ function isExpandoDeclaration(node: Declaration): boolean {
     return !!containingAssignment && getAssignmentDeclarationKind(containingAssignment) === AssignmentDeclarationKind.Property;
 }
 
-function getDefinitionFromSymbol(typeChecker: TypeChecker, symbol: Symbol, node: Node, failedAliasResolution?: boolean, excludeDeclaration?: Node): DefinitionInfo[] | undefined {
-    const filteredDeclarations = filter(symbol.declarations, d => d !== excludeDeclaration);
-    const signatureDefinition = getConstructSignatureDefinition() || getCallSignatureDefinition();
+function getDefinitionFromSymbol(typeChecker: TypeChecker, symbol: Symbol, node: Node, failedAliasResolution?: boolean, declarationFilter?: (d: Declaration) => boolean): DefinitionInfo[] | undefined {
+    const filteredDeclarations = declarationFilter !== undefined ? filter(symbol.declarations, declarationFilter) : symbol.declarations;
+    // If we have a declaration filter, we are looking for specific declaration(s), so we should not return prematurely.
+    const signatureDefinition = !declarationFilter && (getConstructSignatureDefinition() || getCallSignatureDefinition());
     if (signatureDefinition) {
         return signatureDefinition;
     }

diff --git a/tests/baselines/reference/goToDefinitionAmbiants.baseline.jsonc b/tests/baselines/reference/goToDefinitionAmbiants.baseline.jsonc
@@ -64,11 +64,11 @@
 // === /tests/cases/fourslash/goToDefinitionAmbiants.ts ===
 // declare var ambientVar;
 // declare function ambientFunction();
-// declare class ambientClass {
-//     [|constructor();|]
+// <|declare class [|{| defId: 0 |}ambientClass|] {
+//     [|{| defId: 1 |}constructor();|]
 //     static method();
 //     public method();
-// }
+// }|>
 // 
 // ambientVar = 1;
 // ambientFunction();
@@ -79,6 +79,17 @@
   // === Details ===
   [
    {
+    "defId": 0,
+    "kind": "class",
+    "name": "ambientClass",
+    "containerName": "",
+    "isLocal": false,
+    "isAmbient": true,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
     "kind": "constructor",
     "name": "__constructor",
     "containerName": "ambientClass",

diff --git a/tests/baselines/reference/goToDefinitionClassConstructors.baseline.jsonc b/tests/baselines/reference/goToDefinitionClassConstructors.baseline.jsonc
@@ -0,0 +1,100 @@
+// === goToDefinition ===
+// === /tests/cases/fourslash/main.ts ===
+// <|import { [|Derived|] } from './base'|>
+// const derived = new /*GOTO DEF*/Derived(cArg)
+
+  // === Details ===
+  [
+   {
+    "kind": "alias",
+    "name": "Derived",
+    "containerName": "",
+    "isLocal": true,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": true
+   }
+  ]
+
+
+
+// === goToDefinition ===
+// === /tests/cases/fourslash/defInSameFile.ts ===
+// import { Base } from './base'
+// <|class [|SameFile|] extends Base {
+//     readonly name: string = 'SameFile'
+// }|>
+// const SameFile = new /*GOTO DEF*/SameFile(cArg)
+// const wrapper = new Base(cArg)
+
+  // === Details ===
+  [
+   {
+    "kind": "class",
+    "name": "SameFile",
+    "containerName": "",
+    "isLocal": true,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   }
+  ]
+
+
+
+// === goToDefinition ===
+// === /tests/cases/fourslash/hasConstructor.ts ===
+// import { Base } from './base'
+// <|class [|{| defId: 0 |}HasConstructor|] extends Base {
+//     [|{| defId: 1 |}constructor() {}|]
+//     readonly name: string = '';
+// }|>
+// const hasConstructor = new /*GOTO DEF*/HasConstructor(cArg)
+
+  // === Details ===
+  [
+   {
+    "defId": 0,
+    "kind": "class",
+    "name": "HasConstructor",
+    "containerName": "",
+    "isLocal": true,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
+    "kind": "constructor",
+    "name": "__constructor",
+    "containerName": "HasConstructor",
+    "isLocal": true,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   }
+  ]
+
+
+
+// === goToDefinition ===
+// === /tests/cases/fourslash/defInSameFile.ts ===
+// <|import { [|Base|] } from './base'|>
+// class SameFile extends Base {
+//     readonly name: string = 'SameFile'
+// }
+// const SameFile = new SameFile(cArg)
+// const wrapper = new /*GOTO DEF*/Base(cArg)
+
+  // === Details ===
+  [
+   {
+    "kind": "alias",
+    "name": "Base",
+    "containerName": "",
+    "isLocal": true,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": true
+   }
+  ]