Fixed crash in go to definition related to expando classes in JS files (

#57628)
microsoft · Apr 19, 2024 · b1d821a · b1d821a
1 parent 42f238b
commit b1d821a
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 4 deletions.
diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts
@@ -9,7 +9,6 @@ import {
     createTextSpanFromBounds,
     createTextSpanFromNode,
     createTextSpanFromRange,
-    Debug,
     Declaration,
     DefinitionInfo,
     DefinitionInfoAndBoundSpan,
@@ -581,16 +580,20 @@ function isExpandoDeclaration(node: Declaration): boolean {
 
 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();
+    if (signatureDefinition) {
+        return signatureDefinition;
+    }
     const withoutExpandos = filter(filteredDeclarations, d => !isExpandoDeclaration(d));
     const results = some(withoutExpandos) ? withoutExpandos : filteredDeclarations;
-    return getConstructSignatureDefinition() || getCallSignatureDefinition() || map(results, declaration => createDefinitionInfo(declaration, typeChecker, symbol, node, /*unverified*/ false, failedAliasResolution));
+    return map(results, declaration => createDefinitionInfo(declaration, typeChecker, symbol, node, /*unverified*/ false, failedAliasResolution));
 
     function getConstructSignatureDefinition(): DefinitionInfo[] | undefined {
         // Applicable only if we are in a new expression, or we are on a constructor declaration
         // and in either case the symbol has a construct signature definition, i.e. class
         if (symbol.flags & SymbolFlags.Class && !(symbol.flags & (SymbolFlags.Function | SymbolFlags.Variable)) && (isNewExpressionTarget(node) || node.kind === SyntaxKind.ConstructorKeyword)) {
-            const cls = find(filteredDeclarations, isClassLike) || Debug.fail("Expected declaration to have at least one class-like declaration");
-            return getSignatureDefinition(cls.members, /*selectConstructors*/ true);
+            const cls = find(filteredDeclarations, isClassLike);
+            return cls && getSignatureDefinition(cls.members, /*selectConstructors*/ true);
         }
     }
 

diff --git a/tests/baselines/reference/goToDefinitionExpandoClass1.baseline.jsonc b/tests/baselines/reference/goToDefinitionExpandoClass1.baseline.jsonc
@@ -0,0 +1,22 @@
+// === goToDefinition ===
+// === /tests/cases/fourslash/index.js ===
+// const Core = {}
+// 
+// <|Core.[|Test|]|> = class { }
+// 
+// Core.Test.prototype.foo = 10
+// 
+// new Core.Tes/*GOTO DEF*/t()
+
+  // === Details ===
+  [
+   {
+    "kind": "property",
+    "name": "Test",
+    "containerName": "Core",
+    "isLocal": true,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   }
+  ]
diff --git a/tests/baselines/reference/goToDefinitionExpandoClass2.baseline.jsonc b/tests/baselines/reference/goToDefinitionExpandoClass2.baseline.jsonc
@@ -0,0 +1,35 @@
+// === goToDefinition ===
+// === /tests/cases/fourslash/index.js ===
+// const Core = {}
+// 
+// <|Core.[|{| defId: 0 |}Test|]|> = class {
+//   [|{| defId: 1 |}constructor() { }|]
+// }
+// 
+// Core.Test.prototype.foo = 10
+// 
+// new Core.Tes/*GOTO DEF*/t()
+
+  // === Details ===
+  [
+   {
+    "defId": 0,
+    "kind": "property",
+    "name": "Test",
+    "containerName": "Core",
+    "isLocal": true,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   },
+   {
+    "defId": 1,
+    "kind": "constructor",
+    "name": "__constructor",
+    "containerName": "Test",
+    "isLocal": true,
+    "isAmbient": false,
+    "unverified": false,
+    "failedAliasResolution": false
+   }
+  ]
diff --git a/tests/cases/fourslash/goToDefinitionExpandoClass1.ts b/tests/cases/fourslash/goToDefinitionExpandoClass1.ts
@@ -0,0 +1,17 @@
+/// <reference path="fourslash.ts" />
+
+// @strict: true
+// @allowJs: true
+// @checkJs: true
+
+// @filename: index.js
+
+//// const Core = {}
+////
+//// Core.Test = class { }
+////
+//// Core.Test.prototype.foo = 10
+////
+//// new Core.Tes/*1*/t()
+
+verify.baselineGoToDefinition("1");
diff --git a/tests/cases/fourslash/goToDefinitionExpandoClass2.ts b/tests/cases/fourslash/goToDefinitionExpandoClass2.ts
@@ -0,0 +1,19 @@
+/// <reference path="fourslash.ts" />
+
+// @strict: true
+// @allowJs: true
+// @checkJs: true
+
+// @filename: index.js
+
+//// const Core = {}
+////
+//// Core.Test = class {
+////   constructor() { }
+//// }
+////
+//// Core.Test.prototype.foo = 10
+////
+//// new Core.Tes/*1*/t()
+
+verify.baselineGoToDefinition("1");