Skip to content

Commit 8c22770

Browse files
author
Andy
authored
Improve 'isWriteAccess' for findAllReferences (microsoft#26889)
1 parent ddba6d8 commit 8c22770

67 files changed

Lines changed: 183 additions & 118 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/compiler/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,7 @@ namespace ts {
739739
}
740740

741741
export interface ComputedPropertyName extends Node {
742+
parent: Declaration;
742743
kind: SyntaxKind.ComputedPropertyName;
743744
expression: Expression;
744745
}

‎src/compiler/utilities.ts‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2384,29 +2384,33 @@ namespace ts {
23842384
}
23852385

23862386
// See GH#16030
2387-
export function isAnyDeclarationName(name: Node): boolean {
2387+
export function getDeclarationFromName(name: Node): Declaration | undefined {
2388+
const parent = name.parent;
23882389
switch (name.kind) {
2389-
case SyntaxKind.Identifier:
23902390
case SyntaxKind.StringLiteral:
2391-
case SyntaxKind.NumericLiteral: {
2392-
const parent = name.parent;
2391+
case SyntaxKind.NumericLiteral:
2392+
if (isComputedPropertyName(parent)) return parent.parent;
2393+
// falls through
2394+
2395+
case SyntaxKind.Identifier:
23932396
if (isDeclaration(parent)) {
2394-
return parent.name === name;
2397+
return parent.name === name ? parent : undefined;
23952398
}
2396-
else if (isQualifiedName(name.parent)) {
2397-
const tag = name.parent.parent;
2398-
return isJSDocParameterTag(tag) && tag.name === name.parent;
2399+
else if (isQualifiedName(parent)) {
2400+
const tag = parent.parent;
2401+
return isJSDocParameterTag(tag) && tag.name === parent ? tag : undefined;
23992402
}
24002403
else {
2401-
const binExp = name.parent.parent;
2404+
const binExp = parent.parent;
24022405
return isBinaryExpression(binExp) &&
24032406
getSpecialPropertyAssignmentKind(binExp) !== SpecialPropertyAssignmentKind.None &&
24042407
(binExp.left.symbol || binExp.symbol) &&
2405-
getNameOfDeclaration(binExp) === name;
2408+
getNameOfDeclaration(binExp) === name
2409+
? binExp
2410+
: undefined;
24062411
}
2407-
}
24082412
default:
2409-
return false;
2413+
return undefined;
24102414
}
24112415
}
24122416

‎src/services/findAllReferences.ts‎

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ namespace ts.FindAllReferences {
154154
textSpan: getTextSpan(node, sourceFile),
155155
isWriteAccess: isWriteAccessForReference(node),
156156
isDefinition: node.kind === SyntaxKind.DefaultKeyword
157-
|| isAnyDeclarationName(node)
157+
|| !!getDeclarationFromName(node)
158158
|| isLiteralComputedPropertyDeclarationName(node),
159159
isInString,
160160
};
@@ -223,7 +223,65 @@ namespace ts.FindAllReferences {
223223

224224
/** A node is considered a writeAccess iff it is a name of a declaration or a target of an assignment */
225225
function isWriteAccessForReference(node: Node): boolean {
226-
return node.kind === SyntaxKind.DefaultKeyword || isAnyDeclarationName(node) || isWriteAccess(node);
226+
const decl = getDeclarationFromName(node);
227+
return !!decl && declarationIsWriteAccess(decl) || node.kind === SyntaxKind.DefaultKeyword || isWriteAccess(node);
228+
}
229+
230+
/**
231+
* True if 'decl' provides a value, as in `function f() {}`;
232+
* false if 'decl' is just a location for a future write, as in 'let x;'
233+
*/
234+
function declarationIsWriteAccess(decl: Declaration): boolean {
235+
// Consider anything in an ambient declaration to be a write access since it may be coming from JS.
236+
if (!!(decl.flags & NodeFlags.Ambient)) return true;
237+
238+
switch (decl.kind) {
239+
case SyntaxKind.BinaryExpression:
240+
case SyntaxKind.BindingElement:
241+
case SyntaxKind.ClassDeclaration:
242+
case SyntaxKind.ClassExpression:
243+
case SyntaxKind.DefaultKeyword:
244+
case SyntaxKind.EnumDeclaration:
245+
case SyntaxKind.EnumMember:
246+
case SyntaxKind.ExportSpecifier:
247+
case SyntaxKind.ImportClause: // default import
248+
case SyntaxKind.ImportEqualsDeclaration:
249+
case SyntaxKind.ImportSpecifier:
250+
case SyntaxKind.InterfaceDeclaration:
251+
case SyntaxKind.JSDocCallbackTag:
252+
case SyntaxKind.JSDocTypedefTag:
253+
case SyntaxKind.JsxAttribute:
254+
case SyntaxKind.ModuleDeclaration:
255+
case SyntaxKind.NamespaceExportDeclaration:
256+
case SyntaxKind.NamespaceImport:
257+
case SyntaxKind.Parameter:
258+
case SyntaxKind.PropertyAssignment:
259+
case SyntaxKind.ShorthandPropertyAssignment:
260+
case SyntaxKind.TypeAliasDeclaration:
261+
case SyntaxKind.TypeParameter:
262+
return true;
263+
264+
case SyntaxKind.FunctionDeclaration:
265+
case SyntaxKind.FunctionExpression:
266+
case SyntaxKind.Constructor:
267+
case SyntaxKind.MethodDeclaration:
268+
case SyntaxKind.GetAccessor:
269+
case SyntaxKind.SetAccessor:
270+
return !!(decl as FunctionDeclaration | FunctionExpression | ConstructorDeclaration | MethodDeclaration | GetAccessorDeclaration | SetAccessorDeclaration).body;
271+
272+
case SyntaxKind.VariableDeclaration:
273+
case SyntaxKind.PropertyDeclaration:
274+
return !!(decl as VariableDeclaration | PropertyDeclaration).initializer || isCatchClause(decl.parent);
275+
276+
case SyntaxKind.MethodSignature:
277+
case SyntaxKind.PropertySignature:
278+
case SyntaxKind.JSDocPropertyTag:
279+
case SyntaxKind.JSDocParameterTag:
280+
return false;
281+
282+
default:
283+
return Debug.failBadSyntaxKind(decl);
284+
}
227285
}
228286
}
229287

‎tests/baselines/reference/api/tsserverlibrary.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,7 @@ declare namespace ts {
536536
name?: Identifier | StringLiteral | NumericLiteral;
537537
}
538538
interface ComputedPropertyName extends Node {
539+
parent: Declaration;
539540
kind: SyntaxKind.ComputedPropertyName;
540541
expression: Expression;
541542
}

‎tests/baselines/reference/api/typescript.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,7 @@ declare namespace ts {
536536
name?: Identifier | StringLiteral | NumericLiteral;
537537
}
538538
interface ComputedPropertyName extends Node {
539+
parent: Declaration;
539540
kind: SyntaxKind.ComputedPropertyName;
540541
expression: Expression;
541542
}

‎tests/cases/fourslash/findAllReferencesJsDocTypeLiteral.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
//// * @param {string} o.x - a thing, its ok
99
//// * @param {number} o.y - another thing
1010
//// * @param {Object} o.nested - very nested
11-
//// * @param {boolean} o.nested.[|{| "isWriteAccess": true, "isDefinition": true |}great|] - much greatness
11+
//// * @param {boolean} o.nested.[|{| "isDefinition": true |}great|] - much greatness
1212
//// * @param {number} o.nested.times - twice? probably!??
1313
//// */
1414
//// function f(o) { return o.nested.[|great|]; }

‎tests/cases/fourslash/findAllRefsDestructureGeneric.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/// <reference path='fourslash.ts' />
22

33
////interface I<T> {
4-
//// [|{| "isWriteAccess": true, "isDefinition": true |}x|]: boolean;
4+
//// [|{| "isDefinition": true |}x|]: boolean;
55
////}
66
////declare const i: I<number>;
77
////const { [|{| "isWriteAccess": true, "isDefinition": true |}x|] } = i;

‎tests/cases/fourslash/findAllRefsForComputedProperties.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
////}
1010
////
1111
////var x: I = {
12-
//// ["[|{| "isDefinition": true |}prop1|]"]: function () { },
12+
//// ["[|{| "isWriteAccess": true, "isDefinition": true |}prop1|]"]: function () { },
1313
////}
1414

1515
const ranges = test.ranges();

‎tests/cases/fourslash/findAllRefsForComputedProperties2.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
////}
1010
////
1111
////var x: I = {
12-
//// ["[|{| "isDefinition": true |}42|]"]: function () { }
12+
//// ["[|{| "isWriteAccess": true, "isDefinition": true |}42|]"]: function () { }
1313
////}
1414

1515
const ranges = test.ranges();

‎tests/cases/fourslash/findAllRefsForMappedType.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/// <reference path='fourslash.ts'/>
22

3-
////interface T { [|{| "isWriteAccess": true, "isDefinition": true |}a|]: number };
3+
////interface T { [|{| "isDefinition": true |}a|]: number };
44
////type U = { [K in keyof T]: string };
55
////type V = { [K in keyof U]: boolean };
66
////const u: U = { [|{| "isWriteAccess": true, "isDefinition": true |}a|]: "" }

0 commit comments

Comments
 (0)