refactor(compiler-cli): merge duplicate directive matches and report conflicts

Implements the logic at the compiler level that will de-duplicate host directives and merge them together. It will also report if a conflict is detected during merging.

Author: crisbeto

goldens/public-api/compiler-cli/error_code.api.md

@@ -28,6 +28,7 @@ export enum ErrorCode {
     CONFIG_FLAT_MODULE_NO_INDEX = 4001,
     // (undocumented)
     CONFIG_STRICT_TEMPLATES_IMPLIES_FULL_TEMPLATE_TYPECHECK = 4002,
+    CONFLICTING_HOST_DIRECTIVE_BINDING = 8024,
     CONFLICTING_INPUT_TRANSFORM = 2020,
     CONFLICTING_LET_DECLARATION = 8017,
     CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION = 8011,

packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts

@@ -128,6 +128,9 @@ runInEachFileSystem(() => {
       const analysis = analyzeDirective(program, 'TestDir');
       const matcher = new SelectorMatcher<T2DirectiveMeta[]>();
       const dirMeta: T2DirectiveMeta = {
+        ref: {
+          key: 'TestDir',
+        },
         exportAs: null,
         inputs: analysis.inputs,
         outputs: analysis.outputs,

packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts

@@ -449,6 +449,11 @@ export enum ErrorCode {
    */
   MULTIPLE_MATCHING_COMPONENTS = 8023,
 
+  /**
+   * Raised when a host directive input/output is exposed multiple times under the same name.
+   */
+  CONFLICTING_HOST_DIRECTIVE_BINDING = 8024,
+
   /**
    * A two way binding in a template has an incorrect syntax,
    * parentheses outside brackets. For example:

packages/compiler-cli/src/ngtsc/imports/src/references.ts

@@ -82,7 +82,9 @@ export class Reference<T extends ts.Node = ts.Node> {
     if (sourceFile) {
       this.key = `${sourceFile.fileName}#${node.getStart()}`;
     } else {
-      this.key = `${this.bestGuessOwningModule?.specifier}#${id?.text}#${id?.getStart()}#${id?.getEnd()}`;
+      // `getStart` will throw if there's no source file.
+      const position = id && id.getSourceFile() ? id.getStart() : null;
+      this.key = `${this.bestGuessOwningModule?.specifier}#${id?.text}#${position}`;
     }
   }
 

packages/compiler-cli/src/ngtsc/indexer/test/util.ts

@@ -8,6 +8,7 @@
 
 import {
   BoundTarget,
+  ClassPropertyMapping,
   CssSelector,
   DirectiveMatcher,
   MatchSource,
@@ -21,7 +22,6 @@ import ts from 'typescript';
 
 import {absoluteFrom, AbsoluteFsPath} from '../../file_system';
 import {Reference} from '../../imports';
-import {ClassPropertyMapping} from '../../metadata';
 import {ClassDeclaration} from '../../reflection';
 import {getDeclaration, makeProgram} from '../../testing';
 import {ComponentMeta} from '../src/context';

packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts

@@ -10,11 +10,9 @@ import ts from 'typescript';
 
 import {Reference, ReferenceEmitter} from '../../imports';
 import {
-  ClassPropertyMapping,
   CompoundMetadataRegistry,
   DirectiveMeta,
   LocalMetadataRegistry,
-  MatchSource,
   MetadataRegistry,
   MetaKind,
   PipeMeta,
@@ -23,6 +21,7 @@ import {ClassDeclaration} from '../../reflection';
 import {LocalModuleScope, ScopeData} from '../src/api';
 import {DtsModuleScopeResolver} from '../src/dependency';
 import {LocalModuleScopeRegistry} from '../src/local';
+import {ClassPropertyMapping, MatchSource} from '@angular/compiler';
 
 function registerFakeRefs(registry: MetadataRegistry): {
   [name: string]: Reference<ClassDeclaration>;

packages/compiler-cli/src/ngtsc/typecheck/api/oob.ts

@@ -244,4 +244,22 @@ export interface OutOfBandDiagnosticRecorder<T> {
     element: TmplAstElement,
     componentNames: string[],
   ): void;
+
+  /**
+   * Reports that a host directive input/output has been exposed under multiple names.
+   * @param id Type checking ID of the template in which the host directive was used.
+   * @param node Node on which the host directive was used.
+   * @param directiveName Name of the host directive.
+   * @param kind Type of the conflicting binding.
+   * @param classPropertyName Name of the class member that declares the input/output.
+   * @param aliases Aliases under which the binding is exposed.
+   */
+  conflictingHostDirectiveBinding(
+    id: TypeCheckId,
+    node: TmplAstElement | TmplAstTemplate | TmplAstComponent | TmplAstDirective,
+    directiveName: string,
+    kind: 'input' | 'output',
+    classPropertyName: string,
+    aliases: string[],
+  ): void;
 }

packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts

@@ -347,33 +347,11 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
       isComponent ? 'component' : 'directive'
     } ${directiveName} must be specified.`;
 
-    let span: ParseSourceSpan;
-    let name: string | null;
-
-    if (element instanceof TmplAstElement || element instanceof TmplAstDirective) {
-      name = element.name;
-    } else if (element instanceof TmplAstComponent) {
-      name = element.componentName;
-    } else {
-      name = null;
-    }
-
-    if (name === null) {
-      span = element.startSourceSpan;
-    } else {
-      // Only highlight the tag name since highlighting the entire start tag can be noisy.
-      const start = element.startSourceSpan.start.moveBy(1);
-      const end = element.startSourceSpan.end.moveBy(
-        start.offset + name.length - element.startSourceSpan.end.offset,
-      );
-      span = new ParseSourceSpan(start, end);
-    }
-
     this._diagnostics.push(
       makeTemplateDiagnostic(
         id,
         this.resolver.getTemplateSourceMapping(id),
-        span,
+        this.getTagNameSpan(element),
         ts.DiagnosticCategory.Error,
         ngErrorCode(ErrorCode.MISSING_REQUIRED_INPUTS),
         message,
@@ -694,6 +672,59 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
       ),
     );
   }
+
+  conflictingHostDirectiveBinding(
+    id: TypeCheckId,
+    node: TmplAstElement | TmplAstTemplate | TmplAstComponent | TmplAstDirective,
+    directiveName: string,
+    kind: 'input' | 'output',
+    classPropertyName: string,
+    aliases: string[],
+  ): void {
+    const message =
+      `${kind === 'input' ? 'Input' : 'Output'} declared in ${directiveName}.${classPropertyName} ` +
+      `is exposed under the following conflicting names: ${aliases.map((a) => `"${a}"`).join(', ')}. ` +
+      `An ${kind} can only be exposed under a single name.`;
+
+    this._diagnostics.push(
+      makeTemplateDiagnostic(
+        id,
+        this.resolver.getTemplateSourceMapping(id),
+        this.getTagNameSpan(node),
+        ts.DiagnosticCategory.Error,
+        ngErrorCode(ErrorCode.CONFLICTING_HOST_DIRECTIVE_BINDING),
+        message,
+      ),
+    );
+  }
+
+  private getTagNameSpan(
+    node: TmplAstElement | TmplAstTemplate | TmplAstComponent | TmplAstDirective,
+  ) {
+    let span: ParseSourceSpan;
+    let name: string | null;
+
+    if (node instanceof TmplAstElement || node instanceof TmplAstDirective) {
+      name = node.name;
+    } else if (node instanceof TmplAstComponent) {
+      name = node.componentName;
+    } else {
+      name = null;
+    }
+
+    if (name === null) {
+      span = node.startSourceSpan;
+    } else {
+      // Only highlight the tag name since highlighting the entire start tag can be noisy.
+      const start = node.startSourceSpan.start.moveBy(1);
+      const end = node.startSourceSpan.end.moveBy(
+        start.offset + name.length - node.startSourceSpan.end.offset,
+      );
+      span = new ParseSourceSpan(start, end);
+    }
+
+    return span;
+  }
 }
 
 function translateCategory(category: OutOfBadDiagnosticCategory): ts.DiagnosticCategory {

packages/compiler-cli/src/ngtsc/typecheck/src/ops/scope.ts

@@ -579,6 +579,8 @@ export class Scope {
       return;
     }
 
+    this.reportConflictingBindings(node);
+
     if (node instanceof TmplAstElement) {
       const isDeferred = this.tcb.boundTarget.isDeferred(node);
       if (!isDeferred && directives.some((dirMeta) => dirMeta.isExplicitlyDeferred)) {
@@ -688,6 +690,8 @@ export class Scope {
       this.directiveOpMap.set(node, dirMap);
     }
 
+    this.reportConflictingBindings(node);
+
     // In selectorless all directive inputs have to be claimed.
     if (node instanceof TmplAstDirective) {
       for (const input of node.inputs) {
@@ -1043,4 +1047,23 @@ export class Scope {
       );
     }
   }
+
+  private reportConflictingBindings(
+    node: TmplAstElement | TmplAstTemplate | TmplAstComponent | TmplAstDirective,
+  ): void {
+    const conflictingBindings = this.tcb.boundTarget.getConflictingHostDirectiveBindings(node);
+
+    if (conflictingBindings !== null) {
+      for (const binding of conflictingBindings) {
+        this.tcb.oobRecorder.conflictingHostDirectiveBinding(
+          this.tcb.id,
+          node,
+          binding.directive.name,
+          binding.kind,
+          binding.classPropertyName,
+          Array.from(binding.conflictingAliases),
+        );
+      }
+    }
+  }
 }

packages/compiler-cli/src/ngtsc/typecheck/src/tcb_adapter.ts

@@ -30,6 +30,7 @@ import {
   TmplAstTemplate,
   WrappedNodeExpr,
   ClassPropertyMapping,
+  ConflictingHostDirectiveBinding,
 } from '@angular/compiler';
 import {InputMapping} from '../../metadata';
 import {requiresInlineTypeCtor} from './type_constructor';
@@ -176,6 +177,10 @@ export function adaptTypeCheckBlockMetadata(
     getEntitiesInScope: (node) => meta.boundTarget.getEntitiesInScope(node),
     getEagerlyUsedPipes: () => meta.boundTarget.getEagerlyUsedPipes(),
     getDeferBlocks: () => meta.boundTarget.getDeferBlocks(),
+    getConflictingHostDirectiveBindings: (node) =>
+      meta.boundTarget.getConflictingHostDirectiveBindings(node) as
+        | ConflictingHostDirectiveBinding<TcbDirectiveMetadata>[]
+        | null,
   };
 
   const pipes = new Map<string, TcbPipeMetadata>();