Skip to content

Commit a5dba0b

Browse files
author
Andy Hanson
committed
Improve 'isWriteAccess' for findAllReferences
1 parent 0ac3a0a commit a5dba0b

File tree

67 files changed

+193
-117
lines changed

Some content is hidden

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

67 files changed

+193
-117
lines changed

‎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
@@ -2369,29 +2369,33 @@ namespace ts {
23692369
}
23702370

23712371
// See GH#16030
2372-
export function isAnyDeclarationName(name: Node): boolean {
2372+
export function getDeclarationFromName(name: Node): Declaration | undefined {
2373+
const parent = name.parent;
23732374
switch (name.kind) {
2374-
case SyntaxKind.Identifier:
23752375
case SyntaxKind.StringLiteral:
2376-
case SyntaxKind.NumericLiteral: {
2377-
const parent = name.parent;
2376+
case SyntaxKind.NumericLiteral:
2377+
if (isComputedPropertyName(parent)) return parent.parent;
2378+
// falls through
2379+
2380+
case SyntaxKind.Identifier:
23782381
if (isDeclaration(parent)) {
2379-
return parent.name === name;
2382+
return parent.name === name ? parent : undefined;
23802383
}
2381-
else if (isQualifiedName(name.parent)) {
2382-
const tag = name.parent.parent;
2383-
return isJSDocParameterTag(tag) && tag.name === name.parent;
2384+
else if (isQualifiedName(parent)) {
2385+
const tag = parent.parent;
2386+
return isJSDocParameterTag(tag) && tag.name === parent ? tag : undefined;
23842387
}
23852388
else {
2386-
const binExp = name.parent.parent;
2389+
const binExp = parent.parent;
23872390
return isBinaryExpression(binExp) &&
23882391
getSpecialPropertyAssignmentKind(binExp) !== SpecialPropertyAssignmentKind.None &&
23892392
(binExp.left.symbol || binExp.symbol) &&
2390-
getNameOfDeclaration(binExp) === name;
2393+
getNameOfDeclaration(binExp) === name
2394+
? binExp
2395+
: undefined;
23912396
}
2392-
}
23932397
default:
2394-
return false;
2398+
return undefined;
23952399
}
23962400
}
23972401

‎src/services/findAllReferences.ts

Lines changed: 59 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,64 @@ 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.JSDocTypedefTag:
252+
case SyntaxKind.JsxAttribute:
253+
case SyntaxKind.ModuleDeclaration:
254+
case SyntaxKind.NamespaceExportDeclaration:
255+
case SyntaxKind.NamespaceImport:
256+
case SyntaxKind.Parameter:
257+
case SyntaxKind.PropertyAssignment:
258+
case SyntaxKind.ShorthandPropertyAssignment:
259+
case SyntaxKind.TypeAliasDeclaration:
260+
case SyntaxKind.TypeParameter:
261+
return true;
262+
263+
case SyntaxKind.FunctionDeclaration:
264+
case SyntaxKind.FunctionExpression:
265+
case SyntaxKind.Constructor:
266+
case SyntaxKind.MethodDeclaration:
267+
case SyntaxKind.GetAccessor:
268+
case SyntaxKind.SetAccessor:
269+
return !!(decl as FunctionDeclaration | FunctionExpression | ConstructorDeclaration | MethodDeclaration | GetAccessorDeclaration | SetAccessorDeclaration).body;
270+
271+
case SyntaxKind.VariableDeclaration:
272+
case SyntaxKind.PropertyDeclaration:
273+
return !!(decl as VariableDeclaration | PropertyDeclaration).initializer || isCatchClause(decl.parent);
274+
275+
case SyntaxKind.MethodSignature:
276+
case SyntaxKind.PropertySignature:
277+
case SyntaxKind.JSDocPropertyTag:
278+
case SyntaxKind.JSDocParameterTag:
279+
return false;
280+
281+
default:
282+
return Debug.failBadSyntaxKind(decl);
283+
}
227284
}
228285
}
229286

‎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|]: "" }

‎tests/cases/fourslash/findAllRefsForObjectSpread.ts

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

3-
////interface A1 { readonly [|{| "isWriteAccess": true, "isDefinition": true |}a|]: string };
4-
////interface A2 { [|{| "isWriteAccess": true, "isDefinition": true |}a|]?: number };
3+
////interface A1 { readonly [|{| "isDefinition": true |}a|]: string };
4+
////interface A2 { [|{| "isDefinition": true |}a|]?: number };
55
////let a1: A1;
66
////let a2: A2;
77
////let a12 = { ...a1, ...a2 };

