From 976259877585a4ae286c8732ce78c5d57bf5ad42 Mon Sep 17 00:00:00 2001 From: Andy Date: Fri, 6 Apr 2018 16:30:55 -0700 Subject: [PATCH] findAllReferences: forEachRelatedSymbol can always include shorthand destructuring (#23223) * findAllReferences: forEachRelatedSymbol can always include shorthand destructuring * Update rename tests --- src/compiler/checker.ts | 3 --- src/compiler/types.ts | 4 ++++ src/services/findAllReferences.ts | 19 +++++++------------ .../reference/api/tsserverlibrary.d.ts | 4 ++++ tests/baselines/reference/api/typescript.d.ts | 4 ++++ .../renameDestructuringAssignmentInFor2.ts | 10 +++++++--- .../renameDestructuringAssignmentInForOf2.ts | 10 +++++++--- ...ucturingAssignmentNestedInArrayLiteral2.ts | 10 +++++++--- ...nameDestructuringAssignmentNestedInFor2.ts | 10 +++++++--- ...meDestructuringAssignmentNestedInForOf2.ts | 11 +++++++---- 10 files changed, 54 insertions(+), 31 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 21e7ae28da9..7213d4dbbb9 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -25493,9 +25493,6 @@ namespace ts { } function getShorthandAssignmentValueSymbol(location: Node): Symbol { - // The function returns a value symbol of an identifier in the short-hand property assignment. - // This is necessary as an identifier in short-hand property assignment can contains two meaning: - // property name and property value. if (location && location.kind === SyntaxKind.ShorthandPropertyAssignment) { return resolveEntityName((location).name, SymbolFlags.Value | SymbolFlags.Alias); } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index d9305c23b3e..42e1604f69c 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2839,6 +2839,10 @@ namespace ts { getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[]; getSymbolAtLocation(node: Node): Symbol | undefined; getSymbolsOfParameterPropertyDeclaration(parameter: ParameterDeclaration, parameterName: string): Symbol[]; + /** + * The function returns the value (local variable) symbol of an identifier in the short-hand property assignment. + * This is necessary as an identifier in short-hand property assignment can contains two meaning: property name and property value. + */ getShorthandAssignmentValueSymbol(location: Node): Symbol | undefined; getExportSpecifierLocalTargetSymbol(location: ExportSpecifier): Symbol | undefined; /** diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index 412e29c3556..98dd4666e8d 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -1377,8 +1377,7 @@ namespace ts.FindAllReferences.Core { forEachRelatedSymbol(symbol, location, checker, (sym, root, base) => { result.push(base || root || sym); }, parameterProperties => { result.push(...parameterProperties); }, - /*allowBaseTypes*/ () => !implementations, - /*includeShorthandDestructuring*/ false); + /*allowBaseTypes*/ () => !implementations); return result; } @@ -1387,7 +1386,6 @@ namespace ts.FindAllReferences.Core { cbSymbol: (symbol: Symbol, rootSymbol?: Symbol, baseSymbol?: Symbol) => T | undefined, cbParameterProperties: (s: Symbol[]) => T | undefined, allowBaseTypes: (rootSymbol: Symbol) => boolean, - includeShorthandDestructuring: boolean, ): T | undefined { const containingObjectLiteralElement = getContainingObjectLiteralElement(location); if (containingObjectLiteralElement) { @@ -1400,11 +1398,9 @@ namespace ts.FindAllReferences.Core { // If the location is name of property symbol from object literal destructuring pattern // Search the property symbol // for ( { property: p2 } of elems) { } - if (includeShorthandDestructuring || containingObjectLiteralElement.kind !== SyntaxKind.ShorthandPropertyAssignment) { - const propertySymbol = getPropertySymbolOfDestructuringAssignment(location, checker); - const res = propertySymbol && cbSymbol(propertySymbol); - if (res) return res; - } + const propertySymbol = getPropertySymbolOfDestructuringAssignment(location, checker); + const res1 = propertySymbol && cbSymbol(propertySymbol); + if (res1) return res1; /* Because in short-hand property assignment, location has two meaning : property name and as value of the property * When we do findAllReference at the position of the short-hand property assignment, we would want to have references to position of @@ -1418,8 +1414,8 @@ namespace ts.FindAllReferences.Core { * will be included correctly. */ const shorthandValueSymbol = checker.getShorthandAssignmentValueSymbol(location.parent); - const res1 = shorthandValueSymbol && cbSymbol(shorthandValueSymbol); - if (res1) return res1; + const res2 = shorthandValueSymbol && cbSymbol(shorthandValueSymbol); + if (res2) return res2; } const res = fromRoot(symbol); @@ -1495,8 +1491,7 @@ namespace ts.FindAllReferences.Core { ? getRelatedSymbol(search, find(paramProps, x => !!(x.flags & SymbolFlags.Property))!, referenceLocation, state) : undefined, /*allowBaseTypes*/ rootSymbol => - !(search.parents && !some(search.parents, parent => explicitlyInheritsFrom(rootSymbol.parent, parent, state.inheritsFromCache, checker))), - /*includeShorthandDestructuring*/ true); + !(search.parents && !some(search.parents, parent => explicitlyInheritsFrom(rootSymbol.parent, parent, state.inheritsFromCache, checker)))); } /** Gets all symbols for one property. Does not get symbols for every property. */ diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 05ad706c56f..0e5a4873627 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -1783,6 +1783,10 @@ declare namespace ts { getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[]; getSymbolAtLocation(node: Node): Symbol | undefined; getSymbolsOfParameterPropertyDeclaration(parameter: ParameterDeclaration, parameterName: string): Symbol[]; + /** + * The function returns the value (local variable) symbol of an identifier in the short-hand property assignment. + * This is necessary as an identifier in short-hand property assignment can contains two meaning: property name and property value. + */ getShorthandAssignmentValueSymbol(location: Node): Symbol | undefined; getExportSpecifierLocalTargetSymbol(location: ExportSpecifier): Symbol | undefined; /** diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 2278e8bb509..5b6ef18b680 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -1783,6 +1783,10 @@ declare namespace ts { getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[]; getSymbolAtLocation(node: Node): Symbol | undefined; getSymbolsOfParameterPropertyDeclaration(parameter: ParameterDeclaration, parameterName: string): Symbol[]; + /** + * The function returns the value (local variable) symbol of an identifier in the short-hand property assignment. + * This is necessary as an identifier in short-hand property assignment can contains two meaning: property name and property value. + */ getShorthandAssignmentValueSymbol(location: Node): Symbol | undefined; getExportSpecifierLocalTargetSymbol(location: ExportSpecifier): Symbol | undefined; /** diff --git a/tests/cases/fourslash/renameDestructuringAssignmentInFor2.ts b/tests/cases/fourslash/renameDestructuringAssignmentInFor2.ts index 4d79b47a267..17173d0da4d 100644 --- a/tests/cases/fourslash/renameDestructuringAssignmentInFor2.ts +++ b/tests/cases/fourslash/renameDestructuringAssignmentInFor2.ts @@ -1,7 +1,7 @@ /// ////interface I { -//// property1: number; +//// [|property1|]: number; //// property2: string; ////} ////var elems: I[]; @@ -10,7 +10,11 @@ ////for ({ [|property1|] } = elems[0]; p2 < 100; p2++) { //// p2 = [|property1|]++; ////} -////for ({ property1: p2 } = elems[0]; p2 < 100; p2++) { +////for ({ [|property1|]: p2 } = elems[0]; p2 < 100; p2++) { ////} -verify.rangesAreRenameLocations(); +const ranges = test.ranges(); +const [r0, r1, r2, r3, r4] = ranges; +verify.renameLocations([r0, r4], [r0, r2, r4]); +verify.renameLocations([r1, r3], [r1, r2, r3]); +verify.renameLocations(r2, ranges); diff --git a/tests/cases/fourslash/renameDestructuringAssignmentInForOf2.ts b/tests/cases/fourslash/renameDestructuringAssignmentInForOf2.ts index 9987a17d36e..b7432e08a26 100644 --- a/tests/cases/fourslash/renameDestructuringAssignmentInForOf2.ts +++ b/tests/cases/fourslash/renameDestructuringAssignmentInForOf2.ts @@ -1,7 +1,7 @@ /// ////interface I { -//// property1: number; +//// [|property1|]: number; //// property2: string; ////} ////var elems: I[]; @@ -10,7 +10,11 @@ ////for ({ [|property1|] } of elems) { //// [|property1|]++; ////} -////for ({ property1: p2 } of elems) { +////for ({ [|property1|]: p2 } of elems) { ////} -verify.rangesAreRenameLocations(); +const ranges = test.ranges(); +const [r0, r1, r2, r3, r4] = ranges; +verify.renameLocations([r0, r4], [r0, r2, r4]); +verify.renameLocations([r1, r3], [r1, r2, r3]); +verify.renameLocations(r2, ranges); diff --git a/tests/cases/fourslash/renameDestructuringAssignmentNestedInArrayLiteral2.ts b/tests/cases/fourslash/renameDestructuringAssignmentNestedInArrayLiteral2.ts index e45ae3349a4..1f18c443ca6 100644 --- a/tests/cases/fourslash/renameDestructuringAssignmentNestedInArrayLiteral2.ts +++ b/tests/cases/fourslash/renameDestructuringAssignmentNestedInArrayLiteral2.ts @@ -1,11 +1,15 @@ /// ////interface I { -//// property1: number; +//// [|property1|]: number; //// property2: string; ////} ////var elems: I[], p1: number, [|property1|]: number; -////[{ property1: p1 }] = elems; +////[{ [|property1|]: p1 }] = elems; ////[{ [|property1|] }] = elems; -verify.rangesAreRenameLocations(); +const ranges = test.ranges(); +const [r0, r1, r2, r3] = ranges; +verify.renameLocations([r0, r2], [r0, r2, r3]); +verify.renameLocations(r1, [r1, r3]); +verify.renameLocations(r3, ranges); diff --git a/tests/cases/fourslash/renameDestructuringAssignmentNestedInFor2.ts b/tests/cases/fourslash/renameDestructuringAssignmentNestedInFor2.ts index 2ec7d906ad1..c5e7212c99b 100644 --- a/tests/cases/fourslash/renameDestructuringAssignmentNestedInFor2.ts +++ b/tests/cases/fourslash/renameDestructuringAssignmentNestedInFor2.ts @@ -3,16 +3,20 @@ ////interface MultiRobot { //// name: string; //// skills: { -//// primary: string; +//// [|primary|]: string; //// secondary: string; //// }; ////} ////let multiRobot: MultiRobot, [|primary|]: string; -////for ({ skills: { primary: primaryA, secondary: secondaryA } } = multiRobot, i = 0; i < 1; i++) { +////for ({ skills: { [|primary|]: primaryA, secondary: secondaryA } } = multiRobot, i = 0; i < 1; i++) { //// console.log(primaryA); ////} ////for ({ skills: { [|primary|], secondary } } = multiRobot, i = 0; i < 1; i++) { //// console.log([|primary|]); ////} -verify.rangesAreRenameLocations(); +const ranges = test.ranges(); +const [r0, r1, r2, r3, r4] = ranges; +verify.renameLocations([r0, r2], [r0, r2, r3]); +verify.renameLocations([r1, r4], [r1, r3, r4]); +verify.renameLocations(r3, ranges); diff --git a/tests/cases/fourslash/renameDestructuringAssignmentNestedInForOf2.ts b/tests/cases/fourslash/renameDestructuringAssignmentNestedInForOf2.ts index 5364cffebb9..74dfda39bc2 100644 --- a/tests/cases/fourslash/renameDestructuringAssignmentNestedInForOf2.ts +++ b/tests/cases/fourslash/renameDestructuringAssignmentNestedInForOf2.ts @@ -3,17 +3,20 @@ ////interface MultiRobot { //// name: string; //// skills: { -//// primary: string; +//// [|primary|]: string; //// secondary: string; //// }; ////} ////let multiRobots: MultiRobot[], [|primary|]: string; -////for ({ skills: { primary: primaryA, secondary: secondaryA } } of multiRobots) { +////for ({ skills: { [|primary|]: primaryA, secondary: secondaryA } } of multiRobots) { //// console.log(primaryA); ////} ////for ({ skills: { [|primary|], secondary } } of multiRobots) { //// console.log([|primary|]); ////} - -verify.rangesAreRenameLocations(); +const ranges = test.ranges(); +const [r0, r1, r2, r3, r4] = ranges; +verify.renameLocations([r0, r2], [r0, r2, r3]); +verify.renameLocations([r1, r4], [r1, r3, r4]); +verify.renameLocations(r3, ranges);