fix(language-service): fix go to definition for template variables and references (#40455)

The current "go to definition" is broken for template variables and
references when a template is overridden. This is because we get the
file url from the source span, which uses the overridden name
'override.html'. Instead, we can retrieve the template file from the
compiler in the same manner that is done for references.

Another way to fix this would have been to use the real template file path when
overriding a template, but this was the more straightforward fix since
the strategy was already used in find references and rename locations.

fixes https://github.com/angular/vscode-ng-language-service/issues/1054

PR Close #40455

packages/language-service/ivy/definitions.ts

@@ -14,7 +14,7 @@ import * as ts from 'typescript';
 
 import {getTargetAtPosition, TargetNodeKind} from './template_target';
 import {findTightestNode, getParentClassDeclaration} from './ts_utils';
-import {flatMap, getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTemplateInfoAtPosition, getTextSpanOfNode, isDollarEvent, isTypeScriptFile, TemplateInfo, toTextSpan} from './utils';
+import {flatMap, getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTemplateInfoAtPosition, getTemplateLocationFromShimLocation, getTextSpanOfNode, isDollarEvent, isTypeScriptFile, TemplateInfo, toTextSpan} from './utils';
 
 interface DefinitionMeta {
   node: AST|TmplAstNode;
@@ -100,6 +100,12 @@ export class DefinitionBuilder {
       case SymbolKind.Reference: {
         const definitions: ts.DefinitionInfo[] = [];
         if (symbol.declaration !== node) {
+          const shimLocation = symbol.kind === SymbolKind.Variable ? symbol.localVarLocation :
+                                                                     symbol.referenceVarLocation;
+          const mapping = getTemplateLocationFromShimLocation(
+              this.compiler.getTemplateTypeChecker(), shimLocation.shimPath,
+              shimLocation.positionInShimFile);
+          if (mapping !== null) {
             definitions.push({
               name: symbol.declaration.name,
               containerName: '',
@@ -107,9 +113,10 @@ export class DefinitionBuilder {
               kind: ts.ScriptElementKind.variableElement,
               textSpan: getTextSpanOfNode(symbol.declaration),
               contextSpan: toTextSpan(symbol.declaration.sourceSpan),
-            fileName: symbol.declaration.sourceSpan.start.file.url,
+              fileName: mapping.templateUrl,
             });
           }
+        }
         if (symbol.kind === SymbolKind.Variable) {
           definitions.push(
               ...this.getDefinitionsForSymbols({shimLocation: symbol.initializerLocation}));

packages/language-service/ivy/references.ts

@@ -14,7 +14,7 @@ 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, getTemplateLocationFromShimLocation, isWithin, TemplateInfo, toTextSpan} from './utils';
 
 interface FilePosition {
   fileName: string;
@@ -376,27 +376,14 @@ export class ReferencesAndRenameBuilder {
     // TODO(atscott): Determine how to consistently resolve paths. i.e. with the project
     // serverHost or LSParseConfigHost in the adapter. We should have a better defined way to
     // normalize paths.
-    const mapping = templateTypeChecker.getTemplateMappingAtShimLocation({
-      shimPath: absoluteFrom(shimDocumentSpan.fileName),
-      positionInShimFile: shimDocumentSpan.textSpan.start,
-    });
+    const mapping = getTemplateLocationFromShimLocation(
+        templateTypeChecker, absoluteFrom(shimDocumentSpan.fileName),
+        shimDocumentSpan.textSpan.start);
     if (mapping === null) {
       return null;
     }
-    const {templateSourceMapping, span} = mapping;
-
-    let templateUrl: AbsoluteFsPath;
-    if (templateSourceMapping.type === 'direct') {
-      templateUrl = absoluteFromSourceFile(templateSourceMapping.node.getSourceFile());
-    } else if (templateSourceMapping.type === 'external') {
-      templateUrl = absoluteFrom(templateSourceMapping.templateUrl);
-    } else {
-      // This includes indirect mappings, which are difficult to map directly to the code
-      // location. Diagnostics similarly return a synthetic template string for this case rather
-      // than a real location.
-      return null;
-    }
 
+    const {span, templateUrl} = mapping;
     if (requiredNodeText !== undefined && span.toString() !== requiredNodeText) {
       return null;
     }

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

@@ -13,6 +13,28 @@ import {extractCursorInfo, LanguageServiceTestEnvironment} from './env';
 import {assertFileNames, createModuleWithDeclarations, humanizeDocumentSpanLike} from './test_utils';
 
 describe('definitions', () => {
+  it('gets definition for template reference in overridden template', () => {
+    initMockFileSystem('Native');
+    const templateFile = {contents: '', name: absoluteFrom('/app.html')};
+    const appFile = {
+      name: absoluteFrom('/app.ts'),
+      contents: `
+        import {Component} from '@angular/core';
+
+        @Component({templateUrl: '/app.html'})
+        export class AppCmp {}
+      `,
+    };
+
+    const env = createModuleWithDeclarations([appFile], [templateFile]);
+    const {cursor} = env.overrideTemplateWithCursor(
+        absoluteFrom('/app.ts'), 'AppCmp', '<input #myInput /> {{myIn¦put.value}}');
+    env.expectNoSourceDiagnostics();
+    const {definitions} = env.ngLS.getDefinitionAndBoundSpan(absoluteFrom('/app.html'), cursor)!;
+    expect(definitions![0].name).toEqual('myInput');
+    assertFileNames(Array.from(definitions!), ['app.html']);
+  });
+
   it('returns the pipe class as definition when checkTypeOfPipes is false', () => {
     initMockFileSystem('Native');
     const {cursor, text} = extractCursorInfo('{{"1/1/2020" | dat¦e}}');

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

@@ -60,7 +60,7 @@ export function humanizeDocumentSpanLike<T extends ts.DocumentSpan>(
                                                        env.host.readFile(item.fileName)) ??
       '';
   if (!fileContents) {
-    throw new Error('Could not read file ${entry.fileName}');
+    throw new Error(`Could not read file ${item.fileName}`);
   }
   return {
     ...item,

packages/language-service/ivy/utils.ts

@@ -7,9 +7,10 @@
  */
 import {AbsoluteSourceSpan, CssSelector, ParseSourceSpan, SelectorMatcher, TmplAstBoundEvent} 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 {isExternalResource} from '@angular/compiler-cli/src/ngtsc/metadata';
 import {DeclarationNode} from '@angular/compiler-cli/src/ngtsc/reflection';
-import {DirectiveSymbol} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
+import {DirectiveSymbol, TemplateTypeChecker} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
 import * as e from '@angular/compiler/src/expression_parser/ast';  // e for expression AST
 import * as t from '@angular/compiler/src/render3/r3_ast';         // t for template AST
 import * as ts from 'typescript';
@@ -346,3 +347,31 @@ export function isWithin(position: number, span: AbsoluteSourceSpan|ParseSourceS
   // like ¦start and end¦ where ¦ is the cursor.
   return start <= position && position <= end;
 }
+
+/**
+ * For a given location in a shim file, retrieves the corresponding file url for the template and
+ * the span in the template.
+ */
+export function getTemplateLocationFromShimLocation(
+    templateTypeChecker: TemplateTypeChecker, shimPath: AbsoluteFsPath,
+    positionInShimFile: number): {templateUrl: AbsoluteFsPath, span: ParseSourceSpan}|null {
+  const mapping =
+      templateTypeChecker.getTemplateMappingAtShimLocation({shimPath, positionInShimFile});
+  if (mapping === null) {
+    return null;
+  }
+  const {templateSourceMapping, span} = mapping;
+
+  let templateUrl: AbsoluteFsPath;
+  if (templateSourceMapping.type === 'direct') {
+    templateUrl = absoluteFromSourceFile(templateSourceMapping.node.getSourceFile());
+  } else if (templateSourceMapping.type === 'external') {
+    templateUrl = absoluteFrom(templateSourceMapping.templateUrl);
+  } else {
+    // This includes indirect mappings, which are difficult to map directly to the code
+    // location. Diagnostics similarly return a synthetic template string for this case rather
+    // than a real location.
+    return null;
+  }
+  return {templateUrl, span};
+}