Skip to content

Commit

Permalink
fix(compiler-cli): implement more host directive validations as diagn…
Browse files Browse the repository at this point in the history
…ostics

Implements more of the runtime validations for host directives as compiler diagnostics so that they can be caught earlier. Also does some minor cleanup.
  • Loading branch information
crisbeto committed Oct 14, 2022
1 parent d684148 commit b076d7a
Show file tree
Hide file tree
Showing 5 changed files with 277 additions and 65 deletions.
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down
9 changes: 9 additions & 0 deletions packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts
Expand Up @@ -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
Expand Down
231 changes: 220 additions & 11 deletions packages/compiler-cli/test/ngtsc/host_directives_spec.ts
Expand Up @@ -357,8 +357,8 @@ runInEachFileSystem(() => {
import {ɵɵDirectiveDeclaration} from '@angular/core';
export declare class ExternalDir {
static ɵdir: ɵɵDirectiveDeclaration<ExternalDir, '[test]', never, never,
{input: "input"}, {output: "output"}, never, true, never>;
static ɵdir: ɵɵDirectiveDeclaration<ExternalDir, '[test]', never,
{input: "input"}, {output: "output"}, never, never, true, never>;
}
`);

Expand Down Expand Up @@ -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';
Expand All @@ -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';
Expand All @@ -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';
Expand All @@ -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';
Expand All @@ -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';
Expand All @@ -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';
Expand All @@ -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([]);
});
});
});
});

0 comments on commit b076d7a

Please sign in to comment.