diff --git a/packages/language-service/ivy/definitions.ts b/packages/language-service/ivy/definitions.ts index 7de4ecc63a..4ce1e15caa 100644 --- a/packages/language-service/ivy/definitions.ts +++ b/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; 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): - readonly ts.DefinitionInfo[]|undefined { + private getDefinitionsForSymbol({symbol, node, path, component}: DefinitionMeta& + 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: - return this.getTypeDefinitionsForSymbols(...symbol.bindings); + case SymbolKind.Input: { + 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, 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); - if (node === undefined) { + const path = getPathToNodeAtPosition(template, position); + 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}; } } diff --git a/packages/language-service/ivy/hybrid_visitor.ts b/packages/language-service/ivy/hybrid_visitor.ts index c8e8c3a152..391809e38d 100644 --- a/packages/language-service/ivy/hybrid_visitor.ts +++ b/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| + 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 { diff --git a/packages/language-service/ivy/test/definitions_spec.ts b/packages/language-service/ivy/test/definitions_spec.ts index 99313d2328..6e2e568bf1 100644 --- a/packages/language-service/ivy/test/definitions_spec.ts +++ b/packages/language-service/ivy/test/definitions_spec.ts @@ -88,11 +88,14 @@ describe('definitions', () => { templateOverride: `
`, 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(def.textSpan).toEqual('ngIf'); - expect(def.contextSpan).toEqual('set ngIf(condition: T);'); + expect(inputDef.textSpan).toEqual('ngIf'); + expect(inputDef.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: ``, + 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: `
`, @@ -145,12 +160,16 @@ describe('definitions', () => { templateOverride: `
`, 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(def.textSpan).toEqual('ngForOf'); - expect(def.contextSpan) + expect(inputDef.textSpan).toEqual('ngForOf'); + expect(inputDef.contextSpan) .toEqual('set ngForOf(ngForOf: U & NgIterable | 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: `
`, + 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();'); + expect(directiveDef.textSpan).toEqual('EventSelectorDirective'); + expect(directiveDef.contextSpan).toContain('export class EventSelectorDirective'); + }); }); }); diff --git a/packages/language-service/ivy/test/type_definitions_spec.ts b/packages/language-service/ivy/test/type_definitions_spec.ts index fd394b0e0e..cffed6d2fc 100644 --- a/packages/language-service/ivy/test/type_definitions_spec.ts +++ b/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: `
`, }); - 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; - expect(def.textSpan).toEqual('EventEmitter'); - expect(def.contextSpan).toContain('export interface EventEmitter extends Subject'); - expect(xyz.textSpan).toEqual('EventEmitter'); + const [emitterInterface, emitterConst] = definitions; + expect(emitterInterface.textSpan).toEqual('EventEmitter'); + expect(emitterInterface.contextSpan) + .toContain('export interface EventEmitter extends Subject'); + 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: `
`, + }); + 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: `
`, }); - expectAllArrayDefinitions(definitions); + expectAllDefinitions(definitions, new Set(['Array']), possibleArrayDefFiles); }); it('should work for uses of members in structural directives', () => { const definitions = getTypeDefinitionsAndAssertBoundSpan({ templateOverride: `
{{her¦oes2}}
`, }); - expectAllArrayDefinitions(definitions); + expectAllDefinitions(definitions, new Set(['Array']), possibleArrayDefFiles); }); it('should work for members in structural directives', () => { const definitions = getTypeDefinitionsAndAssertBoundSpan({ templateOverride: `
`, }); - 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, + possibleFileNames: Set) { expect(definitions!.length).toBeGreaterThan(0); const actualTextSpans = new Set(definitions.map(d => d.textSpan)); - expect(actualTextSpans).toEqual(new Set(['Array'])); - 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' - ]; + expect(actualTextSpans).toEqual(textSpans); for (const def of definitions) { const fileName = def.fileName.split('/').slice(-1)[0]; expect(possibleFileNames) diff --git a/packages/language-service/ivy/utils.ts b/packages/language-service/ivy/utils.ts index 1e5c4107d6..8fb027c7c6 100644 --- a/packages/language-service/ivy/utils.ts +++ b/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 { - return `[${attribute.name}=${attribute.valueSpan?.toString() ?? ''}]`; +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 { - const attributes: Array = [...node.attributes, ...node.inputs]; +function getAttributes(node: t.Template| + t.Element): Array { + const attributes: Array = + [...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 = - getDirectiveMatchesForSelector(directives, attrsExcludingName.join('')); + const matchesWithoutAttr = getDirectiveMatchesForSelector( + directives, getNodeName(hostNode) + attrsExcludingName.join('')); return difference(allDirectiveMatches, matchesWithoutAttr); } diff --git a/packages/language-service/test/project/app/main.ts b/packages/language-service/test/project/app/main.ts index 331a79cd3b..4860cc3930 100644 --- a/packages/language-service/test/project/app/main.ts +++ b/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 { diff --git a/packages/language-service/test/project/app/parsing-cases.ts b/packages/language-service/test/project/app/parsing-cases.ts index 74004c02c6..5f9bec3e7c 100644 --- a/packages/language-service/test/project/app/parsing-cases.ts +++ b/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(); +} + @Pipe({ name: 'prefixPipe', })