Skip to content

Commit 09d2233

Browse files
yuityuit
committed
Merge pull request microsoft#6084 from Microsoft/fix4686_fixrename
Fix rename/document-highlight/find reference in parameter property declaration
2 parents 0163675 + a5c632c commit 09d2233

19 files changed

Lines changed: 270 additions & 5 deletions

‎src/compiler/binder.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1499,10 +1499,7 @@ namespace ts {
14991499

15001500
// If this is a property-parameter, then also declare the property symbol into the
15011501
// containing class.
1502-
if (node.flags & NodeFlags.AccessibilityModifier &&
1503-
node.parent.kind === SyntaxKind.Constructor &&
1504-
isClassLike(node.parent.parent)) {
1505-
1502+
if (isParameterPropertyDeclaration(node)) {
15061503
const classDeclaration = <ClassLikeDeclaration>node.parent.parent;
15071504
declareSymbol(classDeclaration.symbol.members, classDeclaration.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes);
15081505
}

‎src/compiler/checker.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ namespace ts {
6767
// The language service will always care about the narrowed type of a symbol, because that is
6868
// the type the language says the symbol should have.
6969
getTypeOfSymbolAtLocation: getNarrowedTypeOfSymbol,
70+
getSymbolsOfParameterPropertyDeclaration,
7071
getDeclaredTypeOfSymbol,
7172
getPropertiesOfType,
7273
getPropertyOfType,
@@ -430,6 +431,26 @@ namespace ts {
430431
// return undefined if we can't find a symbol.
431432
}
432433

434+
/**
435+
* Get symbols that represent parameter-property-declaration as parameter and as property declaration
436+
* @param parameter a parameterDeclaration node
437+
* @param parameterName a name of the parameter to get the symbols for.
438+
* @return a tuple of two symbols
439+
*/
440+
function getSymbolsOfParameterPropertyDeclaration(parameter: ParameterDeclaration, parameterName: string): [Symbol, Symbol] {
441+
const constructoDeclaration = parameter.parent;
442+
const classDeclaration = parameter.parent.parent;
443+
444+
const parameterSymbol = getSymbol(constructoDeclaration.locals, parameterName, SymbolFlags.Value);
445+
const propertySymbol = getSymbol(classDeclaration.symbol.members, parameterName, SymbolFlags.Value);
446+
447+
if (parameterSymbol && propertySymbol) {
448+
return [parameterSymbol, propertySymbol];
449+
}
450+
451+
Debug.fail("There should exist two symbols, one as property declaration and one as parameter declaration");
452+
}
453+
433454
function isBlockScopedNameDeclaredBeforeUse(declaration: Declaration, usage: Node): boolean {
434455
const declarationFile = getSourceFileOfNode(declaration);
435456
const useFile = getSourceFileOfNode(usage);

‎src/compiler/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1723,6 +1723,7 @@ namespace ts {
17231723

17241724
getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[];
17251725
getSymbolAtLocation(node: Node): Symbol;
1726+
getSymbolsOfParameterPropertyDeclaration(parameter: ParameterDeclaration, parameterName: string): Symbol[];
17261727
getShorthandAssignmentValueSymbol(location: Node): Symbol;
17271728
getTypeAtLocation(node: Node): Type;
17281729
typeToString(type: Type, enclosingDeclaration?: Node, flags?: TypeFormatFlags): string;

‎src/compiler/utilities.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2748,4 +2748,8 @@ namespace ts {
27482748
}
27492749
}
27502750
}
2751+
2752+
export function isParameterPropertyDeclaration(node: ParameterDeclaration): boolean {
2753+
return node.flags & NodeFlags.AccessibilityModifier && node.parent.kind === SyntaxKind.Constructor && isClassLike(node.parent.parent);
2754+
}
27512755
}

‎src/harness/fourslash.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2782,6 +2782,10 @@ namespace FourSlashInterface {
27822782
this.state.verifyCompletionListItemsCountIsGreaterThan(count, this.negative);
27832783
}
27842784

2785+
public assertHasRanges(ranges: FourSlash.Range[]) {
2786+
assert(ranges.length !== 0, "Array of ranges is expected to be non-empty");
2787+
}
2788+
27852789
public completionListIsEmpty() {
27862790
this.state.verifyCompletionListIsEmpty(this.negative);
27872791
}

‎src/services/services.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5934,7 +5934,7 @@ namespace ts {
59345934

59355935
function populateSearchSymbolSet(symbol: Symbol, location: Node): Symbol[] {
59365936
// The search set contains at least the current symbol
5937-
const result = [symbol];
5937+
let result = [symbol];
59385938

59395939
// If the symbol is an alias, add what it alaises to the list
59405940
if (isImportOrExportSpecifierImportSymbol(symbol)) {
@@ -5966,6 +5966,15 @@ namespace ts {
59665966
}
59675967
}
59685968

5969+
// If the symbol.valueDeclaration is a property parameter declaration,
5970+
// we should include both parameter declaration symbol and property declaration symbol
5971+
// Parameter Declaration symbol is only visible within function scope, so the symbol is stored in contructor.locals.
5972+
// Property Declaration symbol is a member of the class, so the symbol is stored in its class Declaration.symbol.members
5973+
if (symbol.valueDeclaration && symbol.valueDeclaration.kind === SyntaxKind.Parameter &&
5974+
isParameterPropertyDeclaration(<ParameterDeclaration>symbol.valueDeclaration)) {
5975+
result = result.concat(typeChecker.getSymbolsOfParameterPropertyDeclaration(<ParameterDeclaration>symbol.valueDeclaration, symbol.name));
5976+
}
5977+
59695978
// If this is a union property, add all the symbols from all its source symbols in all unioned types.
59705979
// If the symbol is an instantiation from a another symbol (e.g. widened symbol) , add the root the list
59715980
forEach(typeChecker.getRootSymbols(symbol), rootSymbol => {

‎tests/cases/fourslash/documentHighlightAtParameterPropertyDeclaration1.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
// @Filename: file1.ts
4+
//// class Foo {
5+
//// constructor(private /*0*/privateParam: number,
6+
//// public /*1*/publicParam: string,
7+
//// protected /*2*/protectedParam: boolean) {
8+
////
9+
//// let localPrivate = /*3*/privateParam;
10+
//// this./*4*/privateParam += 10;
11+
////
12+
//// let localPublic = /*5*/publicParam;
13+
//// this./*6*/publicParam += " Hello!";
14+
////
15+
//// let localProtected = /*7*/protectedParam;
16+
//// this./*8*/protectedParam = false;
17+
//// }
18+
//// }
19+
20+
let markers = test.markers()
21+
for (let marker of markers) {
22+
goTo.position(marker.position);
23+
verify.documentHighlightsAtPositionCount(3, ["file1.ts"]);
24+
}

‎tests/cases/fourslash/documentHighlightAtParameterPropertyDeclaration2.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
// @Filename: file1.ts
4+
//// class Foo {
5+
//// constructor(private {/*0*/privateParam}: number,
6+
//// public {/*1*/publicParam}: string,
7+
//// protected {/*2*/protectedParam}: boolean) {
8+
////
9+
//// let localPrivate = /*3*/privateParam;
10+
//// this.privateParam += 10; // this is not valid syntax
11+
////
12+
//// let localPublic = /*4*/publicParam;
13+
//// this.publicParam += " Hello!"; // this is not valid syntax
14+
////
15+
//// let localProtected = /*5*/protectedParam;
16+
//// this.protectedParam = false; // this is not valid syntax
17+
//// }
18+
//// }
19+
20+
let markers = test.markers()
21+
for (let marker of markers) {
22+
goTo.position(marker.position);
23+
verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
24+
}

‎tests/cases/fourslash/documentHighlightAtParameterPropertyDeclaration3.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
// @Filename: file1.ts
4+
//// class Foo {
5+
//// constructor(private [/*0*/privateParam]: number,
6+
//// public [/*1*/publicParam]: string,
7+
//// protected [/*2*/protectedParam]: boolean) {
8+
////
9+
//// let localPrivate = /*3*/privateParam;
10+
//// this.privateParam += 10; // this is not valid syntax
11+
////
12+
//// let localPublic = /*4*/publicParam;
13+
//// this.publicParam += " Hello!"; // this is not valid syntax
14+
////
15+
//// let localProtected = /*5*/protectedParam;
16+
//// this.protectedParam = false; // this is not valid syntax
17+
//// }
18+
//// }
19+
20+
let markers = test.markers()
21+
for (let marker of markers) {
22+
goTo.position(marker.position);
23+
verify.documentHighlightsAtPositionCount(2, ["file1.ts"]);
24+
}

‎tests/cases/fourslash/findAllRefsParameterPropertyDeclaration1.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
//// class Foo {
4+
//// constructor(private [|privateParam|]: number) {
5+
//// let localPrivate = [|privateParam|];
6+
//// this.[|privateParam|] += 10;
7+
//// }
8+
//// }
9+
10+
const ranges = test.ranges();
11+
verify.assertHasRanges(ranges);
12+
for (const range of ranges) {
13+
goTo.position(range.start);
14+
15+
if (ranges.length) {
16+
verify.referencesCountIs(ranges.length);
17+
for (const expectedRange of ranges) {
18+
verify.referencesAtPositionContains(expectedRange);
19+
}
20+
}
21+
}

0 commit comments

Comments
 (0)