refactor(language-service): Return directive defs when input name is part of selector (#39243)

When an input name is part of the directive selector, it would be good to return the directive as well
when performing 'go to definition' or 'go to type definition'. As an example, this would allow
'go to type definition' for ngIf to take the user to the NgIf directive.
'Go to type definition' would otherwise return no results because the
input is a generic type. This would also be the case for all primitive
input types.

PR Close #39243

packages/language-service/ivy/definitions.ts

@@ -6,16 +6,17 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {AST, TmplAstNode, TmplAstTextAttribute} from '@angular/compiler';
+import {AST, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstNode, TmplAstTemplate, TmplAstTextAttribute} from '@angular/compiler';
 import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
 import {DirectiveSymbol, DomBindingSymbol, ElementSymbol, ShimLocation, Symbol, SymbolKind, TemplateSymbol} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
 import * as ts from 'typescript';
 
-import {findNodeAtPosition} from './hybrid_visitor';
+import {getPathToNodeAtPosition} from './hybrid_visitor';
 import {flatMap, getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTemplateInfoAtPosition, getTextSpanOfNode, isDollarEvent, TemplateInfo, toTextSpan} from './utils';
 
 interface DefinitionMeta {
   node: AST|TmplAstNode;
+  path: Array<AST|TmplAstNode>;
   symbol: Symbol;
 }
 
@@ -41,12 +42,12 @@ export class DefinitionBuilder {
       return undefined;
     }
 
-    const definitions = this.getDefinitionsForSymbol(definitionMeta.symbol, definitionMeta.node);
+    const definitions = this.getDefinitionsForSymbol({...definitionMeta, ...templateInfo});
     return {definitions, textSpan: getTextSpanOfNode(definitionMeta.node)};
   }
 
