feat(language-service): initial implementation for `findRenameLocations` (#40140)

This commit lays the groundwork for potentially providing rename
locations from the Ivy native LS. The approach is very similar to what
was done with the feature to find references. One difference, however,
is that we did not require the references to be fully "correct". That
is, the exact text spans did not matter so much, as long as we provide a
location that logically includes the referenced item.

An example of a necessary difference between rename locations and references is
directives. The entire element in the template is a "reference" of the
directive's class. However, it's not a valid location to be renamed. The
same goes for aliased inputs/outputs. The locations in the template
directly map to the class property, which is correct for references, but
would not be correct for rename locations, which should instead map to
the string node fo the alias.

As an initial approach to address the aforementioned issues with rename
locations, we check that all the rename location nodes have the same text. If
_any_ node has text that differs from the request, we do not return any
rename locations. This works as a way to prevent renames that could
break the the program by missing some required nodes in the rename action, but
allowing other nodes to be renamed.

PR Close #40140

packages/language-service/ivy/language_service.ts

@@ -18,7 +18,7 @@ import {CompilerFactory} from './compiler_factory';
 import {CompletionBuilder, CompletionNodeContext} from './completions';
 import {DefinitionBuilder} from './definitions';
 import {QuickInfoBuilder} from './quick_info';
-import {ReferenceBuilder} from './references';
+import {ReferencesAndRenameBuilder} from './references';
 import {getTargetAtPosition, TargetContext, TargetNodeKind} from './template_target';
 import {getTemplateInfoAtPosition, isTypeScriptFile} from './utils';
 
@@ -107,8 +107,16 @@ export class LanguageService {
 
   getReferencesAtPosition(fileName: string, position: number): ts.ReferenceEntry[]|undefined {
     const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName);
-    const results =
+    const results = new ReferencesAndRenameBuilder(this.strategy, this.tsLS, compiler)
-        new ReferenceBuilder(this.strategy, this.tsLS, compiler).get(fileName, position);
+                        .getReferencesAtPosition(fileName, position);
+    this.compilerFactory.registerLastKnownProgram();
+    return results;
+  }
+
+  findRenameLocations(fileName: string, position: number): readonly ts.RenameLocation[]|undefined {
+    const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName);
+    const results = new ReferencesAndRenameBuilder(this.strategy, this.tsLS, compiler)
+                        .findRenameLocations(fileName, position);
     this.compilerFactory.registerLastKnownProgram();
     return results;
   }

packages/language-service/ivy/references.ts

