Improve 'isWriteAccess' for findAllReferences (#26889)

Author: Andy

src/compiler/types.ts

@@ -739,6 +739,7 @@ namespace ts {
     }
 
     export interface ComputedPropertyName extends Node {
+        parent: Declaration;
         kind: SyntaxKind.ComputedPropertyName;
         expression: Expression;
     }

src/compiler/utilities.ts

@@ -2384,29 +2384,33 @@ namespace ts {
     }
 
     // See GH#16030
-    export function isAnyDeclarationName(name: Node): boolean {
+    export function getDeclarationFromName(name: Node): Declaration | undefined {
+        const parent = name.parent;
         switch (name.kind) {
-            case SyntaxKind.Identifier:
             case SyntaxKind.StringLiteral:
-            case SyntaxKind.NumericLiteral: {
-                const parent = name.parent;
+            case SyntaxKind.NumericLiteral:
+                if (isComputedPropertyName(parent)) return parent.parent;
+                // falls through
+
+            case SyntaxKind.Identifier:
                 if (isDeclaration(parent)) {
-                    return parent.name === name;
+                    return parent.name === name ? parent : undefined;
                 }
-                else if (isQualifiedName(name.parent)) {
-                    const tag = name.parent.parent;
-                    return isJSDocParameterTag(tag) && tag.name === name.parent;
+                else if (isQualifiedName(parent)) {
+                    const tag = parent.parent;
+                    return isJSDocParameterTag(tag) && tag.name === parent ? tag : undefined;
                 }
                 else {
-                    const binExp = name.parent.parent;
+                    const binExp = parent.parent;
                     return isBinaryExpression(binExp) &&
                         getSpecialPropertyAssignmentKind(binExp) !== SpecialPropertyAssignmentKind.None &&
                         (binExp.left.symbol || binExp.symbol) &&
-                        getNameOfDeclaration(binExp) === name;
+                        getNameOfDeclaration(binExp) === name
+                        ? binExp
+                        : undefined;
                 }
-            }
             default:
-                return false;
+                return undefined;
         }
     }
 

src/services/findAllReferences.ts

@@ -154,7 +154,7 @@ namespace ts.FindAllReferences {
             textSpan: getTextSpan(node, sourceFile),
             isWriteAccess: isWriteAccessForReference(node),
             isDefinition: node.kind === SyntaxKind.DefaultKeyword
-                || isAnyDeclarationName(node)
+                || !!getDeclarationFromName(node)
                 || isLiteralComputedPropertyDeclarationName(node),
             isInString,
         };
@@ -223,7 +223,65 @@ namespace ts.FindAllReferences {
 
     /** A node is considered a writeAccess iff it is a name of a declaration or a target of an assignment */
     function isWriteAccessForReference(node: Node): boolean {
-        return node.kind === SyntaxKind.DefaultKeyword || isAnyDeclarationName(node) || isWriteAccess(node);
+        const decl = getDeclarationFromName(node);
+        return !!decl && declarationIsWriteAccess(decl) || node.kind === SyntaxKind.DefaultKeyword || isWriteAccess(node);
+    }
+
+    /**
+     * True if 'decl' provides a value, as in `function f() {}`;
+     * false if 'decl' is just a location for a future write, as in 'let x;'
+     */
+    function declarationIsWriteAccess(decl: Declaration): boolean {
+        // Consider anything in an ambient declaration to be a write access since it may be coming from JS.
+        if (!!(decl.flags & NodeFlags.Ambient)) return true;
+
+        switch (decl.kind) {
+            case SyntaxKind.BinaryExpression:
+            case SyntaxKind.BindingElement:
+            case SyntaxKind.ClassDeclaration:
+            case SyntaxKind.ClassExpression:
+            case SyntaxKind.DefaultKeyword:
+            case SyntaxKind.EnumDeclaration:
+            case SyntaxKind.EnumMember:
+            case SyntaxKind.ExportSpecifier:
+            case SyntaxKind.ImportClause: // default import
+            case SyntaxKind.ImportEqualsDeclaration:
+            case SyntaxKind.ImportSpecifier:
+            case SyntaxKind.InterfaceDeclaration:
+            case SyntaxKind.JSDocCallbackTag:
+            case SyntaxKind.JSDocTypedefTag:
+            case SyntaxKind.JsxAttribute:
+            case SyntaxKind.ModuleDeclaration:
+            case SyntaxKind.NamespaceExportDeclaration:
+            case SyntaxKind.NamespaceImport:
+            case SyntaxKind.Parameter:
+            case SyntaxKind.PropertyAssignment:
+            case SyntaxKind.ShorthandPropertyAssignment:
+            case SyntaxKind.TypeAliasDeclaration:
+            case SyntaxKind.TypeParameter:
+                return true;
+
+            case SyntaxKind.FunctionDeclaration:
+            case SyntaxKind.FunctionExpression:
+            case SyntaxKind.Constructor:
+            case SyntaxKind.MethodDeclaration:
+            case SyntaxKind.GetAccessor:
+            case SyntaxKind.SetAccessor:
+                return !!(decl as FunctionDeclaration | FunctionExpression | ConstructorDeclaration | MethodDeclaration | GetAccessorDeclaration | SetAccessorDeclaration).body;
+
+            case SyntaxKind.VariableDeclaration:
+            case SyntaxKind.PropertyDeclaration:
+                return !!(decl as VariableDeclaration | PropertyDeclaration).initializer || isCatchClause(decl.parent);
+
+            case SyntaxKind.MethodSignature:
+            case SyntaxKind.PropertySignature:
+            case SyntaxKind.JSDocPropertyTag:
+            case SyntaxKind.JSDocParameterTag:
+                return false;
+
+            default:
+                return Debug.failBadSyntaxKind(decl);
+        }
     }
 }
 

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

@@ -536,6 +536,7 @@ declare namespace ts {
         name?: Identifier | StringLiteral | NumericLiteral;
     }
     interface ComputedPropertyName extends Node {
+        parent: Declaration;
         kind: SyntaxKind.ComputedPropertyName;
         expression: Expression;
     }

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

@@ -536,6 +536,7 @@ declare namespace ts {
         name?: Identifier | StringLiteral | NumericLiteral;
     }
     interface ComputedPropertyName extends Node {
+        parent: Declaration;
         kind: SyntaxKind.ComputedPropertyName;
         expression: Expression;
     }

tests/cases/fourslash/findAllReferencesJsDocTypeLiteral.ts

@@ -8,7 +8,7 @@
 //// * @param {string} o.x - a thing, its ok
 //// * @param {number} o.y - another thing
 //// * @param {Object} o.nested - very nested
-//// * @param {boolean} o.nested.[|{| "isWriteAccess": true, "isDefinition": true |}great|] - much greatness
+//// * @param {boolean} o.nested.[|{| "isDefinition": true |}great|] - much greatness
 //// * @param {number} o.nested.times - twice? probably!??
 //// */
 //// function f(o) { return o.nested.[|great|]; }

tests/cases/fourslash/findAllRefsDestructureGeneric.ts

@@ -1,7 +1,7 @@
 /// <reference path='fourslash.ts' />
 
 ////interface I<T> {
-////    [|{| "isWriteAccess": true, "isDefinition": true |}x|]: boolean;
+////    [|{| "isDefinition": true |}x|]: boolean;
 ////}
 ////declare const i: I<number>;
 ////const { [|{| "isWriteAccess": true, "isDefinition": true |}x|] } = i;

tests/cases/fourslash/findAllRefsForComputedProperties.ts

@@ -9,7 +9,7 @@
 ////}
 ////
 ////var x: I = {
-////    ["[|{| "isDefinition": true |}prop1|]"]: function () { },
+////    ["[|{| "isWriteAccess": true, "isDefinition": true |}prop1|]"]: function () { },
 ////}
 
 const ranges = test.ranges();

tests/cases/fourslash/findAllRefsForComputedProperties2.ts

@@ -9,7 +9,7 @@
 ////}
 ////
 ////var x: I = {
-////    ["[|{| "isDefinition": true |}42|]"]: function () { }
+////    ["[|{| "isWriteAccess": true, "isDefinition": true |}42|]"]: function () { }
 ////}
 
 const ranges = test.ranges();

tests/cases/fourslash/findAllRefsForMappedType.ts

@@ -1,6 +1,6 @@
 /// <reference path='fourslash.ts'/>
 
-////interface T { [|{| "isWriteAccess": true, "isDefinition": true |}a|]: number };
+////interface T { [|{| "isDefinition": true |}a|]: number };
 ////type U = { [K in keyof T]: string };
 ////type V = { [K in keyof U]: boolean };
 ////const u: U = { [|{| "isWriteAccess": true, "isDefinition": true |}a|]: "" }