-  private getDefinitionsForSymbol(symbol: Symbol, node: TmplAstNode|AST):
+  private getDefinitionsForSymbol({symbol, node, path, component}: DefinitionMeta&
-      readonly ts.DefinitionInfo[]|undefined {
+                                  TemplateInfo): readonly ts.DefinitionInfo[]|undefined {
     switch (symbol.kind) {
       case SymbolKind.Directive:
       case SymbolKind.Element:
@@ -58,9 +59,14 @@ export class DefinitionBuilder {
         // LS users to "go to definition" on an item in the template that maps to a class and be
         // taken to the directive or HTML class.
         return this.getTypeDefinitionsForTemplateInstance(symbol, node);
-      case SymbolKind.Input:
       case SymbolKind.Output:
-        return this.getDefinitionsForSymbols(...symbol.bindings);
+      case SymbolKind.Input: {
+        const bindingDefs = this.getDefinitionsForSymbols(...symbol.bindings);
+        // Also attempt to get directive matches for the input name. If there is a directive that
+        // has the input name as part of the selector, we want to return that as well.
+        const directiveDefs = this.getDirectiveTypeDefsForBindingNode(node, path, component);
+        return [...bindingDefs, ...directiveDefs];
+      }
       case SymbolKind.Variable:
       case SymbolKind.Reference: {
         const definitions: ts.DefinitionInfo[] = [];
@@ -112,8 +118,14 @@ export class DefinitionBuilder {
       case SymbolKind.Template:
         return this.getTypeDefinitionsForTemplateInstance(symbol, node);
       case SymbolKind.Output:
-      case SymbolKind.Input:
+      case SymbolKind.Input: {
-        return this.getTypeDefinitionsForSymbols(...symbol.bindings);
+        const bindingDefs = this.getTypeDefinitionsForSymbols(...symbol.bindings);
+        // Also attempt to get directive matches for the input name. If there is a directive that
+        // has the input name as part of the selector, we want to return that as well.
+        const directiveDefs = this.getDirectiveTypeDefsForBindingNode(
+            node, definitionMeta.path, templateInfo.component);
+        return [...bindingDefs, ...directiveDefs];
+      }
       case SymbolKind.Reference:
       case SymbolKind.Expression:
       case SymbolKind.Variable:
@@ -150,6 +162,28 @@ export class DefinitionBuilder {
     }
   }
 
+  private getDirectiveTypeDefsForBindingNode(
+      node: TmplAstNode|AST, pathToNode: Array<TmplAstNode|AST>, component: ts.ClassDeclaration) {
+    if (!(node instanceof TmplAstBoundAttribute) && !(node instanceof TmplAstTextAttribute) &&
+        !(node instanceof TmplAstBoundEvent)) {
+      return [];
+    }
+    const parent = pathToNode[pathToNode.length - 2];
+    if (!(parent instanceof TmplAstTemplate || parent instanceof TmplAstElement)) {
+      return [];
+    }
+    const templateOrElementSymbol =
+        this.compiler.getTemplateTypeChecker().getSymbolOfNode(parent, component);
+    if (templateOrElementSymbol === null ||
+        (templateOrElementSymbol.kind !== SymbolKind.Template &&
+         templateOrElementSymbol.kind !== SymbolKind.Element)) {
+      return [];
+    }
+    const dirs =
+        getDirectiveMatchesForAttribute(node.name, parent, templateOrElementSymbol.directives);
+    return this.getTypeDefinitionsForSymbols(...dirs);
+  }
+
   private getTypeDefinitionsForSymbols(...symbols: HasShimLocation[]): ts.DefinitionInfo[] {
     return flatMap(symbols, ({shimLocation}) => {
       const {shimPath, positionInShimFile} = shimLocation;
@@ -159,15 +193,16 @@ export class DefinitionBuilder {
 
   private getDefinitionMetaAtPosition({template, component}: TemplateInfo, position: number):
       DefinitionMeta|undefined {
-    const node = findNodeAtPosition(template, position);
+    const path = getPathToNodeAtPosition(template, position);
-    if (node === undefined) {
+    if (path === undefined) {
       return;
     }
 
+    const node = path[path.length - 1];
     const symbol = this.compiler.getTemplateTypeChecker().getSymbolOfNode(node, component);
     if (symbol === null) {
       return;
     }
-    return {node, symbol};
+    return {node, path, symbol};
   }
 }

packages/language-service/ivy/hybrid_visitor.ts

@@ -13,12 +13,14 @@ import * as t from '@angular/compiler/src/render3/r3_ast';         // t for temp
 import {isTemplateNode, isTemplateNodeWithKeyAndValue} from './utils';
 
 /**
- * Return the template AST node or expression AST node that most accurately
+ * Return the path to the template AST node or expression AST node that most accurately
  * represents the node at the specified cursor `position`.
+ *
  * @param ast AST tree
  * @param position cursor position
  */
-export function findNodeAtPosition(ast: t.Node[], position: number): t.Node|e.AST|undefined {
+export function getPathToNodeAtPosition(ast: t.Node[], position: number): Array<t.Node|e.AST>|
+    undefined {
   const visitor = new R3Visitor(position);
   visitor.visitAll(ast);
   const candidate = visitor.path[visitor.path.length - 1];
@@ -35,7 +37,22 @@ export function findNodeAtPosition(ast: t.Node[], position: number): t.Node|e.AS
       return;
     }
   }
-  return candidate;
+  return visitor.path;
+}
+
+/**
+ * Return the template AST node or expression AST node that most accurately
+ * represents the node at the specified cursor `position`.
+ *
+ * @param ast AST tree
+ * @param position cursor position
+ */
+export function findNodeAtPosition(ast: t.Node[], position: number): t.Node|e.AST|undefined {
+  const path = getPathToNodeAtPosition(ast, position);
+  if (!path) {
+    return;
+  }
+  return path[path.length - 1];
 }
 
 class R3Visitor implements t.Visitor {

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

@@ -88,11 +88,14 @@ describe('definitions', () => {
         templateOverride: `<div *ng¦If="anyValue"></div>`,
         expectedSpanText: 'ngIf',
       });
-      expect(definitions!.length).toEqual(1);
+      // Because the input is also part of the selector, the directive is also returned.
+      expect(definitions!.length).toEqual(2);
+      const [inputDef, directiveDef] = definitions;
 
-      const [def] = definitions;
+      expect(inputDef.textSpan).toEqual('ngIf');
-      expect(def.textSpan).toEqual('ngIf');
+      expect(inputDef.contextSpan).toEqual('set ngIf(condition: T);');
-      expect(def.contextSpan).toEqual('set ngIf(condition: T);');
+      expect(directiveDef.textSpan).toEqual('NgIf');
+      expect(directiveDef.contextSpan).toContain('export declare class NgIf');
     });
 
     it('should work for directives with compound selectors', () => {
@@ -125,6 +128,18 @@ describe('definitions', () => {
         expect(def.contextSpan).toEqual(`@Input('tcName') name = 'test';`);
       });
 
+      it('should work for text inputs', () => {
+        const definitions = getDefinitionsAndAssertBoundSpan({
+          templateOverride: `<test-comp tcN¦ame="name"></test-comp>`,
+          expectedSpanText: 'tcName="name"',
+        });
+        expect(definitions!.length).toEqual(1);
+
+        const [def] = definitions;
+        expect(def.textSpan).toEqual('name');
+        expect(def.contextSpan).toEqual(`@Input('tcName') name = 'test';`);
+      });
+
       it('should work for structural directive inputs ngForTrackBy', () => {
         const definitions = getDefinitionsAndAssertBoundSpan({
           templateOverride: `<div *ngFor="let item of heroes; tr¦ackBy: test;"></div>`,
@@ -145,12 +160,16 @@ describe('definitions', () => {
           templateOverride: `<div *ngFor="let item o¦f heroes"></div>`,
           expectedSpanText: 'of',
         });
-        expect(definitions!.length).toEqual(1);
+        // Because the input is also part of the selector ([ngFor][ngForOf]), the directive is also
+        // returned.
+        expect(definitions!.length).toEqual(2);
+        const [inputDef, directiveDef] = definitions;
 
-        const [def] = definitions;
+        expect(inputDef.textSpan).toEqual('ngForOf');
-        expect(def.textSpan).toEqual('ngForOf');
+        expect(inputDef.contextSpan)
-        expect(def.contextSpan)
             .toEqual('set ngForOf(ngForOf: U & NgIterable<T> | undefined | null);');
+        expect(directiveDef.textSpan).toEqual('NgForOf');
+        expect(directiveDef.contextSpan).toContain('export declare class NgForOf');
       });
 
       it('should work for two-way binding providers', () => {
@@ -191,6 +210,20 @@ describe('definitions', () => {
         const definitionAndBoundSpan = ngLS.getDefinitionAndBoundSpan(APP_COMPONENT, position);
         expect(definitionAndBoundSpan).toBeUndefined();
       });
+
+      it('should return the directive when the event is part of the selector', () => {
+        const definitions = getDefinitionsAndAssertBoundSpan({
+          templateOverride: `<div (eventSelect¦or)="title = ''"></div>`,
+          expectedSpanText: `(eventSelector)="title = ''"`,
+        });
+        expect(definitions!.length).toEqual(2);
+
+        const [inputDef, directiveDef] = definitions;
+        expect(inputDef.textSpan).toEqual('eventSelector');
+        expect(inputDef.contextSpan).toEqual('@Output() eventSelector = new EventEmitter<void>();');
+        expect(directiveDef.textSpan).toEqual('EventSelectorDirective');
+        expect(directiveDef.contextSpan).toContain('export class EventSelectorDirective');
+      });
     });
   });
 

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

@@ -14,6 +14,10 @@ import {HumanizedDefinitionInfo, humanizeDefinitionInfo} from './test_utils';
 describe('type definitions', () => {
   const {project, service, tsLS} = setup();
   const ngLS = new LanguageService(project, tsLS);
+  const possibleArrayDefFiles = new Set([
+    'lib.es5.d.ts', 'lib.es2015.core.d.ts', 'lib.es2015.iterable.d.ts',
+    'lib.es2015.symbol.wellknown.d.ts', 'lib.es2016.array.include.d.ts'
+  ]);
 
   beforeEach(() => {
     service.reset();
@@ -121,7 +125,11 @@ describe('type definitions', () => {
         const definitions = getTypeDefinitionsAndAssertBoundSpan({
           templateOverride: `<div *ngFor="let item o¦f heroes"></div>`,
         });
-        expectAllArrayDefinitions(definitions);
+        // In addition to all the array defs, this will also return the NgForOf def because the
+        // input is part of the selector ([ngFor][ngForOf]).
+        expectAllDefinitions(
+            definitions, new Set(['Array', 'NgForOf']),
+            new Set([...possibleArrayDefFiles, 'ng_for_of.d.ts']));
       });
 
       it('should return nothing for two-way binding providers', () => {
@@ -141,10 +149,25 @@ describe('type definitions', () => {
         });
         expect(definitions!.length).toEqual(2);
 
-        const [def, xyz] = definitions;
+        const [emitterInterface, emitterConst] = definitions;
-        expect(def.textSpan).toEqual('EventEmitter');
+        expect(emitterInterface.textSpan).toEqual('EventEmitter');
-        expect(def.contextSpan).toContain('export interface EventEmitter<T> extends Subject<T>');
+        expect(emitterInterface.contextSpan)
-        expect(xyz.textSpan).toEqual('EventEmitter');
+            .toContain('export interface EventEmitter<T> extends Subject<T>');
+        expect(emitterInterface.fileName).toContain('event_emitter.d.ts');
+        expect(emitterConst.textSpan).toEqual('EventEmitter');
+        expect(emitterConst.contextSpan).toContain('export declare const EventEmitter');
+        expect(emitterConst.fileName).toContain('event_emitter.d.ts');
+      });
+
+      it('should return the directive when the event is part of the selector', () => {
+        const definitions = getTypeDefinitionsAndAssertBoundSpan({
+          templateOverride: `<div (eventSelect¦or)="title = ''"></div>`,
+        });
+        expect(definitions!.length).toEqual(3);
+
+        // EventEmitter tested in previous test
+        const directiveDef = definitions[2];
+        expect(directiveDef.contextSpan).toContain('export class EventSelectorDirective');
       });
     });
   });
@@ -264,21 +287,21 @@ describe('type definitions', () => {
       const definitions = getTypeDefinitionsAndAssertBoundSpan({
         templateOverride: `<div *ngFor="let item of heroes as her¦oes2; trackBy: test;"></div>`,
       });
-      expectAllArrayDefinitions(definitions);
+      expectAllDefinitions(definitions, new Set(['Array']), possibleArrayDefFiles);
     });
 
     it('should work for uses of members in structural directives', () => {
       const definitions = getTypeDefinitionsAndAssertBoundSpan({
         templateOverride: `<div *ngFor="let item of heroes as heroes2">{{her¦oes2}}</div>`,
       });
-      expectAllArrayDefinitions(definitions);
+      expectAllDefinitions(definitions, new Set(['Array']), possibleArrayDefFiles);
     });
 
     it('should work for members in structural directives', () => {
       const definitions = getTypeDefinitionsAndAssertBoundSpan({
         templateOverride: `<div *ngFor="let item of her¦oes; trackBy: test;"></div>`,
       });
-      expectAllArrayDefinitions(definitions);
+      expectAllDefinitions(definitions, new Set(['Array']), possibleArrayDefFiles);
     });
 
     it('should return nothing for the $any() cast function', () => {
@@ -297,14 +320,12 @@ describe('type definitions', () => {
     return defs!.map(d => humanizeDefinitionInfo(d, service));
   }
 
-  function expectAllArrayDefinitions(definitions: HumanizedDefinitionInfo[]) {
+  function expectAllDefinitions(
+      definitions: HumanizedDefinitionInfo[], textSpans: Set<string>,
+      possibleFileNames: Set<string>) {
     expect(definitions!.length).toBeGreaterThan(0);
     const actualTextSpans = new Set(definitions.map(d => d.textSpan));
-    expect(actualTextSpans).toEqual(new Set(['Array']));
+    expect(actualTextSpans).toEqual(textSpans);
-    const possibleFileNames = [
-      'lib.es5.d.ts', 'lib.es2015.core.d.ts', 'lib.es2015.iterable.d.ts',
-      'lib.es2015.symbol.wellknown.d.ts', 'lib.es2016.array.include.d.ts'
-    ];
     for (const def of definitions) {
       const fileName = def.fileName.split('/').slice(-1)[0];
       expect(possibleFileNames)

packages/language-service/ivy/utils.ts

@@ -143,8 +143,12 @@ function getInlineTemplateInfoAtPosition(
 /**
  * Given an attribute node, converts it to string form.
  */
-function toAttributeString(attribute: t.TextAttribute|t.BoundAttribute): string {
+function toAttributeString(attribute: t.TextAttribute|t.BoundAttribute|t.BoundEvent): string {
+  if (attribute instanceof t.BoundEvent) {
+    return `[${attribute.name}]`;
+  } else {
     return `[${attribute.name}=${attribute.valueSpan?.toString() ?? ''}]`;
+  }
 }
 
 function getNodeName(node: t.Template|t.Element): string {
@@ -154,8 +158,10 @@ function getNodeName(node: t.Template|t.Element): string {
 /**
  * Given a template or element node, returns all attributes on the node.
  */
-function getAttributes(node: t.Template|t.Element): Array<t.TextAttribute|t.BoundAttribute> {
+function getAttributes(node: t.Template|
-  const attributes: Array<t.TextAttribute|t.BoundAttribute> = [...node.attributes, ...node.inputs];
+                       t.Element): Array<t.TextAttribute|t.BoundAttribute|t.BoundEvent> {
+  const attributes: Array<t.TextAttribute|t.BoundAttribute|t.BoundEvent> =
+      [...node.attributes, ...node.inputs, ...node.outputs];
   if (node instanceof t.Template) {
     attributes.push(...node.templateAttrs);
   }
@@ -216,8 +222,8 @@ export function getDirectiveMatchesForAttribute(
   const allDirectiveMatches =
       getDirectiveMatchesForSelector(directives, getNodeName(hostNode) + allAttrs.join(''));
   const attrsExcludingName = attributes.filter(a => a.name !== name).map(toAttributeString);
-  const matchesWithoutAttr =
+  const matchesWithoutAttr = getDirectiveMatchesForSelector(
-      getDirectiveMatchesForSelector(directives, attrsExcludingName.join(''));
+      directives, getNodeName(hostNode) + attrsExcludingName.join(''));
   return difference(allDirectiveMatches, matchesWithoutAttr);
 }
 

packages/language-service/test/project/app/main.ts

@@ -25,6 +25,7 @@ import * as ParsingCases from './parsing-cases';
     ParsingCases.TestPipe,
     ParsingCases.WithContextDirective,
     ParsingCases.CompoundCustomButtonDirective,
+    ParsingCases.EventSelectorDirective,
   ]
 })
 export class AppModule {

packages/language-service/test/project/app/parsing-cases.ts

@@ -75,6 +75,11 @@ export class CompoundCustomButtonDirective {
   @Input() config?: {color?: string};
 }
 
+@Directive({selector: '[eventSelector]'})
+export class EventSelectorDirective {
+  @Output() eventSelector = new EventEmitter<void>();
+}
+
 @Pipe({
   name: 'prefixPipe',
 })