From cc8b76ef7cb908d2c95229f39bf82a13ca59570b Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Mon, 21 Nov 2022 22:47:55 +0100 Subject: [PATCH] fix(language-service): correctly handle host directive inputs/outputs (#48147) Adds some logic to correctly handle hidden or aliased inputs/outputs in the language service. Fixes #48102. PR Close #48147 --- .../src/ngtsc/typecheck/api/api.ts | 4 +- .../src/ngtsc/typecheck/api/symbols.ts | 17 +- .../typecheck/src/template_symbol_builder.ts | 108 ++++++--- .../src/ngtsc/typecheck/testing/index.ts | 2 +- .../src/attribute_completions.ts | 26 ++- .../language-service/test/completions_spec.ts | 213 ++++++++++++++++++ 6 files changed, 328 insertions(+), 42 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts index 88a6182406f95..e9912c4a21542 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts @@ -10,9 +10,8 @@ import {AbsoluteSourceSpan, BoundTarget, DirectiveMeta, ParseSourceSpan, SchemaM import ts from 'typescript'; import {ErrorCode} from '../../diagnostics'; -import {AbsoluteFsPath} from '../../file_system'; import {Reference} from '../../imports'; -import {ClassPropertyMapping, DirectiveTypeCheckMeta} from '../../metadata'; +import {ClassPropertyMapping, DirectiveTypeCheckMeta, HostDirectiveMeta} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; @@ -26,6 +25,7 @@ export interface TypeCheckableDirectiveMeta extends DirectiveMeta, DirectiveType inputs: ClassPropertyMapping; outputs: ClassPropertyMapping; isStandalone: boolean; + hostDirectives: HostDirectiveMeta[]|null; } export type TemplateId = string&{__brand: 'TemplateId'}; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts index 219a23ca048b4..4384a46be3a0a 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts @@ -258,11 +258,8 @@ export interface TemplateSymbol { templateNode: TmplAstTemplate; } -/** - * A representation of a directive/component whose selector matches a node in a component - * template. - */ -export interface DirectiveSymbol extends PotentialDirective { +/** Interface shared between host and non-host directives. */ +interface DirectiveSymbolBase extends PotentialDirective { kind: SymbolKind.Directive; /** The `ts.Type` for the class declaration. */ @@ -272,6 +269,16 @@ export interface DirectiveSymbol extends PotentialDirective { tcbLocation: TcbLocation; } +/** + * A representation of a directive/component whose selector matches a node in a component + * template. + */ +export type DirectiveSymbol = (DirectiveSymbolBase&{isHostDirective: false})|(DirectiveSymbolBase&{ + isHostDirective: true; + exposedInputs: Record|null; + exposedOutputs: Record|null; +}); + /** * A representation of an attribute on an element or template. These bindings aren't currently * type-checked (see `checkTypeOfDomBindings`) so they won't have a `ts.Type`, `ts.Symbol`, or shim diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts index 77fd37d3b6a4f..031da360cc31a 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts @@ -11,6 +11,7 @@ import ts from 'typescript'; import {AbsoluteFsPath} from '../../file_system'; import {Reference} from '../../imports'; +import {HostDirectiveMeta} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; import {ComponentScopeKind, ComponentScopeReader} from '../../scope'; import {isAssignment, isSymbolWithValueDeclaration} from '../../util/src/typescript'; @@ -118,38 +119,80 @@ export class SymbolBuilder { const nodes = findAllMatchingNodes( this.typeCheckBlock, {withSpan: elementSourceSpan, filter: isDirectiveDeclaration}); - return nodes - .map(node => { - const symbol = this.getSymbolOfTsNode(node.parent); - if (symbol === null || !isSymbolWithValueDeclaration(symbol.tsSymbol) || - !ts.isClassDeclaration(symbol.tsSymbol.valueDeclaration)) { - return null; - } - const meta = this.getDirectiveMeta(element, symbol.tsSymbol.valueDeclaration); - if (meta === null) { - return null; - } - - const ngModule = this.getDirectiveModule(symbol.tsSymbol.valueDeclaration); - if (meta.selector === null) { - return null; - } - const isComponent = meta.isComponent ?? null; - const ref = new Reference(symbol.tsSymbol.valueDeclaration as any); - const directiveSymbol: DirectiveSymbol = { - ...symbol, - ref, - tsSymbol: symbol.tsSymbol, - selector: meta.selector, - isComponent, - ngModule, - kind: SymbolKind.Directive, - isStructural: meta.isStructural, - isInScope: true, - }; - return directiveSymbol; - }) - .filter((d): d is DirectiveSymbol => d !== null); + const symbols: DirectiveSymbol[] = []; + + for (const node of nodes) { + const symbol = this.getSymbolOfTsNode(node.parent); + if (symbol === null || !isSymbolWithValueDeclaration(symbol.tsSymbol) || + !ts.isClassDeclaration(symbol.tsSymbol.valueDeclaration)) { + continue; + } + + const meta = this.getDirectiveMeta(element, symbol.tsSymbol.valueDeclaration); + + if (meta !== null && meta.selector !== null) { + const ref = new Reference(symbol.tsSymbol.valueDeclaration as any); + + if (meta.hostDirectives !== null) { + this.addHostDirectiveSymbols(element, meta.hostDirectives, symbols); + } + + const directiveSymbol: DirectiveSymbol = { + ...symbol, + ref, + tsSymbol: symbol.tsSymbol, + selector: meta.selector, + isComponent: meta.isComponent, + ngModule: this.getDirectiveModule(symbol.tsSymbol.valueDeclaration), + kind: SymbolKind.Directive, + isStructural: meta.isStructural, + isInScope: true, + isHostDirective: false, + }; + + symbols.push(directiveSymbol); + } + } + + return symbols; + } + + private addHostDirectiveSymbols( + host: TmplAstTemplate|TmplAstElement, hostDirectives: HostDirectiveMeta[], + symbols: DirectiveSymbol[]): void { + for (const current of hostDirectives) { + if (!ts.isClassDeclaration(current.directive.node)) { + continue; + } + + const symbol = this.getSymbolOfTsNode(current.directive.node); + const meta = this.getDirectiveMeta(host, current.directive.node); + + if (meta !== null && symbol !== null && isSymbolWithValueDeclaration(symbol.tsSymbol)) { + if (meta.hostDirectives !== null) { + this.addHostDirectiveSymbols(host, meta.hostDirectives, symbols); + } + + const directiveSymbol: DirectiveSymbol = { + ...symbol, + isHostDirective: true, + ref: current.directive, + tsSymbol: symbol.tsSymbol, + exposedInputs: current.inputs, + exposedOutputs: current.outputs, + // TODO(crisbeto): rework `DirectiveSymbol` to make + // `selector` nullable and remove the `|| ''` here. + selector: meta.selector || '', + isComponent: meta.isComponent, + ngModule: this.getDirectiveModule(current.directive.node), + kind: SymbolKind.Directive, + isStructural: meta.isStructural, + isInScope: true, + }; + + symbols.push(directiveSymbol); + } + } } private getDirectiveMeta( @@ -376,6 +419,7 @@ export class SymbolBuilder { isStructural, selector, ngModule, + isHostDirective: false, isInScope: true, // TODO: this should always be in scope in this context, right? }; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts index 90f1e0b229306..e11dbdf385d78 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts @@ -228,7 +228,7 @@ export interface TestDirective extends Partial>> { + 'undeclaredInputFields'|'inputs'|'outputs'|'hostDirectives'>>> { selector: string; name: string; file?: AbsoluteFsPath; diff --git a/packages/language-service/src/attribute_completions.ts b/packages/language-service/src/attribute_completions.ts index c844b92677dbf..dc33456fd202d 100644 --- a/packages/language-service/src/attribute_completions.ts +++ b/packages/language-service/src/attribute_completions.ts @@ -220,7 +220,18 @@ export function buildAttributeCompletionTable( continue; } - for (const [classPropertyName, propertyName] of meta.inputs) { + for (const [classPropertyName, rawProperyName] of meta.inputs) { + let propertyName: string; + + if (dirSymbol.isHostDirective) { + if (!dirSymbol.exposedInputs?.hasOwnProperty(rawProperyName)) { + continue; + } + propertyName = dirSymbol.exposedInputs[rawProperyName]; + } else { + propertyName = rawProperyName; + } + if (table.has(propertyName)) { continue; } @@ -234,7 +245,18 @@ export function buildAttributeCompletionTable( }); } - for (const [classPropertyName, propertyName] of meta.outputs) { + for (const [classPropertyName, rawProperyName] of meta.outputs) { + let propertyName: string; + + if (dirSymbol.isHostDirective) { + if (!dirSymbol.exposedOutputs?.hasOwnProperty(rawProperyName)) { + continue; + } + propertyName = dirSymbol.exposedOutputs[rawProperyName]; + } else { + propertyName = rawProperyName; + } + if (table.has(propertyName)) { continue; } diff --git a/packages/language-service/test/completions_spec.ts b/packages/language-service/test/completions_spec.ts index ca83b5121769b..71c7789610931 100644 --- a/packages/language-service/test/completions_spec.ts +++ b/packages/language-service/test/completions_spec.ts @@ -613,6 +613,129 @@ describe('completions', () => { completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY), ['myInput']); }); + + it('should return completion for input coming from a host directive', () => { + const {templateFile} = setup(``, '', { + 'Dir': ` + @Directive({ + standalone: true, + inputs: ['myInput'] + }) + export class HostDir { + myInput = 'foo'; + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{ + directive: HostDir, + inputs: ['myInput'] + }] + }) + export class Dir { + } + ` + }); + templateFile.moveCursorToText('my¦>'); + + const completions = templateFile.getCompletionsAtPosition(); + + expectContain( + completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY), + ['[myInput]']); + }); + + it('should not return completion for hidden host directive input', () => { + const {templateFile} = setup(``, '', { + 'Dir': ` + @Directive({ + standalone: true, + inputs: ['myInput'] + }) + export class HostDir { + myInput = 'foo'; + } + + @Directive({ + selector: '[dir]', + hostDirectives: [HostDir] + }) + export class Dir { + } + ` + }); + templateFile.moveCursorToText('my¦>'); + + const completions = templateFile.getCompletionsAtPosition(); + + expectDoesNotContain( + completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY), + ['[myInput]']); + }); + + it('should return completion for aliased host directive input', () => { + const {templateFile} = setup(``, '', { + 'Dir': ` + @Directive({ + standalone: true, + inputs: ['myInput'] + }) + export class HostDir { + myInput = 'foo'; + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{ + directive: HostDir, + inputs: ['myInput: alias'] + }] + }) + export class Dir { + } + ` + }); + templateFile.moveCursorToText('ali¦>'); + + const completions = templateFile.getCompletionsAtPosition(); + + expectContain( + completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY), + ['[alias]']); + }); + + it('should return completion for aliased host directive input that has a different public name', + () => { + const {templateFile} = setup(``, '', { + 'Dir': ` + @Directive({ + standalone: true, + inputs: ['myInput: myPublicInput'] + }) + export class HostDir { + myInput = 'foo'; + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{ + directive: HostDir, + inputs: ['myPublicInput: alias'] + }] + }) + export class Dir { + } + ` + }); + templateFile.moveCursorToText('ali¦>'); + + const completions = templateFile.getCompletionsAtPosition(); + + expectContain( + completions, + unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY), + ['[alias]']); + }); }); describe('structural directive present', () => { @@ -868,6 +991,96 @@ describe('completions', () => { completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.EVENT), ['customModelChange']); }); + + it('should return completion for output coming from a host directive', () => { + const {templateFile} = setup(``, '', { + 'Dir': ` + @Directive({ + standalone: true, + outputs: ['myOutput'] + }) + export class HostDir { + myOutput: any; + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{ + directive: HostDir, + outputs: ['myOutput'] + }] + }) + export class Dir { + } + ` + }); + templateFile.moveCursorToText('(my¦)'); + + const completions = templateFile.getCompletionsAtPosition(); + expectContain( + completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.EVENT), + ['myOutput']); + expectReplacementText(completions, templateFile.contents, 'my'); + }); + + it('should not return completion for hidden host directive output', () => { + const {templateFile} = setup(``, '', { + 'Dir': ` + @Directive({ + standalone: true, + outputs: ['myOutput'] + }) + export class HostDir { + myOutput: any; + } + + @Directive({ + selector: '[dir]', + hostDirectives: [HostDir] + }) + export class Dir { + } + ` + }); + templateFile.moveCursorToText('(my¦)'); + + const completions = templateFile.getCompletionsAtPosition(); + expectDoesNotContain( + completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.EVENT), + ['myOutput']); + }); + + it('should return completion for aliased host directive output that has a different public name', + () => { + const {templateFile} = setup(``, '', { + 'Dir': ` + @Directive({ + standalone: true, + outputs: ['myOutput: myPublicOutput'] + }) + export class HostDir { + myOutput: any; + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{ + directive: HostDir, + outputs: ['myPublicOutput: alias'] + }] + }) + export class Dir { + } + ` + }); + templateFile.moveCursorToText('(ali¦)'); + + const completions = templateFile.getCompletionsAtPosition(); + expectContain( + completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.EVENT), + ['alias']); + expectReplacementText(completions, templateFile.contents, 'ali'); + }); }); });