‎tests/cases/fourslash/findAllRefsForRest.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
////interface Gen {
33
//// x: number
4-
//// [|{| "isWriteAccess": true, "isDefinition": true |}parent|]: Gen;
4+
//// [|{| "isDefinition": true |}parent|]: Gen;
55
//// millenial: string;
66
////}
77
////let t: Gen;

‎tests/cases/fourslash/findAllRefsInClassExpression.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 I { [|{| "isWriteAccess": true, "isDefinition": true |}boom|](): void; }
3+
////interface I { [|{| "isDefinition": true |}boom|](): void; }
44
////new class C implements I {
55
//// [|{| "isWriteAccess": true, "isDefinition": true |}boom|](){}
66
////}

‎tests/cases/fourslash/findAllRefsIndexedAccessTypes.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/// <reference path='fourslash.ts' />
22

33
////interface I {
4-
//// [|{| "isDefinition": true, "isWriteAccess": true |}0|]: number;
5-
//// [|{| "isDefinition": true, "isWriteAccess": true |}s|]: string;
4+
//// [|{| "isDefinition": true |}0|]: number;
5+
//// [|{| "isDefinition": true |}s|]: string;
66
////}
77
////interface J {
88
//// a: I[[|0|]],

‎tests/cases/fourslash/findAllRefsInheritedProperties1.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
//// class class1 extends class1 {
44
//// [|{| "isWriteAccess": true, "isDefinition": true |}doStuff|]() { }
5-
//// [|{| "isWriteAccess": true, "isDefinition": true |}propName|]: string;
5+
//// [|{| "isDefinition": true |}propName|]: string;
66
//// }
77
////
88
//// var v: class1;

‎tests/cases/fourslash/findAllRefsInheritedProperties2.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/// <reference path='fourslash.ts'/>
22

33
//// interface interface1 extends interface1 {
4-
//// [|{| "isWriteAccess": true, "isDefinition": true |}doStuff|](): void; // r0
5-
//// [|{| "isWriteAccess": true, "isDefinition": true |}propName|]: string; // r1
4+
//// [|{| "isDefinition": true |}doStuff|](): void; // r0
5+
//// [|{| "isDefinition": true |}propName|]: string; // r1
66
//// }
77
////
88
//// var v: interface1;

‎tests/cases/fourslash/findAllRefsInheritedProperties3.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@
22

33
//// class class1 extends class1 {
44
//// [|{| "isWriteAccess": true, "isDefinition": true |}doStuff|]() { } // r0
5-
//// [|{| "isWriteAccess": true, "isDefinition": true |}propName|]: string; // r1
5+
//// [|{| "isDefinition": true |}propName|]: string; // r1
66
//// }
77
//// interface interface1 extends interface1 {
8-
//// [|{| "isWriteAccess": true, "isDefinition": true |}doStuff|](): void; // r2
9-
//// [|{| "isWriteAccess": true, "isDefinition": true |}propName|]: string; // r3
8+
//// [|{| "isDefinition": true |}doStuff|](): void; // r2
9+
//// [|{| "isDefinition": true |}propName|]: string; // r3
1010
//// }
1111
//// class class2 extends class1 implements interface1 {
1212
//// [|{| "isWriteAccess": true, "isDefinition": true |}doStuff|]() { } // r4
13-
//// [|{| "isWriteAccess": true, "isDefinition": true |}propName|]: string; // r5
13+
//// [|{| "isDefinition": true |}propName|]: string; // r5
1414
//// }
1515
////
1616
//// var v: class2;

‎tests/cases/fourslash/findAllRefsInheritedProperties4.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
/// <reference path='fourslash.ts'/>
22

33
//// interface C extends D {
4-
//// [|{| "isWriteAccess": true, "isDefinition": true |}prop0|]: string; // r0
5-
//// [|{| "isWriteAccess": true, "isDefinition": true |}prop1|]: number; // r1
4+
//// [|{| "isDefinition": true |}prop0|]: string; // r0
5+
//// [|{| "isDefinition": true |}prop1|]: number; // r1
66
//// }
77
////
88
//// interface D extends C {
9-
//// [|{| "isWriteAccess": true, "isDefinition": true |}prop0|]: string; // r2
9+
//// [|{| "isDefinition": true |}prop0|]: string; // r2
1010
//// }
1111
////
1212
//// var d: D;

