Skip to content

Commit 5519be3

Browse files
AndaristAndarist
authored
Fixed crash when getting a definition for a class member with a computed name and an override modifier (#60631)
1 parent 2ea2ecf commit 5519be3

35 files changed

+662
-40
lines changed

‎src/compiler/checker.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47343,7 +47343,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
4734347343
const errorMessage = overriddenInstanceProperty ?
4734447344
Diagnostics._0_is_defined_as_an_accessor_in_class_1_but_is_overridden_here_in_2_as_an_instance_property :
4734547345
Diagnostics._0_is_defined_as_a_property_in_class_1_but_is_overridden_here_in_2_as_an_accessor;
47346-
error(getNameOfDeclaration(derived.valueDeclaration) || derived.valueDeclaration, errorMessage, symbolToString(base), typeToString(baseType), typeToString(type));
47346+
error(getNameOfDeclaration(derived.valueDeclaration) || derived.valueDeclaration, errorMessage, symbolToString(base), typeToString(getDeclaredTypeOfSymbol(base.parent!)), typeToString(type));
4734747347
}
4734847348
else if (useDefineForClassFields) {
4734947349
const uninitialized = derived.declarations?.find(d => d.kind === SyntaxKind.PropertyDeclaration && !(d as PropertyDeclaration).initializer);

‎src/services/codefixes/fixAddMissingMember.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {
77
createSignatureDeclarationFromSignature,
88
createStubbedBody,
99
eachDiagnostic,
10-
getAllSupers,
1110
ImportAdder,
1211
registerCodeFix,
1312
} from "../_namespaces/ts.codefix.js";
@@ -40,6 +39,7 @@ import {
4039
firstOrUndefinedIterator,
4140
FunctionExpression,
4241
getCheckFlags,
42+
getClassExtendsHeritageElement,
4343
getClassLikeDeclarationOfSymbol,
4444
getEmitScriptTarget,
4545
getEscapedTextOfJsxAttributeName,
@@ -797,3 +797,18 @@ function findScope(node: Node) {
797797
}
798798
return getSourceFileOfNode(node);
799799
}
800+
801+
function getAllSupers(decl: ClassLikeDeclaration | InterfaceDeclaration | undefined, checker: TypeChecker): readonly ClassLikeDeclaration[] {
802+
const res: ClassLikeDeclaration[] = [];
803+
while (decl) {
804+
const superElement = getClassExtendsHeritageElement(decl);
805+
const superSymbol = superElement && checker.getSymbolAtLocation(superElement.expression);
806+
if (!superSymbol) break;
807+
const symbol = superSymbol.flags & SymbolFlags.Alias ? checker.getAliasedSymbol(superSymbol) : superSymbol;
808+
const superDecl = symbol.declarations && find(symbol.declarations, isClassLike);
809+
if (!superDecl) break;
810+
res.push(superDecl);
811+
decl = superDecl;
812+
}
813+
return res;
814+
}

‎src/services/codefixes/fixPropertyOverrideAccessor.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,22 @@ import {
22
codeFixAll,
33
createCodeFixAction,
44
generateAccessorFromProperty,
5-
getAllSupers,
65
registerCodeFix,
76
} from "../_namespaces/ts.codefix.js";
87
import {
98
CodeFixAllContext,
109
CodeFixContext,
1110
Debug,
1211
Diagnostics,
12+
getEffectiveBaseTypeNode,
1313
getSourceFileOfNode,
1414
getTextOfPropertyName,
1515
getTokenAtPosition,
1616
isAccessor,
17+
isClassExpression,
1718
isClassLike,
18-
singleOrUndefined,
19+
isComputedPropertyName,
20+
skipParentheses,
1921
SourceFile,
2022
unescapeLeadingUnderscores,
2123
} from "../_namespaces/ts.js";
@@ -56,15 +58,20 @@ function doChange(file: SourceFile, start: number, length: number, code: number,
5658
else if (code === Diagnostics._0_is_defined_as_a_property_in_class_1_but_is_overridden_here_in_2_as_an_accessor.code) {
5759
const checker = context.program.getTypeChecker();
5860
const node = getTokenAtPosition(file, start).parent;
61+
if (isComputedPropertyName(node)) {
62+
return;
63+
}
5964
Debug.assert(isAccessor(node), "error span of fixPropertyOverrideAccessor should only be on an accessor");
6065
const containingClass = node.parent;
6166
Debug.assert(isClassLike(containingClass), "erroneous accessors should only be inside classes");
62-
const base = singleOrUndefined(getAllSupers(containingClass, checker));
63-
if (!base) return [];
64-
65-
const name = unescapeLeadingUnderscores(getTextOfPropertyName(node.name));
66-
const baseProp = checker.getPropertyOfType(checker.getTypeAtLocation(base), name);
67-
if (!baseProp || !baseProp.valueDeclaration) return [];
67+
const baseTypeNode = getEffectiveBaseTypeNode(containingClass);
68+
if (!baseTypeNode) return;
69+
const expression = skipParentheses(baseTypeNode.expression);
70+
const base = isClassExpression(expression) ? expression.symbol : checker.getSymbolAtLocation(expression);
71+
if (!base) return;
72+
const baseType = checker.getDeclaredTypeOfSymbol(base);
73+
const baseProp = checker.getPropertyOfType(baseType, unescapeLeadingUnderscores(getTextOfPropertyName(node.name)));
74+
if (!baseProp || !baseProp.valueDeclaration) return;
6875

6976
startPosition = baseProp.valueDeclaration.pos;
7077
endPosition = baseProp.valueDeclaration.end;

‎src/services/codefixes/generateAccessors.ts

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@ import {
99
Diagnostics,
1010
factory,
1111
FileTextChanges,
12-
find,
1312
findAncestor,
14-
getClassExtendsHeritageElement,
1513
getDecorators,
1614
getEffectiveModifierFlags,
1715
getFirstConstructorWithBody,
@@ -22,7 +20,6 @@ import {
2220
hasEffectiveReadonlyModifier,
2321
hasStaticModifier,
2422
Identifier,
25-
InterfaceDeclaration,
2623
isClassLike,
2724
isElementAccessExpression,
2825
isFunctionLike,
@@ -50,10 +47,8 @@ import {
5047
startsWithUnderscore,
5148
StringLiteral,
5249
suppressLeadingAndTrailingTrivia,
53-
SymbolFlags,
5450
SyntaxKind,
5551
textChanges,
56-
TypeChecker,
5752
TypeNode,
5853
} from "../_namespaces/ts.js";
5954

@@ -321,22 +316,3 @@ function getDeclarationType(declaration: AcceptedDeclaration, program: Program):
321316
}
322317
return typeNode;
323318
}
324-
325-
/** @internal */
326-
export function getAllSupers(decl: ClassOrInterface | undefined, checker: TypeChecker): readonly ClassOrInterface[] {
327-
const res: ClassLikeDeclaration[] = [];
328-
while (decl) {
329-
const superElement = getClassExtendsHeritageElement(decl);
330-
const superSymbol = superElement && checker.getSymbolAtLocation(superElement.expression);
331-
if (!superSymbol) break;
332-
const symbol = superSymbol.flags & SymbolFlags.Alias ? checker.getAliasedSymbol(superSymbol) : superSymbol;
333-
const superDecl = symbol.declarations && find(symbol.declarations, isClassLike);
334-
if (!superDecl) break;
335-
res.push(superDecl);
336-
decl = superDecl;
337-
}
338-
return res;
339-
}
340-
341-
/** @internal */
342-
export type ClassOrInterface = ClassLikeDeclaration | InterfaceDeclaration;

‎src/services/goToDefinition.ts

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import {
5151
isClassExpression,
5252
isClassLike,
5353
isClassStaticBlockDeclaration,
54+
isComputedPropertyName,
5455
isConstructorDeclaration,
5556
isDeclarationFileName,
5657
isDefaultClause,
@@ -64,6 +65,7 @@ import {
6465
isJSDocOverrideTag,
6566
isJsxOpeningLikeElement,
6667
isJumpStatementTarget,
68+
isKnownSymbol,
6769
isModifier,
6870
isModuleSpecifierLike,
6971
isNamedDeclaration,
@@ -328,14 +330,29 @@ function getDefinitionFromOverriddenMember(typeChecker: TypeChecker, node: Node)
328330
const expression = skipParentheses(baseTypeNode.expression);
329331
const base = isClassExpression(expression) ? expression.symbol : typeChecker.getSymbolAtLocation(expression);
330332
if (!base) return;
333+
const baseType = hasStaticModifier(classElement) ? typeChecker.getTypeOfSymbol(base) : typeChecker.getDeclaredTypeOfSymbol(base);
334+
let baseProp: Symbol | undefined;
331335

332-
const name = unescapeLeadingUnderscores(getTextOfPropertyName(classElement.name));
333-
const symbol = hasStaticModifier(classElement)
334-
? typeChecker.getPropertyOfType(typeChecker.getTypeOfSymbol(base), name)
335-
: typeChecker.getPropertyOfType(typeChecker.getDeclaredTypeOfSymbol(base), name);
336-
if (!symbol) return;
336+
if (isComputedPropertyName(classElement.name)) {
337+
const prop = typeChecker.getSymbolAtLocation(classElement.name);
337338

338-
return getDefinitionFromSymbol(typeChecker, symbol, node);
339+
if (!prop) {
340+
return;
341+
}
342+
343+
if (isKnownSymbol(prop)) {
344+
baseProp = find(typeChecker.getPropertiesOfType(baseType), s => s.escapedName === prop.escapedName);
345+
}
346+
else {
347+
baseProp = typeChecker.getPropertyOfType(baseType, unescapeLeadingUnderscores(prop.escapedName));
348+
}
349+
}
350+
else {
351+
baseProp = typeChecker.getPropertyOfType(baseType, unescapeLeadingUnderscores(getTextOfPropertyName(classElement.name)));
352+
}
353+
if (!baseProp) return;
354+
355+
return getDefinitionFromSymbol(typeChecker, baseProp, node);
339356
}
340357

341358
/** @internal */

‎tests/baselines/reference/accessorsOverrideProperty10.errors.txt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
accessorsOverrideProperty10.ts(6,7): error TS2611: 'x' is defined as a property in class 'A', but is overridden here in 'C' as an accessor.
2+
3+
4+
==== accessorsOverrideProperty10.ts (1 errors) ====
5+
class A {
6+
x = 1;
7+
}
8+
class B extends A {}
9+
class C extends B {
10+
get x() {
11+
~
12+
!!! error TS2611: 'x' is defined as a property in class 'A', but is overridden here in 'C' as an accessor.
13+
return 2;
14+
}
15+
}
16+

‎tests/baselines/reference/accessorsOverrideProperty10.symbols

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//// [tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty10.ts] ////
2+
3+
=== accessorsOverrideProperty10.ts ===
4+
class A {
5+
>A : Symbol(A, Decl(accessorsOverrideProperty10.ts, 0, 0))
6+
7+
x = 1;
8+
>x : Symbol(A.x, Decl(accessorsOverrideProperty10.ts, 0, 9))
9+
}
10+
class B extends A {}
11+
>B : Symbol(B, Decl(accessorsOverrideProperty10.ts, 2, 1))
12+
>A : Symbol(A, Decl(accessorsOverrideProperty10.ts, 0, 0))
13+
14+
class C extends B {
15+
>C : Symbol(C, Decl(accessorsOverrideProperty10.ts, 3, 20))
16+
>B : Symbol(B, Decl(accessorsOverrideProperty10.ts, 2, 1))
17+
18+
get x() {
19+
>x : Symbol(C.x, Decl(accessorsOverrideProperty10.ts, 4, 19))
20+
21+
return 2;
22+
}
23+
}
24+

‎tests/baselines/reference/accessorsOverrideProperty10.types

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
//// [tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty10.ts] ////
2+
3+
=== accessorsOverrideProperty10.ts ===
4+
class A {
5+
>A : A
6+
> : ^
7+
8+
x = 1;
9+
>x : number
10+
> : ^^^^^^
11+
>1 : 1
12+
> : ^
13+
}
14+
class B extends A {}
15+
>B : B
16+
> : ^
17+
>A : A
18+
> : ^
19+
20+
class C extends B {
21+
>C : C
22+
> : ^
23+
>B : B
24+
> : ^
25+
26+
get x() {
27+
>x : number
28+
> : ^^^^^^
29+
30+
return 2;
31+
>2 : 2
32+
> : ^
33+
}
34+
}
35+

‎tests/baselines/reference/goToDefinitionOverriddenMember17.baseline.jsonc

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// === goToDefinition ===
2+
// === /tests/cases/fourslash/goToDefinitionOverriddenMember17.ts ===
3+
// const entityKind = Symbol.for("drizzle:entityKind");
4+
//
5+
// abstract class MySqlColumn {
6+
// <|static readonly [|[entityKind]|]: string = "MySqlColumn";|>
7+
// }
8+
//
9+
// export class MySqlVarBinary extends MySqlColumn {
10+
// static /*GOTO DEF*/override readonly [entityKind]: string = "MySqlVarBinary";
11+
// }
12+
13+
// === Details ===
14+
[
15+
{
16+
"kind": "property",
17+
"name": "[entityKind]",
18+
"containerName": "MySqlColumn",
19+
"isLocal": true,
20+
"isAmbient": false,
21+
"unverified": false
22+
}
23+
]

‎tests/baselines/reference/goToDefinitionOverriddenMember18.baseline.jsonc

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// === goToDefinition ===
2+
// === /tests/cases/fourslash/goToDefinitionOverriddenMember18.ts ===
3+
// const entityKind = Symbol.for("drizzle:entityKind");
4+
//
5+
// abstract class MySqlColumn {
6+
// <|readonly [|[entityKind]|]: string = "MySqlColumn";|>
7+
// }
8+
//
9+
// export class MySqlVarBinary extends MySqlColumn {
10+
// /*GOTO DEF*/override readonly [entityKind]: string = "MySqlVarBinary";
11+
// }
12+
13+
// === Details ===
14+
[
15+
{
16+
"kind": "property",
17+
"name": "[entityKind]",
18+
"containerName": "MySqlColumn",
19+
"isLocal": true,
20+
"isAmbient": false,
21+
"unverified": false
22+
}
23+
]

0 commit comments

Comments
 (0)