refactor(language-service): rename all references to selectorless directives (#61307)

crisbeto · crisbeto · commit 3aef3e64cdf8 · 2025-05-14T11:06:22.000+02:00
Follow-up to #61240 that adds renaming support for selectorless components/directives both from the template and from the TypeScript source. PR Close #61307
diff --git a/packages/language-service/src/references_and_rename.ts b/packages/language-service/src/references_and_rename.ts
@@ -5,26 +5,21 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.dev/license
  */
-import {AST, TmplAstComponent, TmplAstDirective, TmplAstNode} from '@angular/compiler';
+import {AST, TmplAstComponent, TmplAstNode} from '@angular/compiler';
 import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
 import {absoluteFrom} from '@angular/compiler-cli/src/ngtsc/file_system';
-import {MetaKind, PipeMeta} from '@angular/compiler-cli/src/ngtsc/metadata';
+import {MetaKind, PipeMeta, DirectiveMeta} from '@angular/compiler-cli/src/ngtsc/metadata';
 import {PerfPhase} from '@angular/compiler-cli/src/ngtsc/perf';
-import {
-  SelectorlessComponentSymbol,
-  SelectorlessDirectiveSymbol,
-  SymbolKind,
-  TemplateTypeChecker,
-} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
+import {SymbolKind, TemplateTypeChecker} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
 import ts from 'typescript';
 
 import {
   convertToTemplateDocumentSpan,
   FilePosition,
   getParentClassMeta,
   getRenameTextAndSpanAtPosition,
-  getSelectorlessTemplateSpanFromTcbLocations,
   getTargetDetailsAtTemplatePosition,
+  SelectorlessCollector,
   TemplateLocationDetails,
 } from './references_and_rename_utils';
 import {collectMemberMethods, findTightestNode} from './utils/ts_utils';
@@ -138,9 +133,6 @@ interface SelectorRenameContext {
 interface SelectorlessIdentifierRenameContext {
   type: RequestKind.SelectorlessIdentifier;
 
-  /** Node defining the component/directive. */
-  templateNode: TmplAstComponent | TmplAstDirective;
-
   /** Identifier of the class defining the class. */
   identifier: ts.Identifier;
 
@@ -311,7 +303,7 @@ export class RenameBuilder {
     return allRenameLocations.length > 0 ? allRenameLocations : null;
   }
 
-  findRenameLocationsAtTypescriptPosition(
+  private findRenameLocationsAtTypescriptPosition(
     renameRequest: RenameRequest,
   ): readonly ts.RenameLocation[] | null {
     return this.compiler.perfRecorder.inPhase(PerfPhase.LsReferencesAndRenames, () => {
@@ -336,15 +328,11 @@ export class RenameBuilder {
       for (const location of locations) {
         if (this.ttc.isTrackedTypeCheckFile(absoluteFrom(location.fileName))) {
           if (renameRequest.type === RequestKind.SelectorlessIdentifier) {
-            const selectorlessEntries = getSelectorlessTemplateSpanFromTcbLocations(
-              location,
-              this.ttc,
-              this.compiler.getCurrentProgram(),
-              renameRequest.templateNode,
-            );
-            if (selectorlessEntries !== null) {
-              entries.push(...selectorlessEntries);
+            const selectorlessEntries = this.getSelectorlessRenameLocations(renameRequest);
+            if (selectorlessEntries === null) {
+              return null;
             }
+            entries.push(...selectorlessEntries);
           } else {
             const entry = convertToTemplateDocumentSpan(
               location,
@@ -410,13 +398,15 @@ export class RenameBuilder {
           targetDetails.symbol.kind === SymbolKind.SelectorlessComponent ||
           targetDetails.symbol.kind === SymbolKind.SelectorlessDirective
         ) {
-          const renameRequest = this.buildSelectorlessRenameRequestFromTemplate(
-            targetDetails.symbol,
-          );
-          if (renameRequest === null) {
+          const tsSymbol = targetDetails.symbol.tsSymbol;
+          const meta =
+            tsSymbol === null || tsSymbol.valueDeclaration === undefined
+              ? null
+              : this.compiler.getMeta(tsSymbol.valueDeclaration);
+          if (meta === null || meta.kind !== MetaKind.Directive) {
             return null;
           }
-          renameRequests.push(renameRequest);
+          renameRequests.push(this.buildSelectorlessRenameRequest(meta));
         } else {
           const renameRequest: RenameRequest = {
             type: RequestKind.DirectFromTemplate,
@@ -440,11 +430,16 @@ export class RenameBuilder {
       return null;
     }
     const meta = getParentClassMeta(requestNode, this.compiler);
-    if (meta !== null && meta.kind === MetaKind.Pipe && meta.nameExpr === requestNode) {
+
+    if (meta?.kind === MetaKind.Pipe && meta.nameExpr === requestNode) {
       return this.buildPipeRenameRequest(meta);
-    } else {
-      return {type: RequestKind.DirectFromTypeScript, requestNode};
     }
+
+    if (meta?.kind === MetaKind.Directive && meta.ref.node.name === requestNode) {
+      return this.buildSelectorlessRenameRequest(meta);
+    }
+
+    return {type: RequestKind.DirectFromTypeScript, requestNode};
   }
 
   private buildPipeRenameRequest(meta: PipeMeta): PipeRenameContext | null {
@@ -473,38 +468,100 @@ export class RenameBuilder {
     };
   }
 
-  private buildSelectorlessRenameRequestFromTemplate(
-    symbol: SelectorlessComponentSymbol | SelectorlessDirectiveSymbol,
-  ): SelectorlessIdentifierRenameContext | null {
-    if (symbol.tsSymbol === null || symbol.tsSymbol.valueDeclaration === undefined) {
-      return null;
-    }
+  private buildSelectorlessRenameRequest(meta: DirectiveMeta): SelectorlessIdentifierRenameContext {
+    const identifier = meta.ref.node.name;
+
+    return {
+      type: RequestKind.SelectorlessIdentifier,
+      identifier,
+      renamePosition: {
+        fileName: identifier.getSourceFile().fileName,
+        position: identifier.getStart(),
+      },
+    };
+  }
 
-    const meta = this.compiler.getMeta(symbol.tsSymbol.valueDeclaration);
-    if (meta === null || meta.kind !== MetaKind.Directive) {
+  /** Gets the rename locations for a selectorless request. */
+  private getSelectorlessRenameLocations(
+    request: SelectorlessIdentifierRenameContext,
+  ): ts.RenameLocation[] | null {
+    // Find all the references to the class.
+    const refs = this.tsLS.getReferencesAtPosition(
+      request.renamePosition.fileName,
+      request.renamePosition.position,
+    );
+
+    if (refs === undefined) {
       return null;
     }
 
-    const nameNode = meta.ref.node.name;
-    const templateName =
-      symbol.kind === SymbolKind.SelectorlessComponent
-        ? symbol.templateNode.componentName
-        : symbol.templateNode.name;
+    const entries: ts.RenameLocation[] = [];
+    let hasSelectorlessReferences = false;
 
-    // Do not rename aliased references.
-    if (templateName !== nameNode.text) {
-      return null;
+    for (const ref of refs) {
+      // Preserve the TS-based references.
+      if (!this.ttc.isTrackedTypeCheckFile(absoluteFrom(ref.fileName))) {
+        entries.push(ref);
+        continue;
+      }
+
+      // Resolve the TCB references to their real locations.
+      const entry = convertToTemplateDocumentSpan(ref, this.ttc, this.compiler.getCurrentProgram());
+      const typeCheckInfo =
+        entry === null
+          ? undefined
+          : getTypeCheckInfoAtPosition(entry.fileName, entry.textSpan.start, this.compiler);
+
+      if (entry === null || typeCheckInfo === undefined) {
+        continue;
+      }
+
+      const nodes = SelectorlessCollector.getSelectorlessNodes(typeCheckInfo.nodes);
+
+      // Go through all the selectorless template nodes and look for matches.
+      for (const node of nodes) {
+        const startSpan = node.startSourceSpan;
+        const isComponent = node instanceof TmplAstComponent;
+        const name = isComponent ? node.componentName : node.name;
+
+        if (
+          // The span of the template node should match the span of the reference.
+          startSpan.start.offset !== entry.textSpan.start ||
+          startSpan.end.offset !== entry.textSpan.start + entry.textSpan.length ||
+          // Skip aliased directives.
+          name !== request.identifier.text
+        ) {
+          continue;
+        }
+
+        hasSelectorlessReferences = true;
+
+        entries.push({
+          fileName: entry.fileName,
+          textSpan: {
+            // +1 to skip over the `<` for components and `@` for directives.
+            start: entry.textSpan.start + 1,
+            length: name.length,
+          },
+        });
+
+        // Components also need to rename the closing tag.
+        if (isComponent && !node.isSelfClosing && node.endSourceSpan !== null) {
+          entries.push({
+            fileName: entry.fileName,
+            textSpan: {
+              // +2 to skip over the `</` of the closing tag.
+              start: node.endSourceSpan.start.offset + 2,
+              length: name.length,
+            },
+          });
+        }
+      }
     }
 
-    return {
-      type: RequestKind.SelectorlessIdentifier,
-      templateNode: symbol.templateNode,
-      identifier: nameNode,
-      renamePosition: {
-        fileName: meta.ref.node.getSourceFile().fileName,
-        position: nameNode.getStart(),
-      },
-    };
+    // Do not produce any rename locations if there weren't any references in the template.
+    // This is for backwards compatibility since we should fall back to the TS language service.
+    return hasSelectorlessReferences ? entries : null;
   }
 }
 
diff --git a/packages/language-service/src/references_and_rename_utils.ts b/packages/language-service/src/references_and_rename_utils.ts
@@ -22,6 +22,8 @@ import {
   TmplAstVariable,
   TmplAstComponent,
   TmplAstDirective,
+  TmplAstRecursiveVisitor,
+  tmplAstVisitAll,
 } from '@angular/compiler';
 import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
 import {absoluteFrom} from '@angular/compiler-cli/src/ngtsc/file_system';
@@ -412,71 +414,20 @@ export function getParentClassMeta(
   return compiler.getMeta(parentClass);
 }
 
-/**
- * Converts a given `ts.DocumentSpan` in a shim file into one or more spans in the template,
- * representing a selectorless component or directive. There can be more than one return value
- * when a component has a closing tag.
- */
-export function getSelectorlessTemplateSpanFromTcbLocations(
-  shimDocumentSpan: ts.DocumentSpan,
-  templateTypeChecker: TemplateTypeChecker,
-  program: ts.Program,
-  node: TmplAstComponent | TmplAstDirective,
-): ts.DocumentSpan[] | null {
-  const sf = program.getSourceFile(shimDocumentSpan.fileName);
-  if (sf === undefined) {
-    return null;
-  }
+/** Visitor that collects all selectorless AST nodes from a template. */
+export class SelectorlessCollector extends TmplAstRecursiveVisitor {
+  private nodes: (TmplAstComponent | TmplAstDirective)[] = [];
 
-  let tcbNode = findTightestNode(sf, shimDocumentSpan.textSpan.start);
-  if (tcbNode === undefined) {
-    return null;
+  static getSelectorlessNodes(nodes: TmplAstNode[]): (TmplAstComponent | TmplAstDirective)[] {
+    const visitor = new SelectorlessCollector();
+    tmplAstVisitAll(visitor, nodes);
+    return visitor.nodes;
   }
 
-  // Variables in the typecheck block are generated with the type on the right hand
-  // side: `var _t1 = null! as i1.DirA`. Finding references of DirA will return the type
-  // assertion and we need to map it back to the variable identifier _t1.
-  if (hasExpressionIdentifier(sf, tcbNode, ExpressionIdentifier.VARIABLE_AS_EXPRESSION)) {
-    while (tcbNode && !ts.isVariableDeclaration(tcbNode)) {
-      tcbNode = tcbNode.parent;
+  visit(node: TmplAstNode) {
+    if (node instanceof TmplAstComponent || node instanceof TmplAstDirective) {
+      this.nodes.push(node);
     }
+    node.visit(this);
   }
-
-  const mapping = getTemplateLocationFromTcbLocation(
-    templateTypeChecker,
-    absoluteFrom(shimDocumentSpan.fileName),
-    /* tcbIsShim */ true,
-    tcbNode.getStart(),
-  );
-
-  if (mapping === null) {
-    return null;
-  }
-
-  const fileName = mapping.templateUrl;
-  const {length} = node instanceof TmplAstComponent ? node.componentName : node.name;
-  const spans: ts.DocumentSpan[] = [
-    {
-      fileName,
-      textSpan: {
-        // +1 because of the opening `<` or `@`.
-        start: node.startSourceSpan.start.offset + 1,
-        length,
-      },
-    },
-  ];
-
-  // If it's not a self-closing template tag, we need to rename the end tag too.
-  if (node instanceof TmplAstComponent && !node.isSelfClosing && node.endSourceSpan !== null) {
-    spans.push({
-      fileName,
-      textSpan: {
-        // +2 because of the `</`.
-        start: node.endSourceSpan.start.offset + 2,
-        length,
-      },
-    });
-  }
-
-  return spans;
 }
diff --git a/packages/language-service/test/references_and_rename_spec.ts b/packages/language-service/test/references_and_rename_spec.ts
@@ -2286,17 +2286,20 @@ describe('find references and rename locations', () => {
       it('should handle rename request for selectorless component from source file', () => {
         const file = project.openFile('test-component.ts');
         const template = project.openFile('app.html');
-        template.contents = '<TestComponent/>';
+        template.contents =
+          '<TestComponent/> {{123 + 456}} <div><TestComponent>Hello</TestComponent></div>';
         file.moveCursorToText('export class TestCom¦ponent {');
         const renameLocations = getRenameLocationsAtPosition(file)!;
 
-        expect(renameLocations).toBeUndefined();
-
-        // TODO(crisbeto): investigate if we can make this work.
-        // It would involve looking for all the component references and renaming them.
-        // expect(renameLocations.length).toBe(3);
-        // assertTextSpans(renameLocations, ['TestComponent']);
-        // assertFileNames(renameLocations, ['test-component.ts', 'app.html', 'app.ts']);
+        // There are 5 locations that need to be renamed:
+        // - Source file where the component is defined.
+        // - Self-closing tag in the template.
+        // - Opening tag in the template.
+        // - Closing tag in the template.
+        // - Import in the app component.
+        expect(renameLocations.length).toBe(5);
+        assertTextSpans(renameLocations, ['TestComponent']);
+        assertFileNames(renameLocations, ['test-component.ts', 'app.html', 'app.ts']);
       });
     });
   });
@@ -2476,17 +2479,19 @@ describe('find references and rename locations', () => {
       it('should handle rename request for selectorless component from source file', () => {
         const file = project.openFile('test-directive.ts');
         const template = project.openFile('app.html');
-        template.contents = '<div @TestDirective></div>';
+        template.contents =
+          '<div @TestDirective><span><input @TestDirective([value]="numberValue")></span></div>';
         file.moveCursorToText('export class TestDir¦ective {');
         const renameLocations = getRenameLocationsAtPosition(file)!;
 
-        expect(renameLocations).toBeUndefined();
-
-        // TODO(crisbeto): investigate if we can make this work.
-        // It would involve looking for all the component references and renaming them.
-        // expect(renameLocations.length).toBe(3);
-        // assertTextSpans(renameLocations, ['TestDirective']);
-        // assertFileNames(renameLocations, ['test-directive.ts', 'app.html', 'app.ts']);
+        // There are 4 locations that need to be renamed:
+        // - Source file where the directive is defined.
+        // - Reference on the `div` node.
+        // - Refernce on the `input` node.
+        // - Import in the app component.
+        expect(renameLocations.length).toBe(4);
+        assertTextSpans(renameLocations, ['TestDirective']);
+        assertFileNames(renameLocations, ['test-directive.ts', 'app.html', 'app.ts']);
       });
     });
   });