‎tests/cases/fourslash/findAllRefsInheritedProperties5.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
/// <reference path='fourslash.ts'/>
22

33
//// class C extends D {
4-
//// [|{| "isWriteAccess": true, "isDefinition": true |}prop0|]: string; // r0
5-
//// [|{| "isWriteAccess": true, "isDefinition": true |}prop1|]: number; // r1
4+
//// [|{| "isDefinition": true |}prop0|]: string; // r0
5+
//// [|{| "isDefinition": true |}prop1|]: number; // r1
66
//// }
77
////
88
//// class D extends C {
9-
//// [|{| "isWriteAccess": true, "isDefinition": true |}prop0|]: string; // r2
9+
//// [|{| "isDefinition": true |}prop0|]: string; // r2
1010
//// }
1111
////
1212
//// var d: D;

‎tests/cases/fourslash/findAllRefsMappedType.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 = { readonly [K in keyof T]?: string };
55
////declare const t: T;
66
////t.[|a|];

‎tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName01.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 {
4-
//// [|{| "isWriteAccess": true, "isDefinition": true |}property1|]: number;
4+
//// [|{| "isDefinition": true |}property1|]: number;
55
//// property2: string;
66
////}
77
////

‎tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName02.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 {
4-
//// [|{| "isWriteAccess": true, "isDefinition": true |}property1|]: number;
4+
//// [|{| "isDefinition": true |}property1|]: number;
55
//// property2: string;
66
////}
77
////

‎tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName03.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 {
4-
//// [|{| "isWriteAccess": true, "isDefinition": true |}property1|]: number;
4+
//// [|{| "isDefinition": true |}property1|]: number;
55
//// property2: string;
66
////}
77
////

‎tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName04.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 {
4-
//// [|{| "isWriteAccess": true, "isDefinition": true |}property1|]: number;
4+
//// [|{| "isDefinition": true |}property1|]: number;
55
//// property2: string;
66
////}
77
////

‎tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName06.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 {
4-
//// [|{| "isWriteAccess": true, "isDefinition": true |}property1|]: number;
4+
//// [|{| "isDefinition": true |}property1|]: number;
55
//// property2: string;
66
////}
77
////

‎tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName10.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 Recursive {
4-
//// [|{| "isWriteAccess": true, "isDefinition": true |}next|]?: Recursive;
4+
//// [|{| "isDefinition": true |}next|]?: Recursive;
55
//// value: any;
66
////}
77
////

‎tests/cases/fourslash/findAllRefsPropertyContextuallyTypedByTypeParam01.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 IFoo {
4-
//// [|{| "isWriteAccess": true, "isDefinition": true |}a|]: string;
4+
//// [|{| "isDefinition": true |}a|]: string;
55
////}
66
////class C<T extends IFoo> {
77
//// method() {

‎tests/cases/fourslash/findAllRefsReExportLocal.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// @noLib: true
44

55
// @Filename: /a.ts
6-
////var [|{| "isWriteAccess": true, "isDefinition": true |}x|];
6+
////var [|{| "isDefinition": true |}x|];
77
////export { [|{| "isWriteAccess": true, "isDefinition": true |}x|] };
88
////export { [|x|] as [|{| "isWriteAccess": true, "isDefinition": true |}y|] };
99

‎tests/cases/fourslash/findAllRefsRedeclaredPropertyInDerivedInterface.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
// @noLib: true
44

55
////interface A {
6-
//// readonly [|{| "isWriteAccess": true, "isDefinition": true |}x|]: number | string;
6+
//// readonly [|{| "isDefinition": true |}x|]: number | string;
77
////}
88
////interface B extends A {
9-
//// readonly [|{| "isWriteAccess": true, "isDefinition": true |}x|]: number;
9+
//// readonly [|{| "isDefinition": true |}x|]: number;
1010
////}
1111
////const a: A = { [|{| "isWriteAccess": true, "isDefinition": true |}x|]: 0 };
1212
////const b: B = { [|{| "isWriteAccess": true, "isDefinition": true |}x|]: 0 };

0 commit comments

Comments
 (0)