@@ -5,52 +5,213 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-import {TmplAstBoundAttribute, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
+import {AST, BindingPipe, LiteralPrimitive, MethodCall, PropertyRead, PropertyWrite, SafeMethodCall, SafePropertyRead, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstNode, TmplAstReference, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
 import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
 import {absoluteFrom, absoluteFromSourceFile, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
-import {DirectiveSymbol, SymbolKind, TemplateTypeChecker, TypeCheckingProgramStrategy} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
+import {DirectiveSymbol, ShimLocation, SymbolKind, TemplateTypeChecker, TypeCheckingProgramStrategy} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
 import {ExpressionIdentifier, hasExpressionIdentifier} from '@angular/compiler-cli/src/ngtsc/typecheck/src/comments';
 import * as ts from 'typescript';
 
 import {getTargetAtPosition, TargetNodeKind} from './template_target';
 import {findTightestNode} from './ts_utils';
-import {getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTemplateInfoAtPosition, isWithin, TemplateInfo, toTextSpan} from './utils';
+import {getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTemplateInfoAtPosition, isTemplateNode, isWithin, TemplateInfo, toTextSpan} from './utils';
 
-export class ReferenceBuilder {
+interface FilePosition {
+  fileName: string;
+  position: number;
+}
+
+function toFilePosition(shimLocation: ShimLocation): FilePosition {
+  return {fileName: shimLocation.shimPath, position: shimLocation.positionInShimFile};
+}
+
+enum RequestKind {
+  Template,
+  TypeScript,
+}
+
+interface TemplateRequest {
+  kind: RequestKind.Template;
+  requestNode: TmplAstNode|AST;
+  position: number;
+}
+
+interface TypeScriptRequest {
+  kind: RequestKind.TypeScript;
+  requestNode: ts.Node;
+}
+
+type RequestOrigin = TemplateRequest|TypeScriptRequest;
+
+interface TemplateLocationDetails {
+  /**
+   * A target node in a template.
+   */
+  templateTarget: TmplAstNode|AST;
+
+  /**
+   * TypeScript locations which the template node maps to. A given template node might map to
+   * several TS nodes. For example, a template node for an attribute might resolve to several
+   * directives or a directive and one of its inputs.
+   */
+  typescriptLocations: FilePosition[];
+}
+
+export class ReferencesAndRenameBuilder {
   private readonly ttc = this.compiler.getTemplateTypeChecker();
 
   constructor(
       private readonly strategy: TypeCheckingProgramStrategy,
       private readonly tsLS: ts.LanguageService, private readonly compiler: NgCompiler) {}
 
-  get(filePath: string, position: number): ts.ReferenceEntry[]|undefined {
+  findRenameLocations(filePath: string, position: number): readonly ts.RenameLocation[]|undefined {
     this.ttc.generateAllTypeCheckBlocks();
     const templateInfo = getTemplateInfoAtPosition(filePath, position, this.compiler);
-    return templateInfo !== undefined ?
+    // We could not get a template at position so we assume the request is came from outside the
-        this.getReferencesAtTemplatePosition(templateInfo, position) :
+    // template.
-        this.getReferencesAtTypescriptPosition(filePath, position);
+    if (templateInfo === undefined) {
+      const requestNode = this.getTsNodeAtPosition(filePath, position);
+      if (requestNode === null) {
+        return undefined;
+      }
+      const requestOrigin: TypeScriptRequest = {kind: RequestKind.TypeScript, requestNode};
+      return this.findRenameLocationsAtTypescriptPosition(filePath, position, requestOrigin);
+    }
+
+    return this.findRenameLocationsAtTemplatePosition(templateInfo, position);
   }
 
-  private getReferencesAtTemplatePosition({template, component}: TemplateInfo, position: number):
+  private findRenameLocationsAtTemplatePosition(templateInfo: TemplateInfo, position: number):
+      readonly ts.RenameLocation[]|undefined {
+    const allTargetDetails = this.getTargetDetailsAtTemplatePosition(templateInfo, position);
+    if (allTargetDetails === null) {
+      return undefined;
+    }
+
+    const allRenameLocations: ts.RenameLocation[] = [];
+    for (const targetDetails of allTargetDetails) {
+      const requestOrigin: TemplateRequest = {
+        kind: RequestKind.Template,
+        requestNode: targetDetails.templateTarget,
+        position,
+      };
+
+      for (const location of targetDetails.typescriptLocations) {
+        const locations = this.findRenameLocationsAtTypescriptPosition(
+            location.fileName, location.position, requestOrigin);
+        // If we couldn't find rename locations for _any_ result, we should not allow renaming to
+        // proceed instead of having a partially complete rename.
+        if (locations === undefined) {
+          return undefined;
+        }
+        allRenameLocations.push(...locations);
+      }
+    }
+    return allRenameLocations.length > 0 ? allRenameLocations : undefined;
+  }
+
+  private getTsNodeAtPosition(filePath: string, position: number): ts.Node|null {
+    const sf = this.strategy.getProgram().getSourceFile(filePath);
+    if (!sf) {
+      return null;
+    }
+    return findTightestNode(sf, position) ?? null;
+  }
+
+  findRenameLocationsAtTypescriptPosition(
+      filePath: string, position: number,
+      requestOrigin: RequestOrigin): readonly ts.RenameLocation[]|undefined {
+    let originalNodeText: string;
+    if (requestOrigin.kind === RequestKind.TypeScript) {
+      originalNodeText = requestOrigin.requestNode.getText();
+    } else {
+      const templateNodeText =
+          getTemplateNodeRenameTextAtPosition(requestOrigin.requestNode, requestOrigin.position);
+      if (templateNodeText === null) {
+        return undefined;
+      }
+      originalNodeText = templateNodeText;
+    }
+
+    const locations = this.tsLS.findRenameLocations(
+        filePath, position, /*findInStrings*/ false, /*findInComments*/ false);
+    if (locations === undefined) {
+      return undefined;
+    }
+
+    const entries: ts.RenameLocation[] = [];
+    for (const location of locations) {
+      // TODO(atscott): Determine if a file is a shim file in a more robust way and make the API
+      // available in an appropriate location.
+      if (this.ttc.isTrackedTypeCheckFile(absoluteFrom(location.fileName))) {
+        const entry = this.convertToTemplateDocumentSpan(location, this.ttc, originalNodeText);
+        // There is no template node whose text matches the original rename request. Bail on
+        // renaming completely rather than providing incomplete results.
+        if (entry === null) {
+          return undefined;
+        }
+        entries.push(entry);
+      } else {
+        // Ensure we only allow renaming a TS result with matching text
+        const refNode = this.getTsNodeAtPosition(location.fileName, location.textSpan.start);
+        if (refNode === null || refNode.getText() !== originalNodeText) {
+          return undefined;
+        }
+        entries.push(location);
+      }
+    }
+    return entries;
+  }
+
+  getReferencesAtPosition(filePath: string, position: number): ts.ReferenceEntry[]|undefined {
+    this.ttc.generateAllTypeCheckBlocks();
+    const templateInfo = getTemplateInfoAtPosition(filePath, position, this.compiler);
+    if (templateInfo === undefined) {
+      return this.getReferencesAtTypescriptPosition(filePath, position);
+    }
+    return this.getReferencesAtTemplatePosition(templateInfo, position);
+  }
+
+  private getReferencesAtTemplatePosition(templateInfo: TemplateInfo, position: number):
       ts.ReferenceEntry[]|undefined {
+    const allTargetDetails = this.getTargetDetailsAtTemplatePosition(templateInfo, position);
+    if (allTargetDetails === null) {
+      return undefined;
+    }
+    const allReferences: ts.ReferenceEntry[] = [];
+    for (const targetDetails of allTargetDetails) {
+      for (const location of targetDetails.typescriptLocations) {
+        const refs = this.getReferencesAtTypescriptPosition(location.fileName, location.position);
+        if (refs !== undefined) {
+          allReferences.push(...refs);
+        }
+      }
+    }
+    return allReferences.length > 0 ? allReferences : undefined;
+  }
+
+  private getTargetDetailsAtTemplatePosition({template, component}: TemplateInfo, position: number):
+      TemplateLocationDetails[]|null {
     // Find the AST node in the template at the position.
     const positionDetails = getTargetAtPosition(template, position);
     if (positionDetails === null) {
-      return undefined;
+      return null;
     }
 
     const nodes = positionDetails.context.kind === TargetNodeKind.TwoWayBindingContext ?
         positionDetails.context.nodes :
         [positionDetails.context.node];
 
-    const references: ts.ReferenceEntry[] = [];
+    const details: TemplateLocationDetails[] = [];
+
     for (const node of nodes) {
       // Get the information about the TCB at the template position.
       const symbol = this.ttc.getSymbolOfNode(node, component);
+      const templateTarget = node;
+
       if (symbol === null) {
         continue;
       }
-
       switch (symbol.kind) {
         case SymbolKind.Directive:
         case SymbolKind.Template:
@@ -59,7 +220,8 @@ export class ReferenceBuilder {
           break;
         case SymbolKind.Element: {
           const matches = getDirectiveMatchesForElementTag(symbol.templateNode, symbol.directives);
-          references.push(...this.getReferencesForDirectives(matches) ?? []);
+          details.push(
+              {typescriptLocations: this.getPositionsForDirectives(matches), templateTarget});
           break;
         }
         case SymbolKind.DomBinding: {
@@ -67,69 +229,64 @@ export class ReferenceBuilder {
           // have a shim location. This means we can't match dom bindings to their lib.dom
           // reference, but we can still see if they match to a directive.
           if (!(node instanceof TmplAstTextAttribute) && !(node instanceof TmplAstBoundAttribute)) {
-            break;
+            return null;
           }
           const directives = getDirectiveMatchesForAttribute(
               node.name, symbol.host.templateNode, symbol.host.directives);
-          references.push(...this.getReferencesForDirectives(directives) ?? []);
+          details.push(
+              {typescriptLocations: this.getPositionsForDirectives(directives), templateTarget});
           break;
         }
         case SymbolKind.Reference: {
-          const {shimPath, positionInShimFile} = symbol.referenceVarLocation;
+          details.push(
-          references.push(
+              {typescriptLocations: [toFilePosition(symbol.referenceVarLocation)], templateTarget});
-              ...this.getReferencesAtTypescriptPosition(shimPath, positionInShimFile) ?? []);
           break;
         }
         case SymbolKind.Variable: {
-          const {positionInShimFile: initializerPosition, shimPath} = symbol.initializerLocation;
+          if ((templateTarget instanceof TmplAstVariable)) {
-          const localVarPosition = symbol.localVarLocation.positionInShimFile;
+            if (templateTarget.valueSpan !== undefined &&
-
+                isWithin(position, templateTarget.valueSpan)) {
-          if ((node instanceof TmplAstVariable)) {
-            if (node.valueSpan !== undefined && isWithin(position, node.valueSpan)) {
               // In the valueSpan of the variable, we want to get the reference of the initializer.
-              references.push(
+              details.push({
-                  ...this.getReferencesAtTypescriptPosition(shimPath, initializerPosition) ?? []);
+                typescriptLocations: [toFilePosition(symbol.initializerLocation)],
-            } else if (isWithin(position, node.keySpan)) {
+                templateTarget,
+              });
+            } else if (isWithin(position, templateTarget.keySpan)) {
               // In the keySpan of the variable, we want to get the reference of the local variable.
-              references.push(
+              details.push(
-                  ...this.getReferencesAtTypescriptPosition(shimPath, localVarPosition) ?? []);
+                  {typescriptLocations: [toFilePosition(symbol.localVarLocation)], templateTarget});
             }
           } else {
-            // If the templateNode is not the `TmplAstVariable`, it must be a usage of the variable
+            // If the templateNode is not the `TmplAstVariable`, it must be a usage of the
-            // somewhere in the template.
+            // variable somewhere in the template.
-            references.push(
+            details.push(
-                ...this.getReferencesAtTypescriptPosition(shimPath, localVarPosition) ?? []);
+                {typescriptLocations: [toFilePosition(symbol.localVarLocation)], templateTarget});
           }
-
           break;
         }
         case SymbolKind.Input:
         case SymbolKind.Output: {
-          // TODO(atscott): Determine how to handle when the binding maps to several inputs/outputs
+          details.push({
-          const {shimPath, positionInShimFile} = symbol.bindings[0].shimLocation;
+            typescriptLocations:
-          references.push(
+                symbol.bindings.map(binding => toFilePosition(binding.shimLocation)),
-              ...this.getReferencesAtTypescriptPosition(shimPath, positionInShimFile) ?? []);
+            templateTarget,
+          });
           break;
         }
         case SymbolKind.Pipe:
         case SymbolKind.Expression: {
-          const {shimPath, positionInShimFile} = symbol.shimLocation;
+          details.push(
-          references.push(
+              {typescriptLocations: [toFilePosition(symbol.shimLocation)], templateTarget});
-              ...this.getReferencesAtTypescriptPosition(shimPath, positionInShimFile) ?? []);
           break;
         }
       }
     }
-    if (references.length === 0) {
-      return undefined;
-    }
 
-    return references;
+    return details.length > 0 ? details : null;
   }
 
-  private getReferencesForDirectives(directives: Set<DirectiveSymbol>):
+  private getPositionsForDirectives(directives: Set<DirectiveSymbol>): FilePosition[] {
-      ts.ReferenceEntry[]|undefined {
+    const allDirectives: FilePosition[] = [];
-    const allDirectiveRefs: ts.ReferenceEntry[] = [];
     for (const dir of directives.values()) {
       const dirClass = dir.tsSymbol.valueDeclaration;
       if (dirClass === undefined || !ts.isClassDeclaration(dirClass) ||
@@ -137,15 +294,12 @@ export class ReferenceBuilder {
         continue;
       }
 
-      const dirFile = dirClass.getSourceFile().fileName;
+      const {fileName} = dirClass.getSourceFile();
-      const dirPosition = dirClass.name.getStart();
+      const position = dirClass.name.getStart();
-      const directiveRefs = this.getReferencesAtTypescriptPosition(dirFile, dirPosition);
+      allDirectives.push({fileName, position});
-      if (directiveRefs !== undefined) {
-        allDirectiveRefs.push(...directiveRefs);
-      }
     }
 
-    return allDirectiveRefs.length > 0 ? allDirectiveRefs : undefined;
+    return allDirectives;
   }
 
   private getReferencesAtTypescriptPosition(fileName: string, position: number):
@@ -158,7 +312,7 @@ export class ReferenceBuilder {
     const entries: ts.ReferenceEntry[] = [];
     for (const ref of refs) {
       if (this.ttc.isTrackedTypeCheckFile(absoluteFrom(ref.fileName))) {
-        const entry = this.convertToTemplateReferenceEntry(ref, this.ttc);
+        const entry = this.convertToTemplateDocumentSpan(ref, this.ttc);
         if (entry !== null) {
           entries.push(entry);
         }
@@ -169,27 +323,27 @@ export class ReferenceBuilder {
     return entries;
   }
 
-  private convertToTemplateReferenceEntry(
+  private convertToTemplateDocumentSpan<T extends ts.DocumentSpan>(
-      shimReferenceEntry: ts.ReferenceEntry,
+      shimDocumentSpan: T, templateTypeChecker: TemplateTypeChecker, requiredNodeText?: string): T
-      templateTypeChecker: TemplateTypeChecker): ts.ReferenceEntry|null {
+      |null {
-    const sf = this.strategy.getProgram().getSourceFile(shimReferenceEntry.fileName);
+    const sf = this.strategy.getProgram().getSourceFile(shimDocumentSpan.fileName);
     if (sf === undefined) {
       return null;
     }
-    const tcbNode = findTightestNode(sf, shimReferenceEntry.textSpan.start);
+    const tcbNode = findTightestNode(sf, shimDocumentSpan.textSpan.start);
     if (tcbNode === undefined ||
         hasExpressionIdentifier(sf, tcbNode, ExpressionIdentifier.EVENT_PARAMETER)) {
-      // If the reference result is the $event parameter in the subscribe/addEventListener function
+      // If the reference result is the $event parameter in the subscribe/addEventListener
-      // in the TCB, we want to filter this result out of the references. We really only want to
+      // function in the TCB, we want to filter this result out of the references. We really only
-      // return references to the parameter in the template itself.
+      // want to return references to the parameter in the template itself.
       return null;
     }
-
+    // TODO(atscott): Determine how to consistently resolve paths. i.e. with the project
-    // TODO(atscott): Determine how to consistently resolve paths. i.e. with the project serverHost
+    // serverHost or LSParseConfigHost in the adapter. We should have a better defined way to
-    // or LSParseConfigHost in the adapter. We should have a better defined way to normalize paths.
+    // normalize paths.
     const mapping = templateTypeChecker.getTemplateMappingAtShimLocation({
-      shimPath: absoluteFrom(shimReferenceEntry.fileName),
+      shimPath: absoluteFrom(shimDocumentSpan.fileName),
-      positionInShimFile: shimReferenceEntry.textSpan.start,
+      positionInShimFile: shimDocumentSpan.textSpan.start,
     });
     if (mapping === null) {
       return null;
@@ -202,16 +356,46 @@ export class ReferenceBuilder {
     } else if (templateSourceMapping.type === 'external') {
       templateUrl = absoluteFrom(templateSourceMapping.templateUrl);
     } else {
-      // This includes indirect mappings, which are difficult to map directly to the code location.
+      // This includes indirect mappings, which are difficult to map directly to the code
-      // Diagnostics similarly return a synthetic template string for this case rather than a real
+      // location. Diagnostics similarly return a synthetic template string for this case rather
-      // location.
+      // than a real location.
+      return null;
+    }
+
+    if (requiredNodeText !== undefined && span.toString() !== requiredNodeText) {
       return null;
     }
 
     return {
-      ...shimReferenceEntry,
+      ...shimDocumentSpan,
       fileName: templateUrl,
       textSpan: toTextSpan(span),
     };
   }
 }
+
+function getTemplateNodeRenameTextAtPosition(node: TmplAstNode|AST, position: number): string|null {
+  if (node instanceof TmplAstBoundAttribute || node instanceof TmplAstTextAttribute ||
+      node instanceof TmplAstBoundEvent) {
+    return node.name;
+  } else if (node instanceof TmplAstVariable || node instanceof TmplAstReference) {
+    if (isWithin(position, node.keySpan)) {
+      return node.keySpan.toString();
+    } else if (node.valueSpan && isWithin(position, node.valueSpan)) {
+      return node.valueSpan.toString();
+    }
+  }
+
+  if (node instanceof BindingPipe) {
+    // TODO(atscott): Add support for renaming pipes
+    return null;
+  }
+  if (node instanceof PropertyRead || node instanceof MethodCall || node instanceof PropertyWrite ||
+      node instanceof SafePropertyRead || node instanceof SafeMethodCall) {
+    return node.name;
+  } else if (node instanceof LiteralPrimitive) {
+    return node.value;
+  }
+
+  return null;
+}

packages/language-service/ivy/test/definitions_spec.ts

@@ -10,7 +10,7 @@ import {absoluteFrom, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file
 import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
 
 import {extractCursorInfo, LanguageServiceTestEnvironment} from './env';
-import {assertFileNames, createModuleWithDeclarations, humanizeDefinitionInfo} from './test_utils';
+import {assertFileNames, createModuleWithDeclarations, humanizeDocumentSpanLike} from './test_utils';
 
 describe('definitions', () => {
   it('returns the pipe class as definition when checkTypeOfPipes is false', () => {
@@ -27,8 +27,8 @@ describe('definitions', () => {
         export class AppCmp {}
       `,
     };
-    const env = createModuleWithDeclarations([appFile], [templateFile]);
     // checkTypeOfPipes is set to false when strict templates is false
+    const env = createModuleWithDeclarations([appFile], [templateFile], {strictTemplates: false});
     const {textSpan, definitions} =
         getDefinitionsAndAssertBoundSpan(env, absoluteFrom('/app.html'), cursor);
     expect(text.substr(textSpan.start, textSpan.length)).toEqual('date');
@@ -143,6 +143,6 @@ describe('definitions', () => {
     const definitionAndBoundSpan = env.ngLS.getDefinitionAndBoundSpan(fileName, cursor);
     const {textSpan, definitions} = definitionAndBoundSpan!;
     expect(definitions).toBeTruthy();
-    return {textSpan, definitions: definitions!.map(d => humanizeDefinitionInfo(d, env.host))};
+    return {textSpan, definitions: definitions!.map(d => humanizeDocumentSpanLike(d, env))};
   }
 });

packages/language-service/ivy/test/test_utils.ts

@@ -5,12 +5,11 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-
 import {absoluteFrom as _} from '@angular/compiler-cli/src/ngtsc/file_system';
 import {TestFile} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
-import {LanguageServiceTestEnvironment} from '@angular/language-service/ivy/test/env';
+import {LanguageServiceTestEnvironment, TestableOptions} from '@angular/language-service/ivy/test/env';
+import * as ts from 'typescript/lib/tsserverlibrary';
 
-import {MockServerHost} from './mock_host';
 
 export function getText(contents: string, textSpan: ts.TextSpan) {
   return contents.substr(textSpan.start, textSpan.length);
@@ -29,8 +28,8 @@ function getFirstClassDeclaration(declaration: string) {
 }
 
 export function createModuleWithDeclarations(
-    filesWithClassDeclarations: TestFile[],
+    filesWithClassDeclarations: TestFile[], externalResourceFiles: TestFile[] = [],
-    externalResourceFiles: TestFile[] = []): LanguageServiceTestEnvironment {
+    options: TestableOptions = {}): LanguageServiceTestEnvironment {
   const externalClasses =
       filesWithClassDeclarations.map(file => getFirstClassDeclaration(file.contents));
   const externalImports = filesWithClassDeclarations.map(file => {
@@ -51,30 +50,31 @@ export function createModuleWithDeclarations(
       `;
   const moduleFile = {name: _('/app-module.ts'), contents, isRoot: true};
   return LanguageServiceTestEnvironment.setup(
-      [moduleFile, ...filesWithClassDeclarations, ...externalResourceFiles]);
+      [moduleFile, ...filesWithClassDeclarations, ...externalResourceFiles], options);
 }
 
-export interface HumanizedDefinitionInfo {
+export function humanizeDocumentSpanLike<T extends ts.DocumentSpan>(
-  fileName: string;
+    item: T, env: LanguageServiceTestEnvironment, overrides: Map<string, string> = new Map()): T&
-  textSpan: string;
+    Stringy<ts.DocumentSpan> {
-  contextSpan: string|undefined;
+  const fileContents = (overrides.has(item.fileName) ? overrides.get(item.fileName) :
-}
+                                                       env.host.readFile(item.fileName)) ??
-
-export function humanizeDefinitionInfo(
-    def: ts.DefinitionInfo, host: MockServerHost,
-    overrides: Map<string, string> = new Map()): HumanizedDefinitionInfo {
-  const contents = (overrides.get(def.fileName) !== undefined ? overrides.get(def.fileName) :
-                                                                host.readFile(def.fileName)) ??
       '';
-
+  if (!fileContents) {
+    throw new Error('Could not read file ${entry.fileName}');
+  }
   return {
-    fileName: def.fileName,
+    ...item,
-    textSpan: contents.substr(def.textSpan.start, def.textSpan.start + def.textSpan.length),
+    textSpan: getText(fileContents, item.textSpan),
-    contextSpan: def.contextSpan ?
+    contextSpan: item.contextSpan ? getText(fileContents, item.contextSpan) : undefined,
-        contents.substr(def.contextSpan.start, def.contextSpan.start + def.contextSpan.length) :
+    originalTextSpan: item.originalTextSpan ? getText(fileContents, item.originalTextSpan) :
-        undefined,
+                                              undefined,
+    originalContextSpan:
+        item.originalContextSpan ? getText(fileContents, item.originalContextSpan) : undefined,
   };
 }
+type Stringy<T> = {
+  [P in keyof T]: string;
+};
 
 export function assertFileNames(refs: Array<{fileName: string}>, expectedFileNames: string[]) {
   const actualPaths = refs.map(r => r.fileName);
@@ -85,4 +85,4 @@ export function assertFileNames(refs: Array<{fileName: string}>, expectedFileNam
 export function assertTextSpans(items: Array<{textSpan: string}>, expectedTextSpans: string[]) {
   const actualSpans = items.map(item => item.textSpan);
   expect(new Set(actualSpans)).toEqual(new Set(expectedTextSpans));
-}
+}

packages/language-service/ivy/test/type_definitions_spec.ts

@@ -10,7 +10,7 @@ import {absoluteFrom} from '@angular/compiler-cli/src/ngtsc/file_system';
 import {initMockFileSystem, TestFile} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
 
 import {LanguageServiceTestEnvironment} from './env';
-import {HumanizedDefinitionInfo, humanizeDefinitionInfo} from './test_utils';
+import {humanizeDocumentSpanLike} from './test_utils';
 
 describe('type definitions', () => {
   let env: LanguageServiceTestEnvironment;
@@ -48,8 +48,7 @@ describe('type definitions', () => {
     expect(def.contextSpan).toContain('DatePipe');
   });
 
-  function getTypeDefinitionsAndAssertBoundSpan({templateOverride}: {templateOverride: string}):
+  function getTypeDefinitionsAndAssertBoundSpan({templateOverride}: {templateOverride: string}) {
-      HumanizedDefinitionInfo[] {
     const {cursor, text} =
         env.overrideTemplateWithCursor(absoluteFrom('/app.ts'), 'AppCmp', templateOverride);
     env.expectNoSourceDiagnostics();
@@ -58,6 +57,6 @@ describe('type definitions', () => {
     expect(defs).toBeTruthy();
     const overrides = new Map<string, string>();
     overrides.set(absoluteFrom('/app.html'), text);
-    return defs!.map(d => humanizeDefinitionInfo(d, env.host, overrides));
+    return defs!.map(d => humanizeDocumentSpanLike(d, env, overrides));
   }
 });

packages/language-service/ivy/ts_plugin.ts

@@ -64,8 +64,11 @@ export function create(info: ts.server.PluginCreateInfo): ts.LanguageService {
   function findRenameLocations(
       fileName: string, position: number, findInStrings: boolean, findInComments: boolean,
       providePrefixAndSuffixTextForRename?: boolean): readonly ts.RenameLocation[]|undefined {
-    // TODO(atscott): implement
+    // Most operations combine results from all extensions. However, rename locations are exclusive
-    return undefined;
+    // (results from only one extension are used) so our rename locations are a superset of the TS
+    // rename locations. As a result, we do not check the `angularOnly` flag here because we always
+    // want to include results at TS locations as well as locations in templates.
+    return ngLS.findRenameLocations(fileName, position);
   }
 
   function getCompletionsAtPosition(