also return classes when caling goToDef on a constructor call (#59421)

2026-03-15 14:05:47 -05:00 · 2024-08-16 13:58:29 -07:00
parent f025a5b879
commit 26c4320757
9 changed files with 602 additions and 27 deletions
--- 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;
    }
--- 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",
--- 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
+   }
+  ]
--- a/tests/baselines/reference/goToDefinitionConstructorOfClassExpression01.baseline.jsonc
+++ b/tests/baselines/reference/goToDefinitionConstructorOfClassExpression01.baseline.jsonc
@@ -1,14 +1,282 @@
 // === goToDefinition ===
 // === /tests/cases/fourslash/goToDefinitionConstructorOfClassExpression01.ts ===
-// var x = class C {
-//     [|constructor() {
+// var x = <|class [|{| defId: 0 |}C|] {
+//     [|{| defId: 1 |}constructor() {
 //         var other = new /*GOTO DEF*/C;
 //     }|]
-// }
+// }|>
+// 
+// var y = class C extends x {
+//     constructor() {
+// --- (line: 9) skipped ---

  // === Details ===
  [
   {
+    "defId": 0,
+    "kind": "local class",
+    "name": "C",
+    "containerName": "",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
+    "kind": "constructor",
+    "name": "__constructor",
+    "containerName": "C",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   }
+  ]
+
+
+
+// === goToDefinition ===
+// === /tests/cases/fourslash/goToDefinitionConstructorOfClassExpression01.ts ===
+// --- (line: 3) skipped ---
+//     }
+// }
+// 
+// var y = <|class [|{| defId: 0 |}C|] extends x {
+//     [|{| defId: 1 |}constructor() {
+//         super();
+//         var other = new /*GOTO DEF*/C;
+//     }|]
+// }|>
+// var z = class C extends x {
+//     m() {
+//         return new C;
+// --- (line: 16) skipped ---
+
+  // === Details ===
+  [
+   {
+    "defId": 0,
+    "kind": "local class",
+    "name": "C",
+    "containerName": "",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
+    "kind": "constructor",
+    "name": "__constructor",
+    "containerName": "C",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   }
+  ]
+
+
+
+// === goToDefinition ===
+// === /tests/cases/fourslash/goToDefinitionConstructorOfClassExpression01.ts ===
+// var x = class C {
+//     [|{| defId: 1 |}constructor() {
+//         var other = new C;
+//     }|]
+// }
+// 
+// var y = class C extends x {
+//     constructor() {
+//         super();
+//         var other = new C;
+//     }
+// }
+// var z = <|class [|{| defId: 0 |}C|] extends x {
+//     m() {
+//         return new /*GOTO DEF*/C;
+//     }
+// }|>
+// 
+// var x1 = new C();
+// var x2 = new x();
+// var y1 = new y();
+// var z1 = new z();
+
+  // === Details ===
+  [
+   {
+    "defId": 0,
+    "kind": "local class",
+    "name": "C",
+    "containerName": "",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
+    "kind": "constructor",
+    "name": "__constructor",
+    "containerName": "C",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   }
+  ]
+
+
+
+// === goToDefinition ===
+// === /tests/cases/fourslash/goToDefinitionConstructorOfClassExpression01.ts ===
+// --- (line: 15) skipped ---
+//     }
+// }
+// 
+// var x1 = new /*GOTO DEF*/C();
+// var x2 = new x();
+// var y1 = new y();
+// var z1 = new z();
+
+
+
+// === goToDefinition ===
+// === /tests/cases/fourslash/goToDefinitionConstructorOfClassExpression01.ts ===
+// <|var [|{| defId: 0 |}x|] = class C {
+//     [|{| defId: 1 |}constructor() {
+//         var other = new C;
+//     }|]
+// }|>
+// 
+// var y = class C extends x {
+//     constructor() {
+// --- (line: 9) skipped ---
+
+// --- (line: 16) skipped ---
+// }
+// 
+// var x1 = new C();
+// var x2 = new /*GOTO DEF*/x();
+// var y1 = new y();
+// var z1 = new z();
+
+  // === Details ===
+  [
+   {
+    "defId": 0,
+    "kind": "var",
+    "name": "x",
+    "containerName": "",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
+    "kind": "constructor",
+    "name": "__constructor",
+    "containerName": "C",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   }
+  ]
+
+
+
+// === goToDefinition ===
+// === /tests/cases/fourslash/goToDefinitionConstructorOfClassExpression01.ts ===
+// --- (line: 3) skipped ---
+//     }
+// }
+// 
+// <|var [|{| defId: 0 |}y|] = class C extends x {
+//     [|{| defId: 1 |}constructor() {
+//         super();
+//         var other = new C;
+//     }|]
+// }|>
+// var z = class C extends x {
+//     m() {
+//         return new C;
+//     }
+// }
+// 
+// var x1 = new C();
+// var x2 = new x();
+// var y1 = new /*GOTO DEF*/y();
+// var z1 = new z();
+
+  // === Details ===
+  [
+   {
+    "defId": 0,
+    "kind": "var",
+    "name": "y",
+    "containerName": "",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
+    "kind": "constructor",
+    "name": "__constructor",
+    "containerName": "C",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   }
+  ]
+
+
+
+// === goToDefinition ===
+// === /tests/cases/fourslash/goToDefinitionConstructorOfClassExpression01.ts ===
+// var x = class C {
+//     [|{| defId: 1 |}constructor() {
+//         var other = new C;
+//     }|]
+// }
+// 
+// var y = class C extends x {
+//     constructor() {
+//         super();
+//         var other = new C;
+//     }
+// }
+// <|var [|{| defId: 0 |}z|] = class C extends x {
+//     m() {
+//         return new C;
+//     }
+// }|>
+// 
+// var x1 = new C();
+// var x2 = new x();
+// var y1 = new y();
+// var z1 = new /*GOTO DEF*/z();
+
+  // === Details ===
+  [
+   {
+    "defId": 0,
+    "kind": "var",
+    "name": "z",
+    "containerName": "",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
    "kind": "constructor",
    "name": "__constructor",
    "containerName": "C",
--- a/tests/baselines/reference/goToDefinitionConstructorOfClassWhenClassIsPrecededByNamespace01.baseline.jsonc
+++ b/tests/baselines/reference/goToDefinitionConstructorOfClassWhenClassIsPrecededByNamespace01.baseline.jsonc
@@ -4,16 +4,27 @@
 //     export var x;
 // }
 // 
-// class Foo {
-//     [|constructor() {
+// <|class [|{| defId: 0 |}Foo|] {
+//     [|{| defId: 1 |}constructor() {
 //     }|]
-// }
+// }|>
 // 
 // var x = new /*GOTO DEF*/Foo();

  // === Details ===
  [
   {
+    "defId": 0,
+    "kind": "class",
+    "name": "Foo",
+    "containerName": "",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
    "kind": "constructor",
    "name": "__constructor",
    "containerName": "Foo",
--- a/tests/baselines/reference/goToDefinitionConstructorOverloads.baseline.jsonc
+++ b/tests/baselines/reference/goToDefinitionConstructorOverloads.baseline.jsonc
@@ -1,17 +1,31 @@
 // === goToDefinition ===
 // === /tests/cases/fourslash/goToDefinitionConstructorOverloads.ts ===
-// class ConstructorOverload {
-//     [|constructor();|]
+// <|class [|{| defId: 0 |}ConstructorOverload|] {
+//     [|{| defId: 1 |}constructor();|]
 //     constructor(foo: string);
 //     constructor(foo: any)  { }
-// }
+// }|>
 // 
 // var constructorOverload = new /*GOTO DEF*/ConstructorOverload();
 // var constructorOverload = new ConstructorOverload("foo");
+// 
+// class Extended extends ConstructorOverload {
+// --- (line: 11) skipped ---

  // === Details ===
  [
   {
+    "defId": 0,
+    "kind": "class",
+    "name": "ConstructorOverload",
+    "containerName": "",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
    "kind": "constructor",
    "name": "__constructor",
    "containerName": "ConstructorOverload",
@@ -26,18 +40,33 @@

 // === goToDefinition ===
 // === /tests/cases/fourslash/goToDefinitionConstructorOverloads.ts ===
-// class ConstructorOverload {
+// <|class [|{| defId: 0 |}ConstructorOverload|] {
 //     constructor();
-//     [|constructor(foo: string);|]
+//     [|{| defId: 1 |}constructor(foo: string);|]
 //     constructor(foo: any)  { }
-// }
+// }|>
 // 
 // var constructorOverload = new ConstructorOverload();
 // var constructorOverload = new /*GOTO DEF*/ConstructorOverload("foo");
+// 
+// class Extended extends ConstructorOverload {
+//     readonly name = "extended";
+// --- (line: 12) skipped ---

  // === Details ===
  [
   {
+    "defId": 0,
+    "kind": "class",
+    "name": "ConstructorOverload",
+    "containerName": "",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
    "kind": "constructor",
    "name": "__constructor",
    "containerName": "ConstructorOverload",
@@ -59,7 +88,7 @@
 // }
 // 
 // var constructorOverload = new ConstructorOverload();
-// var constructorOverload = new ConstructorOverload("foo");
+// --- (line: 8) skipped ---

  // === Details ===
  [
@@ -70,4 +99,90 @@
    "isLocal": false,
    "isAmbient": false
   }
+  ]
+
+
+
+// === goToDefinition ===
+// === /tests/cases/fourslash/goToDefinitionConstructorOverloads.ts ===
+// class ConstructorOverload {
+//     [|{| defId: 1 |}constructor();|]
+//     constructor(foo: string);
+//     constructor(foo: any)  { }
+// }
+// 
+// var constructorOverload = new ConstructorOverload();
+// var constructorOverload = new ConstructorOverload("foo");
+// 
+// <|class [|{| defId: 0 |}Extended|] extends ConstructorOverload {
+//     readonly name = "extended";
+// }|>
+// var extended1 = new /*GOTO DEF*/Extended();
+// var extended2 = new Extended("foo");
+
+  // === Details ===
+  [
+   {
+    "defId": 0,
+    "kind": "class",
+    "name": "Extended",
+    "containerName": "",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
+    "kind": "constructor",
+    "name": "__constructor",
+    "containerName": "ConstructorOverload",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   }
+  ]
+
+
+
+// === goToDefinition ===
+// === /tests/cases/fourslash/goToDefinitionConstructorOverloads.ts ===
+// class ConstructorOverload {
+//     constructor();
+//     [|{| defId: 1 |}constructor(foo: string);|]
+//     constructor(foo: any)  { }
+// }
+// 
+// var constructorOverload = new ConstructorOverload();
+// var constructorOverload = new ConstructorOverload("foo");
+// 
+// <|class [|{| defId: 0 |}Extended|] extends ConstructorOverload {
+//     readonly name = "extended";
+// }|>
+// var extended1 = new Extended();
+// var extended2 = new /*GOTO DEF*/Extended("foo");
+
+  // === Details ===
+  [
+   {
+    "defId": 0,
+    "kind": "class",
+    "name": "Extended",
+    "containerName": "",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
+    "kind": "constructor",
+    "name": "__constructor",
+    "containerName": "ConstructorOverload",
+    "isLocal": false,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   }
  ]
--- a/tests/cases/fourslash/goToDefinitionClassConstructors.ts
+++ b/tests/cases/fourslash/goToDefinitionClassConstructors.ts
@@ -0,0 +1,39 @@
+/// <reference path='fourslash.ts'/>
+
+// @filename: definitions.ts
+//// export class Base {
+////     constructor(protected readonly cArg: string) {}
+//// }
+//// 
+//// export class Derived extends Base {
+////     readonly email = this.cArg.getByLabel('Email')
+////     readonly password =  this.cArg.getByLabel('Password')
+//// }
+
+// @filename: main.ts
+//// import { Derived } from './base'
+//// const derived = new [|/*Derived*/Derived|](cArg)
+
+// @filename: defInSameFile.ts
+//// import { Base } from './base'
+//// class SameFile extends Base {
+////     readonly name: string = 'SameFile'
+//// }
+//// const SameFile = new [|/*SameFile*/SameFile|](cArg)
+//// const wrapper = new [|/*Base*/Base|](cArg)
+
+// @filename: hasConstructor.ts
+//// import { Base } from './base'
+//// class HasConstructor extends Base {
+////     constructor() {}
+////     readonly name: string = '';
+//// }
+//// const hasConstructor = new [|/*HasConstructor*/HasConstructor|](cArg)
+
+
+verify.baselineGoToDefinition(
+    "Derived",
+    "SameFile",
+    "HasConstructor",
+    "Base",
+);
--- a/tests/cases/fourslash/goToDefinitionConstructorOfClassExpression01.ts
+++ b/tests/cases/fourslash/goToDefinitionConstructorOfClassExpression01.ts
@@ -2,8 +2,25 @@

 ////var x = class C {
 ////    /*definition*/constructor() {
-////        var other = new [|/*usage*/C|];
+////        var other = new [|/*xusage*/C|];
 ////    }
 ////}
+////
+////var y = class C extends x {
+////    constructor() {
+////        super();
+////        var other = new [|/*yusage*/C|];
+////    }
+////}
+////var z = class C extends x {
+////    m() {
+////        return new [|/*zusage*/C|];
+////    }
+////}
+////
+////var x1 = new [|/*cref*/C|]();
+////var x2 = new [|/*xref*/x|]();
+////var y1 = new [|/*yref*/y|]();
+////var z1 = new [|/*zref*/z|]();

-verify.baselineGoToDefinition("usage");
+verify.baselineGoToDefinition("xusage", "yusage", "zusage", "cref", "xref", "yref", "zref");
--- a/tests/cases/fourslash/goToDefinitionConstructorOverloads.ts
+++ b/tests/cases/fourslash/goToDefinitionConstructorOverloads.ts
@@ -8,9 +8,17 @@
 ////
 ////var constructorOverload = new [|/*constructorOverloadReference1*/ConstructorOverload|]();
 ////var constructorOverload = new [|/*constructorOverloadReference2*/ConstructorOverload|]("foo");
+////
+////class Extended extends ConstructorOverload {
+////    readonly name = "extended";
+////}
+////var extended1 = new [|/*extendedRef1*/Extended|]();
+////var extended2 = new [|/*extendedRef2*/Extended|]("foo");

 verify.baselineGoToDefinition(
    "constructorOverloadReference1",
    "constructorOverloadReference2",
    "constructorOverload1",
+    "extendedRef1",
+    "extendedRef2",
 );