diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/annotations/common/src/diagnostics.ts index 9aa5ee6aef93d3..b41b7fbbac6bda 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/common/src/diagnostics.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/common/src/diagnostics.ts @@ -10,7 +10,7 @@ import ts from 'typescript'; import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../../diagnostics'; import {Reference} from '../../../imports'; -import {HostDirectiveMeta, MetadataReader} from '../../../metadata'; +import {DirectiveMeta, flattenInheritedDirectiveMetadata, HostDirectiveMeta, MetadataReader} from '../../../metadata'; import {describeResolvedType, DynamicValue, PartialEvaluator, ResolvedValue, traceDynamicValue} from '../../../partial_evaluator'; import {ClassDeclaration, ReflectionHost} from '../../../reflection'; import {DeclarationData, LocalModuleScopeRegistry} from '../../../scope'; @@ -161,7 +161,7 @@ export function validateHostDirectives( const diagnostics: ts.DiagnosticWithLocation[] = []; for (const current of hostDirectives) { - const hostMeta = metaReader.getDirectiveMetadata(current.directive); + const hostMeta = flattenInheritedDirectiveMetadata(metaReader, current.directive); if (hostMeta === null) { diagnostics.push(makeDiagnostic( @@ -184,11 +184,52 @@ export function validateHostDirectives( ErrorCode.HOST_DIRECTIVE_COMPONENT, current.directive.getOriginForDiagnostics(origin), `Host directive ${hostMeta.name} cannot be a component`)); } + + validateHostDirectiveMappings('input', current, hostMeta, origin, diagnostics); + validateHostDirectiveMappings('output', current, hostMeta, origin, diagnostics); } return diagnostics; } +function validateHostDirectiveMappings( + bindingType: 'input'|'output', hostDirectiveMeta: HostDirectiveMeta, meta: DirectiveMeta, + origin: ts.Expression, diagnostics: ts.DiagnosticWithLocation[]) { + const className = meta.name; + const hostDirectiveMappings = + bindingType === 'input' ? hostDirectiveMeta.inputs : hostDirectiveMeta.outputs; + const existingBindings = bindingType === 'input' ? meta.inputs : meta.outputs; + + for (const publicName in hostDirectiveMappings) { + if (hostDirectiveMappings.hasOwnProperty(publicName)) { + if (!existingBindings.hasBindingPropertyName(publicName)) { + diagnostics.push(makeDiagnostic( + ErrorCode.HOST_DIRECTIVE_UNDEFINED_BINDING, + hostDirectiveMeta.directive.getOriginForDiagnostics(origin), + `Directive ${className} does not have an ${bindingType} with a public name of ${ + publicName}.`)); + } + + const remappedPublicName = hostDirectiveMappings[publicName]; + const bindingsForPublicName = existingBindings.getByBindingPropertyName(remappedPublicName); + + if (bindingsForPublicName !== null) { + for (const binding of bindingsForPublicName) { + if (binding.bindingPropertyName !== publicName) { + diagnostics.push(makeDiagnostic( + ErrorCode.HOST_DIRECTIVE_CONFLICTING_ALIAS, + hostDirectiveMeta.directive.getOriginForDiagnostics(origin), + `Cannot alias ${bindingType} ${publicName} of host directive ${className} to ${ + remappedPublicName}, because it already has a different ${ + bindingType} with the same public name.`)); + } + } + } + } + } +} + + export function getUndecoratedClassWithAngularFeaturesDiagnostic(node: ClassDeclaration): ts.Diagnostic { return makeDiagnostic( diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts index 545c3a861d7b40..c426299d80466c 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -92,6 +92,15 @@ export enum ErrorCode { */ INJECTABLE_INHERITS_INVALID_CONSTRUCTOR = 2016, + /** Raised when a host tries to alias a host directive binding that does not exist. */ + HOST_DIRECTIVE_UNDEFINED_BINDING = 2017, + + /** + * Raised when a host tries to alias a host directive + * binding to a pre-existing binding's public name. + */ + HOST_DIRECTIVE_CONFLICTING_ALIAS = 2018, + SYMBOL_NOT_EXPORTED = 3001, /** * Raised when a relationship between directives and/or pipes would cause a cyclic import to be diff --git a/packages/compiler-cli/test/ngtsc/host_directives_spec.ts b/packages/compiler-cli/test/ngtsc/host_directives_spec.ts index 9285ab02847a93..cbade7b9e2aa6a 100644 --- a/packages/compiler-cli/test/ngtsc/host_directives_spec.ts +++ b/packages/compiler-cli/test/ngtsc/host_directives_spec.ts @@ -357,8 +357,8 @@ runInEachFileSystem(() => { import {ɵɵDirectiveDeclaration} from '@angular/core'; export declare class ExternalDir { - static ɵdir: ɵɵDirectiveDeclaration; + static ɵdir: ɵɵDirectiveDeclaration; } `); @@ -434,7 +434,7 @@ runInEachFileSystem(() => { diag.messageText.messageText; } - it('should throw if a host directive is not standalone', () => { + it('should produce a diagnostic if a host directive is not standalone', () => { env.write('test.ts', ` import {Directive, Component, NgModule} from '@angular/core'; @@ -451,7 +451,7 @@ runInEachFileSystem(() => { expect(messages).toEqual(['Host directive HostDir must be standalone']); }); - it('should throw if a host directive is a component', () => { + it('should produce a diagnostic if a host directive is a component', () => { env.write('test.ts', ` import {Directive, Component, NgModule} from '@angular/core'; @@ -471,7 +471,7 @@ runInEachFileSystem(() => { expect(messages).toEqual(['Host directive HostComp cannot be a component']); }); - it('should throw if hostDirectives is not an array', () => { + it('should produce a diagnostic if hostDirectives is not an array', () => { env.write('test.ts', ` import {Component} from '@angular/core'; @@ -486,7 +486,7 @@ runInEachFileSystem(() => { expect(messages).toContain('hostDirectives must be an array'); }); - it('should throw if a host directive is not a reference', () => { + it('should produce a diagnostic if a host directive is not a reference', () => { env.write('test.ts', ` import {Component} from '@angular/core'; @@ -503,7 +503,7 @@ runInEachFileSystem(() => { expect(messages).toEqual(['Host directive must be a reference']); }); - it('should throw if a host directive is not a reference to a class', () => { + it('should produce a diagnostic if a host directive is not a reference to a class', () => { env.write('test.ts', ` import {Component} from '@angular/core'; @@ -520,7 +520,7 @@ runInEachFileSystem(() => { expect(messages).toEqual(['Host directive reference must be a class']); }); - it('should only throw host directive error once in a chain of directives', () => { + it('should only produce a diagnostic once in a chain of directives', () => { env.write('test.ts', ` import {Directive, Component, NgModule} from '@angular/core'; @@ -544,12 +544,221 @@ runInEachFileSystem(() => { export class Host {} `); - // What we're checking here is that the errors aren't produced recursively. If that were - // the case, the same error would show up more than once in the diagnostics since `HostDirB` - // is in the chain of both `Host` and `HostDirA`. + // What we're checking here is that the diagnostics aren't produced recursively. If that + // were the case, the same diagnostic would show up more than once in the diagnostics since + // `HostDirB` is in the chain of both `Host` and `HostDirA`. const messages = env.driveDiagnostics().map(extractMessage); expect(messages).toEqual(['Host directive HostDirB must be standalone']); }); + + it('should produce a diagnostic if a host directive output does not exist', () => { + env.write('test.ts', ` + import {Directive, Output, EventEmitter} from '@angular/core'; + + @Directive({standalone: true}) + class HostDir { + @Output() foo = new EventEmitter(); + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{ + directive: HostDir, + outputs: ['doesNotExist'], + }] + }) + class Dir {} + `); + + const messages = env.driveDiagnostics().map(extractMessage); + expect(messages).toEqual( + ['Directive HostDir does not have an output with a public name of doesNotExist.']); + }); + + it('should produce a diagnostic if a host directive output alias does not exist', () => { + env.write('test.ts', ` + import {Directive, Output, EventEmitter} from '@angular/core'; + + @Directive({standalone: true}) + class HostDir { + @Output('alias') foo = new EventEmitter(); + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{ + directive: HostDir, + outputs: ['foo'], + }] + }) + class Dir {} + `); + + const messages = env.driveDiagnostics().map(extractMessage); + expect(messages).toEqual( + ['Directive HostDir does not have an output with a public name of foo.']); + }); + + it('should produce a diagnostic if a host directive input does not exist', () => { + env.write('test.ts', ` + import {Directive, Input} from '@angular/core'; + + @Directive({standalone: true}) + class HostDir { + @Input() foo: any; + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{ + directive: HostDir, + inputs: ['doesNotExist'], + }] + }) + class Dir {} + `); + + const messages = env.driveDiagnostics().map(extractMessage); + expect(messages).toEqual( + ['Directive HostDir does not have an input with a public name of doesNotExist.']); + }); + + it('should produce a diagnostic if a host directive input alias does not exist', () => { + env.write('test.ts', ` + import {Directive, Input} from '@angular/core'; + + @Directive({standalone: true}) + class HostDir { + @Input('alias') foo: any; + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{directive: HostDir, inputs: ['foo']}], + }) + class Dir {} + `); + + const messages = env.driveDiagnostics().map(extractMessage); + expect(messages).toEqual( + ['Directive HostDir does not have an input with a public name of foo.']); + }); + + it('should produce a diagnostic if a host directive tries to alias to an existing input', + () => { + env.write('test.ts', ` + import {Directive, Input} from '@angular/core'; + + @Directive({selector: '[host-dir]', standalone: true}) + class HostDir { + @Input('colorAlias') color?: string; + @Input() buttonColor?: string; + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{directive: HostDir, inputs: ['colorAlias: buttonColor']}] + }) + class Dir {} + `); + + const messages = env.driveDiagnostics().map(extractMessage); + expect(messages).toEqual([ + 'Cannot alias input colorAlias of host directive HostDir to buttonColor, because it ' + + 'already has a different input with the same public name.' + ]); + }); + + it('should produce a diagnostic if a host directive tries to alias to an existing input alias', + () => { + env.write('test.ts', ` + import {Directive, Input} from '@angular/core'; + + @Directive({selector: '[host-dir]', standalone: true}) + class HostDir { + @Input('colorAlias') color?: string; + @Input('buttonColorAlias') buttonColor?: string; + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{directive: HostDir, inputs: ['colorAlias: buttonColorAlias']}] + }) + class Dir {} + `); + + const messages = env.driveDiagnostics().map(extractMessage); + expect(messages).toEqual( + ['Cannot alias input colorAlias of host directive HostDir to buttonColorAlias, ' + + 'because it already has a different input with the same public name.']); + }); + + it('should not produce a diagnostic if a host directive input aliases to the same name', + () => { + env.write('test.ts', ` + import {Directive, Input} from '@angular/core'; + + @Directive({selector: '[host-dir]', standalone: true}) + class HostDir { + @Input('color') color?: string; + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{directive: HostDir, inputs: ['color: buttonColor']}] + }) + class Dir {} + `); + + const messages = env.driveDiagnostics().map(extractMessage); + expect(messages).toEqual([]); + }); + + it('should produce a diagnostic if a host directive tries to alias to an existing output alias', + () => { + env.write('test.ts', ` + import {Directive, Output, EventEmitter} from '@angular/core'; + + @Directive({selector: '[host-dir]', standalone: true}) + class HostDir { + @Output('clickedAlias') clicked = new EventEmitter(); + @Output('tappedAlias') tapped = new EventEmitter(); + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{directive: HostDir, outputs: ['clickedAlias: tappedAlias']}] + }) + class Dir {} + `); + + const messages = env.driveDiagnostics().map(extractMessage); + expect(messages).toEqual([ + 'Cannot alias output clickedAlias of host directive HostDir ' + + 'to tappedAlias, because it already has a different output with the same public name.' + ]); + }); + + it('should not produce a diagnostic if a host directive output aliases to the same name', + () => { + env.write('test.ts', ` + import {Directive, Output, EventEmitter} from '@angular/core'; + + @Directive({selector: '[host-dir]', standalone: true}) + class HostDir { + @Output('clicked') clicked = new EventEmitter(); + } + + @Directive({ + selector: '[dir]', + hostDirectives: [{directive: HostDir, outputs: ['clicked: wasClicked']}] + }) + class Dir {} + `); + + const messages = env.driveDiagnostics().map(extractMessage); + expect(messages).toEqual([]); + }); }); }); }); diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index bdf5314a845af6..080a445ba4cc8f 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -3072,52 +3072,6 @@ suppress `Argument of type 'string' is not assignable to parameter of type 'number'.` ]); }); - - it('should check bindings to host directive inputs referring to the private name when there is a different public name', - () => { - env.write('test.ts', ` - import {Component, Directive, NgModule, Input, Output} from '@angular/core'; - - @Directive({ - standalone: true, - }) - class HostDir { - @Input('inputAlias') input: number; - @Output('outputAlias') output: string; - } - - @Directive({ - selector: '[dir]', - hostDirectives: [{directive: HostDir, inputs: ['input'], outputs: ['output']}] - }) - class Dir {} - - @Component({ - selector: 'test', - template: '
', - }) - class TestCmp { - person: { - name: string; - }; - handleStringEvent(event: string): void {} - } - - @NgModule({ - declarations: [TestCmp, Dir], - }) - class Module {} - `); - - const messages = env.driveDiagnostics().map(d => d.messageText); - - // These messages are expected to refer to the native - // typings since the inputs/outputs haven't been exposed. - expect(messages).toEqual([ - `Argument of type 'Event' is not assignable to parameter of type 'string'.`, - `Can't bind to 'input' since it isn't a known property of 'div'.` - ]); - }); }); }); }); diff --git a/packages/core/src/render3/features/host_directives_feature.ts b/packages/core/src/render3/features/host_directives_feature.ts index c9fd4a8df9f3eb..6e3e2afde86710 100644 --- a/packages/core/src/render3/features/host_directives_feature.ts +++ b/packages/core/src/render3/features/host_directives_feature.ts @@ -149,7 +149,6 @@ function patchDeclaredInputs( function validateHostDirective( hostDirectiveConfig: HostDirectiveDef, directiveDef: DirectiveDef|null, matchedDefs: DirectiveDef[]): asserts directiveDef is DirectiveDef { - // TODO(crisbeto): implement more of these checks in the compiler. const type = hostDirectiveConfig.directive; if (directiveDef === null) { @@ -186,16 +185,16 @@ function validateHostDirective( * Checks that the host directive inputs/outputs configuration is valid. * @param bindingType Kind of binding that is being validated. Used in the error message. * @param def Definition of the host directive that is being validated against. - * @param hostDirectiveDefs Host directive mapping object that shold be validated. + * @param hostDirectiveBindings Host directive mapping object that shold be validated. */ function validateMappings( bindingType: 'input'|'output', def: DirectiveDef, - hostDirectiveDefs: HostDirectiveBindingMap) { + hostDirectiveBindings: HostDirectiveBindingMap) { const className = def.type.name; const bindings: Record = bindingType === 'input' ? def.inputs : def.outputs; - for (const publicName in hostDirectiveDefs) { - if (hostDirectiveDefs.hasOwnProperty(publicName)) { + for (const publicName in hostDirectiveBindings) { + if (hostDirectiveBindings.hasOwnProperty(publicName)) { if (!bindings.hasOwnProperty(publicName)) { throw new RuntimeError( RuntimeErrorCode.HOST_DIRECTIVE_UNDEFINED_BINDING, @@ -203,7 +202,7 @@ function validateMappings( publicName}.`); } - const remappedPublicName = hostDirectiveDefs[publicName]; + const remappedPublicName = hostDirectiveBindings[publicName]; if (bindings.hasOwnProperty(remappedPublicName) && bindings[remappedPublicName] !== publicName) {