Merge pull request #6084 from Microsoft/fix4686_fixrename

Fix rename/document-highlight/find reference in parameter property declaration

src/compiler/binder.ts

@@ -1499,10 +1499,7 @@ namespace ts {
 
             // If this is a property-parameter, then also declare the property symbol into the
             // containing class.
-            if (node.flags & NodeFlags.AccessibilityModifier &&
-                node.parent.kind === SyntaxKind.Constructor &&
-                isClassLike(node.parent.parent)) {
-
+            if (isParameterPropertyDeclaration(node)) {
                 const classDeclaration = <ClassLikeDeclaration>node.parent.parent;
                 declareSymbol(classDeclaration.symbol.members, classDeclaration.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes);
             }

src/compiler/checker.ts

@@ -67,6 +67,7 @@ namespace ts {
             // The language service will always care about the narrowed type of a symbol, because that is
             // the type the language says the symbol should have.
             getTypeOfSymbolAtLocation: getNarrowedTypeOfSymbol,
+            getSymbolsOfParameterPropertyDeclaration,
             getDeclaredTypeOfSymbol,
             getPropertiesOfType,
             getPropertyOfType,
@@ -430,6 +431,26 @@ namespace ts {
             // return undefined if we can't find a symbol.
         }
 
+        /**
+         * Get symbols that represent parameter-property-declaration as parameter and as property declaration
+         * @param parameter a parameterDeclaration node
+         * @param parameterName a name of the parameter to get the symbols for.
+         * @return a tuple of two symbols 
+         */
+        function getSymbolsOfParameterPropertyDeclaration(parameter: ParameterDeclaration, parameterName: string): [Symbol, Symbol] {
+            const constructoDeclaration = parameter.parent;
+            const classDeclaration = parameter.parent.parent;
+
+            const parameterSymbol = getSymbol(constructoDeclaration.locals, parameterName, SymbolFlags.Value);
+            const propertySymbol = getSymbol(classDeclaration.symbol.members, parameterName, SymbolFlags.Value);
+
+            if (parameterSymbol && propertySymbol) {
+                return [parameterSymbol, propertySymbol];
+            }
+
+            Debug.fail("There should exist two symbols, one as property declaration and one as parameter declaration");
+        }
+
         function isBlockScopedNameDeclaredBeforeUse(declaration: Declaration, usage: Node): boolean {
             const declarationFile = getSourceFileOfNode(declaration);
             const useFile = getSourceFileOfNode(usage);

src/compiler/types.ts

@@ -1723,6 +1723,7 @@ namespace ts {
 
         getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[];
         getSymbolAtLocation(node: Node): Symbol;
+        getSymbolsOfParameterPropertyDeclaration(parameter: ParameterDeclaration, parameterName: string): Symbol[];
         getShorthandAssignmentValueSymbol(location: Node): Symbol;
         getTypeAtLocation(node: Node): Type;
         typeToString(type: Type, enclosingDeclaration?: Node, flags?: TypeFormatFlags): string;

src/compiler/utilities.ts

@@ -2748,4 +2748,8 @@ namespace ts {
             }
         }
     }
+
+    export function isParameterPropertyDeclaration(node: ParameterDeclaration): boolean {
+        return node.flags & NodeFlags.AccessibilityModifier && node.parent.kind === SyntaxKind.Constructor && isClassLike(node.parent.parent);
+    }
 }

src/harness/fourslash.ts

@@ -2782,6 +2782,10 @@ namespace FourSlashInterface {
             this.state.verifyCompletionListItemsCountIsGreaterThan(count, this.negative);
         }
 
+        public assertHasRanges(ranges: FourSlash.Range[]) {
+            assert(ranges.length !== 0, "Array of ranges is expected to be non-empty");
+        }
+
         public completionListIsEmpty() {
             this.state.verifyCompletionListIsEmpty(this.negative);
         }

src/services/services.ts

@@ -5934,7 +5934,7 @@ namespace ts {
 
             function populateSearchSymbolSet(symbol: Symbol, location: Node): Symbol[] {
                 // The search set contains at least the current symbol
-                const result = [symbol];
+                let result = [symbol];
 
                 // If the symbol is an alias, add what it alaises to the list
                 if (isImportOrExportSpecifierImportSymbol(symbol)) {
@@ -5966,6 +5966,15 @@ namespace ts {
                     }
                 }
 
+                // If the symbol.valueDeclaration is a property parameter declaration,
+                // we should include both parameter declaration symbol and property declaration symbol
+                // Parameter Declaration symbol is only visible within function scope, so the symbol is stored in contructor.locals.
+                // Property Declaration symbol is a member of the class, so the symbol is stored in its class Declaration.symbol.members
+                if (symbol.valueDeclaration && symbol.valueDeclaration.kind === SyntaxKind.Parameter &&
+                    isParameterPropertyDeclaration(<ParameterDeclaration>symbol.valueDeclaration)) {
+                    result = result.concat(typeChecker.getSymbolsOfParameterPropertyDeclaration(<ParameterDeclaration>symbol.valueDeclaration, symbol.name));
+                }
+
                 // If this is a union property, add all the symbols from all its source symbols in all unioned types.
                 // If the symbol is an instantiation from a another symbol (e.g. widened symbol) , add the root the list
                 forEach(typeChecker.getRootSymbols(symbol), rootSymbol => {

tests/cases/fourslash/documentHighlightAtParameterPropertyDeclaration1.ts

@@ -0,0 +1,24 @@
+/// <reference path='fourslash.ts'/>
+
+// @Filename: file1.ts
+//// class Foo {
+////     constructor(private /*0*/privateParam: number,
+////         public /*1*/publicParam: string,
+////         protected /*2*/protectedParam: boolean) {
+//// 
+////         let localPrivate = /*3*/privateParam;
+////         this./*4*/privateParam += 10;
+//// 
+////         let localPublic = /*5*/publicParam;
+////         this./*6*/publicParam += " Hello!";
+//// 
+////         let localProtected = /*7*/protectedParam;
+////         this./*8*/protectedParam = false;
+////     }
+//// }
+
+let markers = test.markers()
+for (let marker of markers) {
+    goTo.position(marker.position);
+    verify.documentHighlightsAtPositionCount(3, ["file1.ts"]);
+}

tests/cases/fourslash/documentHighlightAtParameterPropertyDeclaration2.ts

@@ -0,0 +1,24 @@
+/// <reference path='fourslash.ts'/>
+
+// @Filename: file1.ts
+//// class Foo {
+////     constructor(private {/*0*/privateParam}: number,
+////         public {/*1*/publicParam}: string,
+////         protected {/*2*/protectedParam}: boolean) {
+//// 
+////         let localPrivate = /*3*/privateParam;
+////         this.privateParam += 10; // this is not valid syntax
+//// 
+////         let localPublic = /*4*/publicParam;
+////         this.publicParam += " Hello!";  // this is not valid syntax
+//// 
+////         let localProtected = /*5*/protectedParam;
+////         this.protectedParam = false;  // this is not valid syntax
+////     }
+//// }
+
+let markers = test.markers()
+for (let marker of markers) {
+    goTo.position(marker.position);
+    verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
+}

tests/cases/fourslash/documentHighlightAtParameterPropertyDeclaration3.ts

@@ -0,0 +1,24 @@
+/// <reference path='fourslash.ts'/>
+
+// @Filename: file1.ts
+//// class Foo {
+////     constructor(private [/*0*/privateParam]: number,
+////         public [/*1*/publicParam]: string,
+////         protected [/*2*/protectedParam]: boolean) {
+//// 
+////         let localPrivate = /*3*/privateParam;
+////         this.privateParam += 10;  // this is not valid syntax
+//// 
+////         let localPublic = /*4*/publicParam;
+////         this.publicParam += " Hello!";  // this is not valid syntax
+//// 
+////         let localProtected = /*5*/protectedParam;
+////         this.protectedParam = false;  // this is not valid syntax
+////     }
+//// }
+
+let markers = test.markers()
+for (let marker of markers) {
+    goTo.position(marker.position);
+    verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
+}

tests/cases/fourslash/findAllRefsParameterPropertyDeclaration1.ts

@@ -0,0 +1,21 @@
+/// <reference path='fourslash.ts'/>
+
+//// class Foo {
+////     constructor(private [|privateParam|]: number) {
+////         let localPrivate = [|privateParam|];
+////         this.[|privateParam|] += 10;
+////     }
+//// }
+
+const ranges = test.ranges();
+verify.assertHasRanges(ranges);
+for (const range of ranges) {
+    goTo.position(range.start);
+
+    if (ranges.length) {
+        verify.referencesCountIs(ranges.length);
+        for (const expectedRange of ranges) {
+            verify.referencesAtPositionContains(expectedRange);
+        }
+    }
+}

tests/cases/fourslash/findAllRefsParameterPropertyDeclaration2.ts

@@ -0,0 +1,19 @@
+/// <reference path='fourslash.ts'/>
+
+//// class Foo {
+////     constructor(public [|publicParam|]: number) {
+////         let localPublic = [|publicParam|];
+////         this.[|publicParam|] += 10;
+////     }
+//// }
+
+let ranges = test.ranges();
+verify.assertHasRanges(ranges);
+for (let range of ranges) {
+    goTo.position(range.start);
+
+    verify.referencesCountIs(ranges.length);
+    for (let expectedRange of ranges) {
+        verify.referencesAtPositionContains(expectedRange);
+    }
+}

tests/cases/fourslash/findAllRefsParameterPropertyDeclaration3.ts

@@ -0,0 +1,19 @@
+/// <reference path='fourslash.ts'/>
+
+//// class Foo {
+////     constructor(protected [|protectedParam|]: number) {
+////         let localProtected = [|protectedParam|];
+////         this.[|protectedParam|] += 10;
+////     }
+//// }
+
+const ranges = test.ranges();
+verify.assertHasRanges(ranges);
+for (const range of ranges) {
+    goTo.position(range.start);
+
+    verify.referencesCountIs(ranges.length);
+    for (const expectedRange of ranges) {
+        verify.referencesAtPositionContains(expectedRange);
+    }
+}

tests/cases/fourslash/fourslash.ts

@@ -137,6 +137,7 @@ declare namespace FourSlashInterface {
         verifyDefinitionsName(name: string, containerName: string): void;
     }
     class verify extends verifyNegatable {
+        assertHasRanges(ranges: FourSlash.Range[]): void;
         caretAtMarker(markerName?: string): void;
         indentationIs(numberOfSpaces: number): void;
         indentationAtPositionIs(fileName: string, position: number, numberOfSpaces: number, indentStyle?: ts.IndentStyle): void;

tests/cases/fourslash/referenceInParameterPropertyDeclaration.ts

@@ -0,0 +1,24 @@
+/// <reference path='fourslash.ts'/>
+
+// @Filename: file1.ts
+//// class Foo {
+////     constructor(private /*0*/privateParam: number,
+////         public /*1*/publicParam: string,
+////         protected /*2*/protectedParam: boolean) {
+//// 
+////         let localPrivate = /*3*/privateParam;
+////         this./*4*/privateParam += 10;
+//// 
+////         let localPublic = /*5*/publicParam;
+////         this./*6*/publicParam += " Hello!";
+//// 
+////         let localProtected = /*7*/protectedParam;
+////         this./*8*/protectedParam = false;
+////     }
+//// }
+
+let markers = test.markers()
+for (let marker of markers) {
+    goTo.position(marker.position);
+    verify.referencesCountIs(3);
+}

tests/cases/fourslash/renameParameterPropertyDeclaration1.ts

@@ -0,0 +1,15 @@
+/// <reference path='fourslash.ts'/>
+
+//// class Foo {
+////     constructor(private [|privateParam|]: number) {
+////         let localPrivate = [|privateParam|];
+////         this.[|privateParam|] += 10;
+////     }
+//// }
+
+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/renameParameterPropertyDeclaration2.ts

@@ -0,0 +1,15 @@
+/// <reference path='fourslash.ts'/>
+
+//// class Foo {
+////     constructor(public [|publicParam|]: number) {
+////         let publicParam = [|publicParam|];
+////         this.[|publicParam|] += 10;
+////     }
+//// }
+
+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/renameParameterPropertyDeclaration3.ts

@@ -0,0 +1,15 @@
+/// <reference path='fourslash.ts'/>
+
+//// class Foo {
+////     constructor(protected [|protectedParam|]: number) {
+////         let protectedParam = [|protectedParam|];
+////         this.[|protectedParam|] += 10;
+////     }
+//// }
+
+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/renameParameterPropertyDeclaration4.ts

@@ -0,0 +1,14 @@
+/// <reference path='fourslash.ts'/>
+
+//// class Foo {
+////     constructor(protected { [|protectedParam|] }) {
+////         let myProtectedParam = [|protectedParam|];
+////     }
+//// }
+
+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/renameParameterPropertyDeclaration5.ts

@@ -0,0 +1,14 @@
+/// <reference path='fourslash.ts'/>
+
+//// class Foo {
+////     constructor(protected [ [|protectedParam|] ]) {
+////         let myProtectedParam = [|protectedParam|];
+////     }
+//// }
+
+let ranges = test.ranges();
+verify.assertHasRanges(ranges);
+for (let range of ranges) {
+    goTo.position(range.start);
+    verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
+}