fix(compiler-cli): implement more host directive validations as diagnostics (#47768)

crisbeto · dylhunn · commit f97bebf17af2 · 2022-10-17T12:12:22.000+02:00
Implements more of the runtime validations for host directives as compiler diagnostics so that they can be caught earlier. Also does some minor cleanup. PR Close #47768
diff --git a/goldens/public-api/compiler-cli/error_code.md b/goldens/public-api/compiler-cli/error_code.md
@@ -38,8 +38,10 @@ export enum ErrorCode {
     DUPLICATE_VARIABLE_DECLARATION = 8006,
     HOST_BINDING_PARSE_ERROR = 5001,
     HOST_DIRECTIVE_COMPONENT = 2015,
+    HOST_DIRECTIVE_CONFLICTING_ALIAS = 2018,
     HOST_DIRECTIVE_INVALID = 2013,
     HOST_DIRECTIVE_NOT_STANDALONE = 2014,
+    HOST_DIRECTIVE_UNDEFINED_BINDING = 2017,
     IMPORT_CYCLE_DETECTED = 3003,
     IMPORT_GENERATION_FAILURE = 3004,
     INJECTABLE_DUPLICATE_PROV = 9001,
diff --git 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
@@ -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
@@ -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>;
         }
       `);
 
@@ -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
diff --git a/packages/core/src/render3/features/host_directives_feature.ts b/packages/core/src/render3/features/host_directives_feature.ts
