Merge pull request #6227 from Microsoft/fix4581_inheritedPropertiesCrash

Fix crash in language service when using on inherited properties of self-extend

src/services/services.ts

@@ -6018,14 +6018,38 @@ namespace ts {
 
                     // Add symbol of properties/methods of the same name in base classes and implemented interfaces definitions
                     if (rootSymbol.parent && rootSymbol.parent.flags & (SymbolFlags.Class | SymbolFlags.Interface)) {
-                        getPropertySymbolsFromBaseTypes(rootSymbol.parent, rootSymbol.getName(), result);
+                        getPropertySymbolsFromBaseTypes(rootSymbol.parent, rootSymbol.getName(), result, /*previousIterationSymbolsCache*/ {});
                     }
                 });
 
                 return result;
             }
 
-            function getPropertySymbolsFromBaseTypes(symbol: Symbol, propertyName: string, result: Symbol[]): void {
+            /**
+             * Find symbol of the given property-name and add the symbol to the given result array
+             * @param symbol a symbol to start searching for the given propertyName
+             * @param propertyName a name of property to serach for
+             * @param result an array of symbol of found property symbols
+             * @param previousIterationSymbolsCache a cache of symbol from previous iterations of calling this function to prevent infinite revisitng of the same symbol.
+             *                                The value of previousIterationSymbol is undefined when the function is first called.
+             */
+            function getPropertySymbolsFromBaseTypes(symbol: Symbol, propertyName: string, result: Symbol[],
+                previousIterationSymbolsCache: SymbolTable): void {
+                // If the current symbol is the same as the previous-iteration symbol, we can just return the symbol that has already been visited
+                // This is particularly important for the following cases, so that we do not infinitely visit the same symbol.
+                // For example:
+                //      interface C extends C {
+                //          /*findRef*/propName: string;
+                //      }
+                // The first time getPropertySymbolsFromBaseTypes is called when finding-all-references at propName,
+                // the symbol argument will be the symbol of an interface "C" and previousIterationSymbol is undefined,
+                // the function will add any found symbol of the property-name, then its sub-routine will call
+                // getPropertySymbolsFromBaseTypes again to walk up any base types to prevent revisiting already
+                // visited symbol, interface "C", the sub-routine will pass the current symbol as previousIterationSymbol.
+                if (hasProperty(previousIterationSymbolsCache, symbol.name)) {
+                    return;
+                }
+
                 if (symbol && symbol.flags & (SymbolFlags.Class | SymbolFlags.Interface)) {
                     forEach(symbol.getDeclarations(), declaration => {
                         if (declaration.kind === SyntaxKind.ClassDeclaration) {
@@ -6049,7 +6073,8 @@ namespace ts {
                             }
 
                             // Visit the typeReference as well to see if it directly or indirectly use that property
-                            getPropertySymbolsFromBaseTypes(type.symbol, propertyName, result);
+                            previousIterationSymbolsCache[symbol.name] = symbol;
+                            getPropertySymbolsFromBaseTypes(type.symbol, propertyName, result, previousIterationSymbolsCache);
                         }
                     }
                 }
@@ -6101,7 +6126,7 @@ namespace ts {
                     // see if any is in the list
                     if (rootSymbol.parent && rootSymbol.parent.flags & (SymbolFlags.Class | SymbolFlags.Interface)) {
                         const result: Symbol[] = [];
-                        getPropertySymbolsFromBaseTypes(rootSymbol.parent, rootSymbol.getName(), result);
+                        getPropertySymbolsFromBaseTypes(rootSymbol.parent, rootSymbol.getName(), result, /*previousIterationSymbolsCache*/ {});
                         return forEach(result, s => searchSymbols.indexOf(s) >= 0 ? s : undefined);
                     }
 

tests/cases/fourslash/documentHighlightAtInheritedProperties1.ts

@@ -0,0 +1,13 @@
+/// <reference path='fourslash.ts'/>
+
+// @Filename: file1.ts
+//// interface interface1 extends interface1 {
+////    /*1*/doStuff(): void;
+////    /*2*/propName: string;
+//// }
+
+let markers = test.markers()
+for (let marker of markers) {
+    goTo.position(marker.position);
+    verify.documentHighlightsAtPositionCount(1, ["file1.ts"]);
+}

tests/cases/fourslash/documentHighlightAtInheritedProperties2.ts

@@ -0,0 +1,13 @@
+/// <reference path='fourslash.ts'/>
+
+// @Filename: file1.ts
+//// class class1 extends class1 {
+////    /*1*/doStuff() { }
+////    /*2*/propName: string;
+//// }
+
+let markers = test.markers()
+for (let marker of markers) {
+    goTo.position(marker.position);
+    verify.documentHighlightsAtPositionCount(1, ["file1.ts"]);
+}

tests/cases/fourslash/documentHighlightAtInheritedProperties3.ts

@@ -0,0 +1,17 @@
+/// <reference path='fourslash.ts'/>
+
+// @Filename: file1.ts
+//// interface interface1 extends interface1 {
+////    /*1*/doStuff(): void;
+////    /*2*/propName: string;
+//// }
+////
+//// var v: interface1;
+//// v./*3*/propName;
+//// v./*4*/doStuff();
+
+let markers = test.markers()
+for (let marker of markers) {
+    goTo.position(marker.position);
+    verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
+}

tests/cases/fourslash/documentHighlightAtInheritedProperties4.ts

@@ -0,0 +1,17 @@
+/// <reference path='fourslash.ts'/>
+
+// @Filename: file1.ts
+//// class class1 extends class1 {
+////    /*1*/doStuff() { }
+////    /*2*/propName: string;
+//// }
+////
+//// var c: class1;
+//// c./*3*/doStuff();
+//// c./*4*/propName;
+
+let markers = test.markers()
+for (let marker of markers) {
+    goTo.position(marker.position);
+    verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
+}

tests/cases/fourslash/documentHighlightAtInheritedProperties5.ts

@@ -0,0 +1,30 @@
+/// <reference path='fourslash.ts'/>
+
+// @Filename: file1.ts
+//// interface C extends D {
+////     /*0*/prop0: string;
+////     /*1*/prop1: number;
+//// }
+//// 
+//// interface D extends C {
+////     /*2*/prop0: string;
+////     /*3*/prop1: number;
+//// }
+//// 
+//// var d: D;
+//// d./*4*/prop1;
+
+goTo.marker("0");
+verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
+
+goTo.marker("1");
+verify.documentHighlightsAtPositionCount(3, ["file1.ts"]);
+
+goTo.marker("2");
+verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
+
+goTo.marker("3");
+verify.documentHighlightsAtPositionCount(3, ["file1.ts"]);
+
+goTo.marker("4");
+verify.documentHighlightsAtPositionCount(3, ["file1.ts"]);

tests/cases/fourslash/documentHighlightAtInheritedProperties6.ts

@@ -0,0 +1,30 @@
+/// <reference path='fourslash.ts'/>
+
+// @Filename: file1.ts
+//// class C extends D {
+////     /*0*/prop0: string;
+////     /*1*/prop1: string;
+//// }
+//// 
+//// class D extends C {
+////     /*2*/prop0: string;
+////     /*3*/prop1: string;
+//// }
+//// 
+//// var d: D;
+//// d./*4*/prop1;
+
+goTo.marker("0");
+verify.documentHighlightsAtPositionCount(1, ["file1.ts"]);
+
+goTo.marker("1");
+verify.documentHighlightsAtPositionCount(1, ["file1.ts"]);
+
+goTo.marker("2");
+verify.documentHighlightsAtPositionCount(1, ["file1.ts"]);
+
+goTo.marker("3");
+verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
+
+goTo.marker("4");
+verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);

tests/cases/fourslash/findAllRefsInheritedProperties1.ts

@@ -0,0 +1,25 @@
+/// <reference path='fourslash.ts'/>
+
+//// class class1 extends class1 {
+////    [|doStuff|]() { }
+////    [|propName|]: string;
+//// }
+////
+//// var v: class1;
+//// v.[|doStuff|]();
+//// v.[|propName|];
+
+function verifyReferences(query: FourSlashInterface.Range, references: FourSlashInterface.Range[]) {
+    goTo.position(query.start);
+    for (const ref of references) {
+        verify.referencesAtPositionContains(ref);
+    }
+}
+
+const ranges = test.ranges();
+verify.assertHasRanges(ranges);
+const [r0, r1, r2, r3] = ranges;
+verifyReferences(r0, [r0, r2]);
+verifyReferences(r1, [r1, r3]);
+verifyReferences(r2, [r0, r2]);
+verifyReferences(r3, [r1, r3]);

tests/cases/fourslash/findAllRefsInheritedProperties2.ts

@@ -0,0 +1,25 @@
+/// <reference path='fourslash.ts'/>
+
+//// interface interface1 extends interface1 {
+////    [|doStuff|](): void;   // r0
+////    [|propName|]: string;  // r1
+//// }
+////
+//// var v: interface1;
+//// v.[|doStuff|]();  // r2
+//// v.[|propName|];   // r3
+
+function verifyReferences(query: FourSlashInterface.Range, references: FourSlashInterface.Range[]) {
+    goTo.position(query.start);
+    for (const ref of references) {
+        verify.referencesAtPositionContains(ref);
+    }
+}
+
+const ranges = test.ranges();
+verify.assertHasRanges(ranges);
+const [r0, r1, r2, r3] = ranges;
+verifyReferences(r0, [r0, r2]);
+verifyReferences(r1, [r1, r3]);
+verifyReferences(r2, [r0, r2]);
+verifyReferences(r3, [r1, r3]);

tests/cases/fourslash/findAllRefsInheritedProperties3.ts

@@ -0,0 +1,37 @@
+/// <reference path='fourslash.ts'/>
+
+//// class class1 extends class1 {
+////     [|doStuff|]() { }     // r0
+////     [|propName|]: string; // r1
+//// }
+//// interface interface1 extends interface1 {
+////     [|doStuff|](): void;   // r2
+////     [|propName|]: string;  // r3
+//// }
+//// class class2 extends class1 implements interface1 {
+////     [|doStuff|]() { }      // r4
+////     [|propName|]: string;  // r5
+//// }
+//// 
+//// var v: class2;
+//// v.[|propName|];   // r6
+//// v.[|doStuff|]();  // r7
+
+function verifyReferences(query: FourSlashInterface.Range, references: FourSlashInterface.Range[]) {
+    goTo.position(query.start);
+    for (const ref of references) {
+        verify.referencesAtPositionContains(ref);
+    }
+}
+
+const ranges = test.ranges();
+verify.assertHasRanges(ranges);
+const [r0, r1, r2, r3, r4, r5, r6, r7] = ranges;
+verifyReferences(r0, [r0]);
+verifyReferences(r1, [r1, r5, r6]);
+verifyReferences(r2, [r2, r4, r7]);
+verifyReferences(r3, [r3, r5, r6]);
+verifyReferences(r4, [r2, r4, r7]);
+verifyReferences(r5, [r1, r3, r5, r6]);
+verifyReferences(r6, [r1, r3, r5, r6]);
+verifyReferences(r7, [r2, r4, r7]);

tests/cases/fourslash/findAllRefsInheritedProperties4.ts

@@ -0,0 +1,30 @@
+/// <reference path='fourslash.ts'/>
+
+//// interface C extends D {
+////     [|prop0|]: string;  // r0
+////     [|prop1|]: number;  // r1
+//// }
+////
+//// interface D extends C {
+////     [|prop0|]: string;  // r2
+//// }
+////
+//// var d: D;
+//// d.[|prop0|];  // r3
+//// d.[|prop1|];  // r4
+
+function verifyReferences(query: FourSlashInterface.Range, references: FourSlashInterface.Range[]) {
+    goTo.position(query.start);
+    for (const ref of references) {
+        verify.referencesAtPositionContains(ref);
+    }
+}
+
+const ranges = test.ranges();
+verify.assertHasRanges(ranges);
+const [r0, r1, r2, r3, r4] = ranges;
+verifyReferences(r0, [r0, r2, r3]);
+verifyReferences(r1, [r1]);
+verifyReferences(r2, [r0, r2, r3]);
+verifyReferences(r3, [r0, r2, r3]);
+verifyReferences(r4, []);

tests/cases/fourslash/findAllRefsInheritedProperties5.ts

@@ -0,0 +1,30 @@
+/// <reference path='fourslash.ts'/>
+
+//// class C extends D {
+////     [|prop0|]: string;  // r0
+////     [|prop1|]: number;  // r1
+//// }
+////
+//// class D extends C {
+////     [|prop0|]: string;  // r2
+//// }
+////
+//// var d: D;
+//// d.[|prop0|];  // r3
+//// d.[|prop1|];  // r4
+
+function verifyReferences(query: FourSlashInterface.Range, references: FourSlashInterface.Range[]) {
+    goTo.position(query.start);
+    for (const ref of references) {
+        verify.referencesAtPositionContains(ref);
+    }
+}
+
+const ranges = test.ranges();
+verify.assertHasRanges(ranges);
+const [r0, r1, r2, r3, r4] = ranges;
+verifyReferences(r0, [r0]);
+verifyReferences(r1, [r1]);
+verifyReferences(r2, [r2, r3]);
+verifyReferences(r3, [r2, r3]);
+verifyReferences(r4, []);

tests/cases/fourslash/referencesForInheritedProperties3.ts

@@ -0,0 +1,15 @@
+/// <reference path='fourslash.ts'/>
+
+//// interface interface1 extends interface1 {
+////    /*1*/doStuff(): void;
+////    /*2*/propName: string;
+//// }
+////
+//// var v: interface1;
+//// v./*3*/propName;
+//// v./*4*/doStuff();
+
+test.markers().forEach(m => {
+    goTo.position(m.position, m.fileName);
+    verify.referencesCountIs(2);
+});

tests/cases/fourslash/referencesForInheritedProperties4.ts

@@ -0,0 +1,15 @@
+/// <reference path='fourslash.ts'/>
+
+//// class class1 extends class1 {
+////    /*1*/doStuff() { }
+////    /*2*/propName: string;
+//// }
+////
+//// var c: class1;
+//// c./*3*/doStuff();
+//// c./*4*/propName;
+
+test.markers().forEach(m => {
+    goTo.position(m.position, m.fileName);
+    verify.referencesCountIs(2);
+});

tests/cases/fourslash/referencesForInheritedProperties5.ts

@@ -0,0 +1,19 @@
+/// <reference path='fourslash.ts'/>
+
+//// interface interface1 extends interface1 {
+////    /*1*/doStuff(): void;
+////    /*2*/propName: string;
+//// }
+//// interface interface2 extends interface1 {
+////    /*3*/doStuff(): void;
+////    /*4*/propName: string;
+//// }
+////
+//// var v: interface1;
+//// v./*5*/propName;
+//// v./*6*/doStuff();
+
+test.markers().forEach(m => {
+    goTo.position(m.position, m.fileName);
+    verify.referencesCountIs(3);
+});

tests/cases/fourslash/referencesForInheritedProperties6.ts

@@ -0,0 +1,32 @@
+/// <reference path='fourslash.ts'/>
+
+//// class class1 extends class1 {
+////    /*1*/doStuff() { }
+////    /*2*/propName: string;
+//// }
+//// class class2 extends class1 {
+////    /*3*/doStuff() { }
+////    /*4*/propName: string;
+//// }
+////
+//// var v: class2;
+//// v./*5*/propName;
+//// v./*6*/doStuff();
+
+goTo.marker("1");
+verify.referencesCountIs(1);
+
+goTo.marker("2");
+verify.referencesCountIs(3);
+
+goTo.marker("3");
+verify.referencesCountIs(2);
+
+goTo.marker("4");
+verify.referencesCountIs(3);
+
+goTo.marker("5");
+verify.referencesCountIs(3);
+
+goTo.marker("6");
+verify.referencesCountIs(2);

tests/cases/fourslash/referencesForInheritedProperties7.ts

@@ -0,0 +1,42 @@
+/// <reference path='fourslash.ts'/>
+
+//// class class1 extends class1 {
+////    /*1*/doStuff() { }
+////    /*2*/propName: string;
+//// }
+//// interface interface1 extends interface1 {
+////    /*3*/doStuff(): void;
+////    /*4*/propName: string;
+//// }
+//// class class2 extends class1 implements interface1 {
+////    /*5*/doStuff() { }
+////    /*6*/propName: string;
+//// }
+////
+//// var v: class2;
+//// v./*7*/propName;
+//// v./*8*/doStuff();
+
+goTo.marker("1");
+verify.referencesCountIs(1);
+
+goTo.marker("2");
+verify.referencesCountIs(3);
+
+goTo.marker("3");
+verify.referencesCountIs(3);
+
+goTo.marker("4");
+verify.referencesCountIs(3);
+
+goTo.marker("5");
+verify.referencesCountIs(3);
+
+goTo.marker("6");
+verify.referencesCountIs(4);
+
+goTo.marker("7");
+verify.referencesCountIs(4);
+
+goTo.marker("8");
+verify.referencesCountIs(3);

tests/cases/fourslash/referencesForInheritedProperties8.ts

@@ -0,0 +1,27 @@
+/// <reference path='fourslash.ts'/>
+
+//// interface C extends D {
+////     /*0*/propD: number;
+//// }
+//// interface D extends C {
+////     /*1*/propD: string;
+////     /*3*/propC: number;
+//// }
+//// var d: D;
+//// d./*2*/propD;
+//// d./*4*/propC;
+
+goTo.marker("0");
+verify.referencesCountIs(3);
+
+goTo.marker("1");
+verify.referencesCountIs(3);
+
+goTo.marker("2");
+verify.referencesCountIs(3);
+
+goTo.marker("3");
+verify.referencesCountIs(2);
+
+goTo.marker("4");
+verify.referencesCountIs(2);

tests/cases/fourslash/referencesForInheritedProperties9.ts

@@ -0,0 +1,21 @@
+/// <reference path='fourslash.ts'/>
+
+//// class D extends C {
+////     /*0*/prop1: string;
+//// }
+//// 
+//// class C extends D {
+////     /*1*/prop1: string;
+//// }
+//// 
+//// var c: C;
+//// c./*2*/prop1;
+
+goTo.marker("0");
+verify.referencesCountIs(1);
+
+goTo.marker("1");
+verify.referencesCountIs(2)
+
+goTo.marker("2");
+verify.referencesCountIs(2)

tests/cases/fourslash/renameInheritedProperties1.ts

@@ -0,0 +1,15 @@
+/// <reference path='fourslash.ts'/>
+
+//// class class1 extends class1 {
+////    [|propName|]: string;
+//// }
+////
+//// var v: class1;
+//// v.[|propName|];
+
+const ranges = test.ranges();
+verify.assertHasRanges(ranges);
+for (const range of ranges) {
+    goTo.position(range.start);
+    verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
+}

tests/cases/fourslash/renameInheritedProperties2.ts

@@ -0,0 +1,15 @@
+/// <reference path='fourslash.ts'/>
+
+//// class class1 extends class1 {
+////    [|doStuff|]() { }
+//// }
+////
+//// var v: class1;
+//// v.[|doStuff|]();
+
+let ranges = test.ranges();
+verify.assertHasRanges(ranges);
+for (let range of ranges) {
+    goTo.position(range.start);
+    verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
+}

tests/cases/fourslash/renameInheritedProperties3.ts

@@ -0,0 +1,15 @@
+/// <reference path='fourslash.ts'/>
+
+//// interface interface1 extends interface1 {
+////    [|propName|]: string;
+//// }
+////
+//// var v: interface1;
+//// v.[|propName|];
+
+let ranges = test.ranges();
+verify.assertHasRanges(ranges);
+for (let range of ranges) {
+    goTo.position(range.start);
+    verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
+}

tests/cases/fourslash/renameInheritedProperties4.ts

@@ -0,0 +1,15 @@
+/// <reference path='fourslash.ts'/>
+
+//// interface interface1 extends interface1 {
+////    [|doStuff|](): string;
+//// }
+////
+//// var v: interface1;
+//// v.[|doStuff|]();
+
+let ranges = test.ranges();
+verify.assertHasRanges(ranges);
+for (let range of ranges) {
+    goTo.position(range.start);
+    verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
+}

tests/cases/fourslash/renameInheritedProperties5.ts

@@ -0,0 +1,17 @@
+/// <reference path='fourslash.ts'/>
+
+//// interface C extends D {
+////     propC: number;
+//// }
+//// interface D extends C {
+////     [|propD|]: string;
+//// }
+//// var d: D;
+//// d.[|propD|];
+
+const ranges = test.ranges();
+verify.assertHasRanges(ranges);
+for (const range of ranges) {
+    goTo.position(range.start);
+    verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
+}

tests/cases/fourslash/renameInheritedProperties6.ts

@@ -0,0 +1,17 @@
+/// <reference path='fourslash.ts'/>
+
+//// interface C extends D {
+////     propD: number;
+//// }
+//// interface D extends C {
+////     [|propC|]: number;
+//// }
+//// var d: D;
+//// d.[|propC|];
+
+const ranges = test.ranges();
+verify.assertHasRanges(ranges);
+for (const range of ranges) {
+    goTo.position(range.start);
+    verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
+}

tests/cases/fourslash/renameInheritedProperties7.ts

@@ -0,0 +1,19 @@
+/// <reference path='fourslash.ts'/>
+
+//// class C extends D {
+////     [|prop1|]: string;
+//// }
+//// 
+//// class D extends C {
+////     prop1: string;
+//// }
+//// 
+//// var c: C;
+//// c.[|prop1|];
+
+const ranges = test.ranges();
+verify.assertHasRanges(ranges);
+for (const range of ranges) {
+    goTo.position(range.start);
+    verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
+}

tests/cases/fourslash/renameInheritedProperties8.ts

@@ -0,0 +1,19 @@
+/// <reference path='fourslash.ts'/>
+
+//// class C implements D {
+////     [|prop1|]: string;
+//// }
+//// 
+//// interface D extends C {
+////     [|prop1|]: string;
+//// }
+//// 
+//// var c: C;
+//// c.[|prop1|];
+
+const ranges = test.ranges();
+verify.assertHasRanges(ranges);
+for (const range of ranges) {
+    goTo.position(range.start);
+    verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
+}