feat(language-service): support multiple symbol definitions (#34782)

In Angular, symbol can have multiple definitions (e.g. a two-way
binding). This commit adds support for for multiple definitions for a
queried location in a template.

PR Close #34782

packages/language-service/src/definitions.ts

@@ -9,9 +9,9 @@
 import * as path from 'path';
 import * as ts from 'typescript'; // used as value and is provided at runtime
 import {AstResult} from './common';
-import {locateSymbol} from './locate_symbol';
+import {locateSymbols} from './locate_symbol';
 import {getPropertyAssignmentFromValue, isClassDecoratorProperty} from './template';
-import {Span, TemplateSource} from './types';
+import {Span} from './types';
 import {findTightestNode} from './utils';
 
 /**
@@ -34,30 +34,38 @@ function ngSpanToTsTextSpan(span: Span): ts.TextSpan {
  */
 export function getDefinitionAndBoundSpan(
     info: AstResult, position: number): ts.DefinitionInfoAndBoundSpan|undefined {
-  const symbolInfo = locateSymbol(info, position);
+  const symbols = locateSymbols(info, position);
-  if (!symbolInfo) {
+  if (!symbols.length) {
     return;
   }
-  const textSpan = ngSpanToTsTextSpan(symbolInfo.span);
+
-  const {symbol} = symbolInfo;
+  const textSpan = ngSpanToTsTextSpan(symbols[0].span);
-  const {container, definition: locations} = symbol;
+  const definitions: ts.DefinitionInfo[] = [];
-  if (!locations || !locations.length) {
+  for (const symbolInfo of symbols) {
+    const {symbol} = symbolInfo;
+
     // symbol.definition is really the locations of the symbol. There could be
     // more than one. No meaningful info could be provided without any location.
-    return {textSpan};
+    const {kind, name, container, definition: locations} = symbol;
+    if (!locations || !locations.length) {
+      continue;
+    }
+
+    const containerKind =
+        container ? container.kind as ts.ScriptElementKind : ts.ScriptElementKind.unknown;
+    const containerName = container ? container.name : '';
+    definitions.push(...locations.map((location) => {
+      return {
+        kind: kind as ts.ScriptElementKind,
+        name,
+        containerKind,
+        containerName,
+        textSpan: ngSpanToTsTextSpan(location.span),
+        fileName: location.fileName,
+      };
+    }));
   }
-  const containerKind = container ? container.kind : ts.ScriptElementKind.unknown;
+
-  const containerName = container ? container.name : '';
-  const definitions = locations.map((location) => {
-    return {
-      kind: symbol.kind as ts.ScriptElementKind,
-      name: symbol.name,
-      containerKind: containerKind as ts.ScriptElementKind,
-      containerName: containerName,
-      textSpan: ngSpanToTsTextSpan(location.span),
-      fileName: location.fileName,
-    };
-  });
   return {
       definitions, textSpan,
   };

packages/language-service/src/expressions.ts

@@ -148,8 +148,14 @@ export function getExpressionSymbol(
     },
     visitPropertyWrite(ast) {
       const receiverType = getType(ast.receiver);
+      const {start} = ast.span;
       symbol = receiverType && receiverType.members().get(ast.name);
-      span = ast.span;
+      // A PropertyWrite span includes both the LHS (name) and the RHS (value) of the write. In this
+      // visit, only the name is relevant.
+      //   prop=$event
+      //   ^^^^        name
+      //        ^^^^^^ value; visited separately as a nested AST
+      span = {start, end: start + ast.name.length};
     },
     visitQuote(ast) {},
     visitSafeMethodCall(ast) {

packages/language-service/src/hover.ts

@@ -10,7 +10,7 @@ import {CompileSummaryKind, StaticSymbol} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {AstResult} from './common';
-import {locateSymbol} from './locate_symbol';
+import {locateSymbols} from './locate_symbol';
 import * as ng from './types';
 import {TypeScriptServiceHost} from './typescript_host';
 import {findTightestNode} from './utils';
@@ -32,10 +32,11 @@ const SYMBOL_INTERFACE = ts.SymbolDisplayPartKind[ts.SymbolDisplayPartKind.inter
  */
 export function getHover(info: AstResult, position: number, host: Readonly<TypeScriptServiceHost>):
     ts.QuickInfo|undefined {
-  const symbolInfo = locateSymbol(info, position);
+  const symbolInfo = locateSymbols(info, position)[0];
   if (!symbolInfo) {
     return;
   }
+
   const {symbol, span, compileTypeSummary} = symbolInfo;
   const textSpan = {start: span.start, length: span.end - span.start};
 

packages/language-service/src/locate_symbol.ts

@@ -12,7 +12,7 @@ import {AstResult} from './common';
 import {getExpressionScope} from './expression_diagnostics';
 import {getExpressionSymbol} from './expressions';
 import {Definition, DirectiveKind, Span, Symbol} from './types';
-import {diagnosticInfoFromTemplateInfo, findTemplateAstAt, getPathToNodeAtPosition, inSpan, offsetSpan, spanOf} from './utils';
+import {diagnosticInfoFromTemplateInfo, findTemplateAstAt, getPathToNodeAtPosition, inSpan, isNarrower, offsetSpan, spanOf} from './utils';
 
 export interface SymbolInfo {
   symbol: Symbol;
@@ -21,122 +21,144 @@ export interface SymbolInfo {
 }
 
 /**
- * Traverse the template AST and locate the Symbol at the specified `position`.
+ * Traverses a template AST and locates symbol(s) at a specified position.
- * @param info Ast and Template Source
+ * @param info template AST information set
- * @param position location to look for
+ * @param position location to locate symbols at
  */
-export function locateSymbol(info: AstResult, position: number): SymbolInfo|undefined {
+export function locateSymbols(info: AstResult, position: number): SymbolInfo[] {
   const templatePosition = position - info.template.span.start;
+  // TODO: update `findTemplateAstAt` to use absolute positions.
   const path = findTemplateAstAt(info.templateAst, templatePosition);
-  let compileTypeSummary: CompileTypeSummary|undefined = undefined;
+  if (!path.tail) return [];
-  if (path.tail) {
-    let symbol: Symbol|undefined = undefined;
-    let span: Span|undefined = undefined;
-    const attributeValueSymbol = (ast: AST, inEvent: boolean = false): boolean => {
-      const attribute = findAttribute(info, position);
-      if (attribute) {
-        if (inSpan(templatePosition, spanOf(attribute.valueSpan))) {
-          const dinfo = diagnosticInfoFromTemplateInfo(info);
-          const scope = getExpressionScope(dinfo, path);
-          if (attribute.valueSpan) {
-            const result = getExpressionSymbol(scope, ast, templatePosition, info.template.query);
-            if (result) {
-              symbol = result.symbol;
-              const expressionOffset = attribute.valueSpan.start.offset;
-              span = offsetSpan(result.span, expressionOffset);
-            }
-          }
-          return true;
-        }
-      }
-      return false;
-    };
-    path.tail.visit(
-        {
-          visitNgContent(ast) {},
-          visitEmbeddedTemplate(ast) {},
-          visitElement(ast) {
-            const component = ast.directives.find(d => d.directive.isComponent);
-            if (component) {
-              compileTypeSummary = component.directive;
-              symbol = info.template.query.getTypeSymbol(compileTypeSummary.type.reference);
-              symbol = symbol && new OverrideKindSymbol(symbol, DirectiveKind.COMPONENT);
-              span = spanOf(ast);
-            } else {
-              // Find a directive that matches the element name
-              const directive = ast.directives.find(
-                  d => d.directive.selector != null && d.directive.selector.indexOf(ast.name) >= 0);
-              if (directive) {
-                compileTypeSummary = directive.directive;
-                symbol = info.template.query.getTypeSymbol(compileTypeSummary.type.reference);
-                symbol = symbol && new OverrideKindSymbol(symbol, DirectiveKind.DIRECTIVE);
-                span = spanOf(ast);
-              }
-            }
-          },
-          visitReference(ast) {
-            symbol = ast.value && info.template.query.getTypeSymbol(tokenReference(ast.value));
-            span = spanOf(ast);
-          },
-          visitVariable(ast) {},
-          visitEvent(ast) {
-            if (!attributeValueSymbol(ast.handler, /* inEvent */ true)) {
-              symbol = findOutputBinding(info, path, ast);
-              symbol = symbol && new OverrideKindSymbol(symbol, DirectiveKind.EVENT);
-              span = spanOf(ast);
-            }
-          },
-          visitElementProperty(ast) { attributeValueSymbol(ast.value); },
-          visitAttr(ast) {
-            const element = path.head;
-            if (!element || !(element instanceof ElementAst)) return;
-            // Create a mapping of all directives applied to the element from their selectors.
-            const matcher = new SelectorMatcher<DirectiveAst>();
-            for (const dir of element.directives) {
-              if (!dir.directive.selector) continue;
-              matcher.addSelectables(CssSelector.parse(dir.directive.selector), dir);
-            }
 
-            // See if this attribute matches the selector of any directive on the element.
+  const narrowest = spanOf(path.tail);
-            const attributeSelector = `[${ast.name}=${ast.value}]`;
+  const toVisit: TemplateAst[] = [];
-            const parsedAttribute = CssSelector.parse(attributeSelector);
+  for (let node: TemplateAst|undefined = path.tail;
-            if (!parsedAttribute.length) return;
+       node && isNarrower(spanOf(node.sourceSpan), narrowest); node = path.parentOf(node)) {
-            matcher.match(parsedAttribute[0], (_, directive) => {
+    toVisit.push(node);
-              symbol = info.template.query.getTypeSymbol(directive.directive.type.reference);
+  }
-              symbol = symbol && new OverrideKindSymbol(symbol, DirectiveKind.DIRECTIVE);
+
-              span = spanOf(ast);
+  return toVisit.map(ast => locateSymbol(ast, path, info))
-            });
+      .filter((sym): sym is SymbolInfo => sym !== undefined);
-          },
+}
-          visitBoundText(ast) {
+
-            const expressionPosition = templatePosition - ast.sourceSpan.start.offset;
+/**
-            if (inSpan(expressionPosition, ast.value.span)) {
+ * Visits a template node and locates the symbol in that node at a path position.
-              const dinfo = diagnosticInfoFromTemplateInfo(info);
+ * @param ast template AST node to visit
-              const scope = getExpressionScope(dinfo, path);
+ * @param path non-empty set of narrowing AST nodes at a position
-              const result =
+ * @param info template AST information set
-                  getExpressionSymbol(scope, ast.value, templatePosition, info.template.query);
+ */
-              if (result) {
+function locateSymbol(ast: TemplateAst, path: TemplateAstPath, info: AstResult): SymbolInfo|
-                symbol = result.symbol;
+    undefined {
-                span = offsetSpan(result.span, ast.sourceSpan.start.offset);
+  const templatePosition = path.position;
-              }
+  const position = templatePosition + info.template.span.start;
-            }
+  let compileTypeSummary: CompileTypeSummary|undefined = undefined;
-          },
+  let symbol: Symbol|undefined;
-          visitText(ast) {},
+  let span: Span|undefined;
-          visitDirective(ast) {
+  const attributeValueSymbol = (ast: AST): boolean => {
-            compileTypeSummary = ast.directive;
+    const attribute = findAttribute(info, position);
+    if (attribute) {
+      if (inSpan(templatePosition, spanOf(attribute.valueSpan))) {
+        const dinfo = diagnosticInfoFromTemplateInfo(info);
+        const scope = getExpressionScope(dinfo, path);
+        if (attribute.valueSpan) {
+          const result = getExpressionSymbol(scope, ast, templatePosition, info.template.query);
+          if (result) {
+            symbol = result.symbol;
+            const expressionOffset = attribute.valueSpan.start.offset;
+            span = offsetSpan(result.span, expressionOffset);
+          }
+        }
+        return true;
+      }
+    }
+    return false;
+  };
+  ast.visit(
+      {
+        visitNgContent(ast) {},
+        visitEmbeddedTemplate(ast) {},
+        visitElement(ast) {
+          const component = ast.directives.find(d => d.directive.isComponent);
+          if (component) {
+            compileTypeSummary = component.directive;
             symbol = info.template.query.getTypeSymbol(compileTypeSummary.type.reference);
+            symbol = symbol && new OverrideKindSymbol(symbol, DirectiveKind.COMPONENT);
             span = spanOf(ast);
-          },
+          } else {
-          visitDirectiveProperty(ast) {
+            // Find a directive that matches the element name
-            if (!attributeValueSymbol(ast.value)) {
+            const directive = ast.directives.find(
-              symbol = findInputBinding(info, templatePosition, ast);
+                d => d.directive.selector != null && d.directive.selector.indexOf(ast.name) >= 0);
+            if (directive) {
+              compileTypeSummary = directive.directive;
+              symbol = info.template.query.getTypeSymbol(compileTypeSummary.type.reference);
+              symbol = symbol && new OverrideKindSymbol(symbol, DirectiveKind.DIRECTIVE);
               span = spanOf(ast);
             }
           }
         },
-        null);
+        visitReference(ast) {
-    if (symbol && span) {
+          symbol = ast.value && info.template.query.getTypeSymbol(tokenReference(ast.value));
-      return {symbol, span: offsetSpan(span, info.template.span.start), compileTypeSummary};
+          span = spanOf(ast);
-    }
+        },
+        visitVariable(ast) {},
+        visitEvent(ast) {
+          if (!attributeValueSymbol(ast.handler)) {
+            symbol = findOutputBinding(info, path, ast);
+            symbol = symbol && new OverrideKindSymbol(symbol, DirectiveKind.EVENT);
+            span = spanOf(ast);
+          }
+        },
+        visitElementProperty(ast) { attributeValueSymbol(ast.value); },
+        visitAttr(ast) {
+          const element = path.head;
+          if (!element || !(element instanceof ElementAst)) return;
+          // Create a mapping of all directives applied to the element from their selectors.
+          const matcher = new SelectorMatcher<DirectiveAst>();
+          for (const dir of element.directives) {
+            if (!dir.directive.selector) continue;
+            matcher.addSelectables(CssSelector.parse(dir.directive.selector), dir);
+          }
+
+          // See if this attribute matches the selector of any directive on the element.
+          const attributeSelector = `[${ast.name}=${ast.value}]`;
+          const parsedAttribute = CssSelector.parse(attributeSelector);
+          if (!parsedAttribute.length) return;
+          matcher.match(parsedAttribute[0], (_, directive) => {
+            symbol = info.template.query.getTypeSymbol(directive.directive.type.reference);
+            symbol = symbol && new OverrideKindSymbol(symbol, DirectiveKind.DIRECTIVE);
+            span = spanOf(ast);
+          });
+        },
+        visitBoundText(ast) {
+          const expressionPosition = templatePosition - ast.sourceSpan.start.offset;
+          if (inSpan(expressionPosition, ast.value.span)) {
+            const dinfo = diagnosticInfoFromTemplateInfo(info);
+            const scope = getExpressionScope(dinfo, path);
+            const result =
+                getExpressionSymbol(scope, ast.value, templatePosition, info.template.query);
+            if (result) {
+              symbol = result.symbol;
+              span = offsetSpan(result.span, ast.sourceSpan.start.offset);
+            }
+          }
+        },
+        visitText(ast) {},
+        visitDirective(ast) {
+          compileTypeSummary = ast.directive;
+          symbol = info.template.query.getTypeSymbol(compileTypeSummary.type.reference);
+          span = spanOf(ast);
+        },
+        visitDirectiveProperty(ast) {
+          if (!attributeValueSymbol(ast.value)) {
+            symbol = findInputBinding(info, templatePosition, ast);
+            span = spanOf(ast);
+          }
+        }
+      },
+      null);
+  if (symbol && span) {
+    return {symbol, span: offsetSpan(span, info.template.span.start), compileTypeSummary};
   }
 }
 

packages/language-service/test/definitions_spec.ts

@@ -14,6 +14,7 @@ import {TypeScriptServiceHost} from '../src/typescript_host';
 import {MockTypescriptHost} from './test_utils';
 
 const TEST_TEMPLATE = '/app/test.ng';
+const PARSING_CASES = '/app/parsing-cases.ts';
 
 describe('definitions', () => {
   const mockHost = new MockTypescriptHost(['/app/main.ts']);
@@ -49,28 +50,29 @@ describe('definitions', () => {
   });
 
   it('should be able to find a field in a attribute reference', () => {
-    const fileName = mockHost.addCode(`
+    mockHost.override(TEST_TEMPLATE, `<input [(ngModel)]="«title»">`);
-      @Component({
-        template: '<input [(ngModel)]="«name»">'
-      })
-      export class MyComponent {
-        «ᐱnameᐱ: string;»
-      }`);
 
-    const marker = mockHost.getReferenceMarkerFor(fileName, 'name');
+    const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'title');
-    const result = ngService.getDefinitionAndBoundSpan(fileName, marker.start);
+    const result = ngService.getDefinitionAndBoundSpan(TEST_TEMPLATE, marker.start);
     expect(result).toBeDefined();
     const {textSpan, definitions} = result !;
 
     expect(textSpan).toEqual(marker);
     expect(definitions).toBeDefined();
-    expect(definitions !.length).toBe(1);
-    const def = definitions ![0];
 
-    expect(def.fileName).toBe(fileName);
+    // There are exactly two, indentical definitions here, corresponding to the "name" on the
-    expect(def.name).toBe('name');
+    // property and event bindings of the two-way binding. The two-way binding is effectively
-    expect(def.kind).toBe('property');
+    // syntactic sugar for `[ngModel]="name" (ngModel)="name=$event"`.
-    expect(def.textSpan).toEqual(mockHost.getDefinitionMarkerFor(fileName, 'name'));
+    expect(definitions !.length).toBe(2);
+    for (const def of definitions !) {
+      expect(def.fileName).toBe(PARSING_CASES);
+      expect(def.name).toBe('title');
+      expect(def.kind).toBe('property');
+
+      const fileContent = mockHost.readFile(def.fileName);
+      expect(fileContent !.substring(def.textSpan.start, def.textSpan.start + def.textSpan.length))
+          .toEqual(`title = 'Some title';`);
+    }
   });
 
   it('should be able to find a method from a call', () => {
@@ -304,18 +306,24 @@ describe('definitions', () => {
     const boundedText = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'my');
     expect(textSpan).toEqual(boundedText);
 
-    // There should be exactly 1 definition
     expect(definitions).toBeDefined();
-    expect(definitions !.length).toBe(1);
+    expect(definitions !.length).toBe(2);
-    const def = definitions ![0];
+    const [def1, def2] = definitions !;
 
     const refFileName = '/app/parsing-cases.ts';
-    expect(def.fileName).toBe(refFileName);
+    expect(def1.fileName).toBe(refFileName);
-    expect(def.name).toBe('model');
+    expect(def1.name).toBe('model');
-    expect(def.kind).toBe('property');
+    expect(def1.kind).toBe('property');
-    const content = mockHost.readFile(refFileName) !;
+    let content = mockHost.readFile(refFileName) !;
-    expect(content.substring(def.textSpan.start, def.textSpan.start + def.textSpan.length))
+    expect(content.substring(def1.textSpan.start, def1.textSpan.start + def1.textSpan.length))
         .toEqual(`@Input() model: string = 'model';`);
+
+    expect(def2.fileName).toBe(refFileName);
+    expect(def2.name).toBe('modelChange');
+    expect(def2.kind).toBe('event');
+    content = mockHost.readFile(refFileName) !;
+    expect(content.substring(def2.textSpan.start, def2.textSpan.start + def2.textSpan.length))
+        .toEqual(`@Output() modelChange: EventEmitter<string> = new EventEmitter();`);
   });
 
   it('should be able to find a template from a url', () => {