Skip to content

Commit 9df1ed4

Browse files
yuityuit
committed
Merge pull request microsoft#6227 from Microsoft/fix4581_inheritedPropertiesCrash
Fix crash in language service when using on inherited properties of self-extend
2 parents c5dd17f + d4a04a1 commit 9df1ed4

27 files changed

Lines changed: 599 additions & 4 deletions

‎src/services/services.ts‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6018,14 +6018,38 @@ namespace ts {
60186018

60196019
// Add symbol of properties/methods of the same name in base classes and implemented interfaces definitions
60206020
if (rootSymbol.parent && rootSymbol.parent.flags & (SymbolFlags.Class | SymbolFlags.Interface)) {
6021-
getPropertySymbolsFromBaseTypes(rootSymbol.parent, rootSymbol.getName(), result);
6021+
getPropertySymbolsFromBaseTypes(rootSymbol.parent, rootSymbol.getName(), result, /*previousIterationSymbolsCache*/ {});
60226022
}
60236023
});
60246024

60256025
return result;
60266026
}
60276027

6028-
function getPropertySymbolsFromBaseTypes(symbol: Symbol, propertyName: string, result: Symbol[]): void {
6028+
/**
6029+
* Find symbol of the given property-name and add the symbol to the given result array
6030+
* @param symbol a symbol to start searching for the given propertyName
6031+
* @param propertyName a name of property to serach for
6032+
* @param result an array of symbol of found property symbols
6033+
* @param previousIterationSymbolsCache a cache of symbol from previous iterations of calling this function to prevent infinite revisitng of the same symbol.
6034+
* The value of previousIterationSymbol is undefined when the function is first called.
6035+
*/
6036+
function getPropertySymbolsFromBaseTypes(symbol: Symbol, propertyName: string, result: Symbol[],
6037+
previousIterationSymbolsCache: SymbolTable): void {
6038+
// If the current symbol is the same as the previous-iteration symbol, we can just return the symbol that has already been visited
6039+
// This is particularly important for the following cases, so that we do not infinitely visit the same symbol.
6040+
// For example:
6041+
// interface C extends C {
6042+
// /*findRef*/propName: string;
6043+
// }
6044+
// The first time getPropertySymbolsFromBaseTypes is called when finding-all-references at propName,
6045+
// the symbol argument will be the symbol of an interface "C" and previousIterationSymbol is undefined,
6046+
// the function will add any found symbol of the property-name, then its sub-routine will call
6047+
// getPropertySymbolsFromBaseTypes again to walk up any base types to prevent revisiting already
6048+
// visited symbol, interface "C", the sub-routine will pass the current symbol as previousIterationSymbol.
6049+
if (hasProperty(previousIterationSymbolsCache, symbol.name)) {
6050+
return;
6051+
}
6052+
60296053
if (symbol && symbol.flags & (SymbolFlags.Class | SymbolFlags.Interface)) {
60306054
forEach(symbol.getDeclarations(), declaration => {
60316055
if (declaration.kind === SyntaxKind.ClassDeclaration) {
@@ -6049,7 +6073,8 @@ namespace ts {
60496073
}
60506074

60516075
// Visit the typeReference as well to see if it directly or indirectly use that property
6052-
getPropertySymbolsFromBaseTypes(type.symbol, propertyName, result);
6076+
previousIterationSymbolsCache[symbol.name] = symbol;
6077+
getPropertySymbolsFromBaseTypes(type.symbol, propertyName, result, previousIterationSymbolsCache);
60536078
}
60546079
}
60556080
}
@@ -6101,7 +6126,7 @@ namespace ts {
61016126
// see if any is in the list
61026127
if (rootSymbol.parent && rootSymbol.parent.flags & (SymbolFlags.Class | SymbolFlags.Interface)) {
61036128
const result: Symbol[] = [];
6104-
getPropertySymbolsFromBaseTypes(rootSymbol.parent, rootSymbol.getName(), result);
6129+
getPropertySymbolsFromBaseTypes(rootSymbol.parent, rootSymbol.getName(), result, /*previousIterationSymbolsCache*/ {});
61056130
return forEach(result, s => searchSymbols.indexOf(s) >= 0 ? s : undefined);
61066131
}
61076132

‎tests/cases/fourslash/documentHighlightAtInheritedProperties1.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
// @Filename: file1.ts
4+
//// interface interface1 extends interface1 {
5+
//// /*1*/doStuff(): void;
6+
//// /*2*/propName: string;
7+
//// }
8+
9+
let markers = test.markers()
10+
for (let marker of markers) {
11+
goTo.position(marker.position);
12+
verify.documentHighlightsAtPositionCount(1, ["file1.ts"]);
13+
}

‎tests/cases/fourslash/documentHighlightAtInheritedProperties2.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
// @Filename: file1.ts
4+
//// class class1 extends class1 {
5+
//// /*1*/doStuff() { }
6+
//// /*2*/propName: string;
7+
//// }
8+
9+
let markers = test.markers()
10+
for (let marker of markers) {
11+
goTo.position(marker.position);
12+
verify.documentHighlightsAtPositionCount(1, ["file1.ts"]);
13+
}

‎tests/cases/fourslash/documentHighlightAtInheritedProperties3.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
// @Filename: file1.ts
4+
//// interface interface1 extends interface1 {
5+
//// /*1*/doStuff(): void;
6+
//// /*2*/propName: string;
7+
//// }
8+
////
9+
//// var v: interface1;
10+
//// v./*3*/propName;
11+
//// v./*4*/doStuff();
12+
13+
let markers = test.markers()
14+
for (let marker of markers) {
15+
goTo.position(marker.position);
16+
verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
17+
}

‎tests/cases/fourslash/documentHighlightAtInheritedProperties4.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
// @Filename: file1.ts
4+
//// class class1 extends class1 {
5+
//// /*1*/doStuff() { }
6+
//// /*2*/propName: string;
7+
//// }
8+
////
9+
//// var c: class1;
10+
//// c./*3*/doStuff();
11+
//// c./*4*/propName;
12+
13+
let markers = test.markers()
14+
for (let marker of markers) {
15+
goTo.position(marker.position);
16+
verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
17+
}

‎tests/cases/fourslash/documentHighlightAtInheritedProperties5.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
// @Filename: file1.ts
4+
//// interface C extends D {
5+
//// /*0*/prop0: string;
6+
//// /*1*/prop1: number;
7+
//// }
8+
////
9+
//// interface D extends C {
10+
//// /*2*/prop0: string;
11+
//// /*3*/prop1: number;
12+
//// }
13+
////
14+
//// var d: D;
15+
//// d./*4*/prop1;
16+
17+
goTo.marker("0");
18+
verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
19+
20+
goTo.marker("1");
21+
verify.documentHighlightsAtPositionCount(3, ["file1.ts"]);
22+
23+
goTo.marker("2");
24+
verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
25+
26+
goTo.marker("3");
27+
verify.documentHighlightsAtPositionCount(3, ["file1.ts"]);
28+
29+
goTo.marker("4");
30+
verify.documentHighlightsAtPositionCount(3, ["file1.ts"]);

‎tests/cases/fourslash/documentHighlightAtInheritedProperties6.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
// @Filename: file1.ts
4+
//// class C extends D {
5+
//// /*0*/prop0: string;
6+
//// /*1*/prop1: string;
7+
//// }
8+
////
9+
//// class D extends C {
10+
//// /*2*/prop0: string;
11+
//// /*3*/prop1: string;
12+
//// }
13+
////
14+
//// var d: D;
15+
//// d./*4*/prop1;
16+
17+
goTo.marker("0");
18+
verify.documentHighlightsAtPositionCount(1, ["file1.ts"]);
19+
20+
goTo.marker("1");
21+
verify.documentHighlightsAtPositionCount(1, ["file1.ts"]);
22+
23+
goTo.marker("2");
24+
verify.documentHighlightsAtPositionCount(1, ["file1.ts"]);
25+
26+
goTo.marker("3");
27+
verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
28+
29+
goTo.marker("4");
30+
verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);

‎tests/cases/fourslash/findAllRefsInheritedProperties1.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
//// class class1 extends class1 {
4+
//// [|doStuff|]() { }
5+
//// [|propName|]: string;
6+
//// }
7+
////
8+
//// var v: class1;
9+
//// v.[|doStuff|]();
10+
//// v.[|propName|];
11+
12+
function verifyReferences(query: FourSlashInterface.Range, references: FourSlashInterface.Range[]) {
13+
goTo.position(query.start);
14+
for (const ref of references) {
15+
verify.referencesAtPositionContains(ref);
16+
}
17+
}
18+
19+
const ranges = test.ranges();
20+
verify.assertHasRanges(ranges);
21+
const [r0, r1, r2, r3] = ranges;
22+
verifyReferences(r0, [r0, r2]);
23+
verifyReferences(r1, [r1, r3]);
24+
verifyReferences(r2, [r0, r2]);
25+
verifyReferences(r3, [r1, r3]);

‎tests/cases/fourslash/findAllRefsInheritedProperties2.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
//// interface interface1 extends interface1 {
4+
//// [|doStuff|](): void; // r0
5+
//// [|propName|]: string; // r1
6+
//// }
7+
////
8+
//// var v: interface1;
9+
//// v.[|doStuff|](); // r2
10+
//// v.[|propName|]; // r3
11+
12+
function verifyReferences(query: FourSlashInterface.Range, references: FourSlashInterface.Range[]) {
13+
goTo.position(query.start);
14+
for (const ref of references) {
15+
verify.referencesAtPositionContains(ref);
16+
}
17+
}
18+
19+
const ranges = test.ranges();
20+
verify.assertHasRanges(ranges);
21+
const [r0, r1, r2, r3] = ranges;
22+
verifyReferences(r0, [r0, r2]);
23+
verifyReferences(r1, [r1, r3]);
24+
verifyReferences(r2, [r0, r2]);
25+
verifyReferences(r3, [r1, r3]);

‎tests/cases/fourslash/findAllRefsInheritedProperties3.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
//// class class1 extends class1 {
4+
//// [|doStuff|]() { } // r0
5+
//// [|propName|]: string; // r1
6+
//// }
7+
//// interface interface1 extends interface1 {
8+
//// [|doStuff|](): void; // r2
9+
//// [|propName|]: string; // r3
10+
//// }
11+
//// class class2 extends class1 implements interface1 {
12+
//// [|doStuff|]() { } // r4
13+
//// [|propName|]: string; // r5
14+
//// }
15+
////
16+
//// var v: class2;
17+
//// v.[|propName|]; // r6
18+
//// v.[|doStuff|](); // r7
19+
20+
function verifyReferences(query: FourSlashInterface.Range, references: FourSlashInterface.Range[]) {
21+
goTo.position(query.start);
22+
for (const ref of references) {
23+
verify.referencesAtPositionContains(ref);
24+
}
25+
}
26+
27+
const ranges = test.ranges();
28+
verify.assertHasRanges(ranges);
29+
const [r0, r1, r2, r3, r4, r5, r6, r7] = ranges;
30+
verifyReferences(r0, [r0]);
31+
verifyReferences(r1, [r1, r5, r6]);
32+
verifyReferences(r2, [r2, r4, r7]);
33+
verifyReferences(r3, [r3, r5, r6]);
34+
verifyReferences(r4, [r2, r4, r7]);
35+
verifyReferences(r5, [r1, r3, r5, r6]);
36+
verifyReferences(r6, [r1, r3, r5, r6]);
37+
verifyReferences(r7, [r2, r4, r7]);

0 commit comments

Comments
 (0)