refactor(compiler-cli): generalize out of band diagnostics recorder

Currently the `OutOfBandDiagnosticRecorder` is tied to producing TypeScript diagnostics, however some of our use cases might call for a different form.

These changes decouple the recorder from TypeScript and make the diagnostic type generic.

Author: crisbeto

packages/compiler-cli/private/hybrid_analysis.ts

@@ -18,10 +18,11 @@ export type {
   TemplateDiagnostic,
   TcbReferenceMetadata,
   SourceMapping,
+  OutOfBandDiagnosticRecorder,
+  OutOfBadDiagnosticCategory,
 } from '../src/ngtsc/typecheck/api';
 export {DomSchemaChecker, RegistryDomSchemaChecker} from '../src/ngtsc/typecheck/src/dom';
 export {Environment} from '../src/ngtsc/typecheck/src/environment';
-export {OutOfBandDiagnosticRecorder} from '../src/ngtsc/typecheck/src/oob';
 export {TcbGenericContextBehavior} from '../src/ngtsc/typecheck/src/ops/context';
 export {ImportManager} from '../src/ngtsc/translator';
 export type {ReferenceEmitter} from '../src/ngtsc/imports';

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

@@ -12,3 +12,4 @@ export * from './completion';
 export * from './context';
 export * from './scope';
 export * from './symbols';
+export * from './oob';

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

@@ -0,0 +1,247 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.dev/license
+ */
+
+import {
+  AST,
+  BindingPipe,
+  PropertyRead,
+  TmplAstBoundAttribute,
+  TmplAstBoundEvent,
+  TmplAstComponent,
+  TmplAstDirective,
+  TmplAstElement,
+  TmplAstForLoopBlock,
+  TmplAstForLoopBlockEmpty,
+  TmplAstHoverDeferredTrigger,
+  TmplAstIfBlockBranch,
+  TmplAstInteractionDeferredTrigger,
+  TmplAstLetDeclaration,
+  TmplAstReference,
+  TmplAstSwitchBlockCase,
+  TmplAstTemplate,
+  TmplAstTextAttribute,
+  TmplAstVariable,
+  TmplAstViewportDeferredTrigger,
+} from '@angular/compiler';
+
+import {TcbDirectiveMetadata, TypeCheckId} from './api';
+
+/** Categories of diagnostics that can be reported by a `OutOfBandDiagnosticRecorder`. */
+export enum OutOfBadDiagnosticCategory {
+  Error,
+  Warning,
+}
+
+/**
+ * Collects diagnostics on problems which occur in the template which aren't directly sourced
+ * from type check blocks.
+ *
+ * During the creation of a type check block, the template is traversed and the
+ * `OutOfBandDiagnosticRecorder` is called to record cases when a correct interpretation for the
+ * template cannot be found. These operations create diagnostics which are stored by the
+ * recorder for later display.
+ */
+export interface OutOfBandDiagnosticRecorder<T> {
+  readonly diagnostics: ReadonlyArray<T>;
+
+  /**
+   * Reports a `#ref="target"` expression in the template for which a target directive could not be
+   * found.
+   *
+   * @param id the type-checking ID of the template which contains the broken reference.
+   * @param ref the `TmplAstReference` which could not be matched to a directive.
+   */
+  missingReferenceTarget(id: TypeCheckId, ref: TmplAstReference): void;
+
+  /**
+   * Reports usage of a `| pipe` expression in the template for which the named pipe could not be
+   * found.
+   *
+   * @param id the type-checking ID of the template which contains the unknown pipe.
+   * @param ast the `BindingPipe` invocation of the pipe which could not be found.
+   * @param isStandalone whether the host component is standalone.
+   */
+  missingPipe(id: TypeCheckId, ast: BindingPipe, isStandalone: boolean): void;
+
+  /**
+   * Reports usage of a pipe imported via `@Component.deferredImports` outside
+   * of a `@defer` block in a template.
+   *
+   * @param id the type-checking ID of the template which contains the unknown pipe.
+   * @param ast the `BindingPipe` invocation of the pipe which could not be found.
+   */
+  deferredPipeUsedEagerly(id: TypeCheckId, ast: BindingPipe): void;
+
+  /**
+   * Reports usage of a component/directive imported via `@Component.deferredImports` outside
+   * of a `@defer` block in a template.
+   *
+   * @param id the type-checking ID of the template which contains the unknown pipe.
+   * @param element the element which hosts a component that was defer-loaded.
+   */
+  deferredComponentUsedEagerly(id: TypeCheckId, element: TmplAstElement): void;
+
+  /**
+   * Reports a duplicate declaration of a template variable.
+   *
+   * @param id the type-checking ID of the template which contains the duplicate
+   * declaration.
+   * @param variable the `TmplAstVariable` which duplicates a previously declared variable.
+   * @param firstDecl the first variable declaration which uses the same name as `variable`.
+   */
+  duplicateTemplateVar(
+    id: TypeCheckId,
+    variable: TmplAstVariable,
+    firstDecl: TmplAstVariable,
+  ): void;
+
+  /**
+   * Report a warning when structural directives support context guards, but the current
+   * type-checking configuration prohibits their usage.
+   */
+  suboptimalTypeInference(id: TypeCheckId, variables: TmplAstVariable[]): void;
+
+  /**
+   * Reports a split two way binding error message.
+   */
+  splitTwoWayBinding(
+    id: TypeCheckId,
+    input: TmplAstBoundAttribute,
+    output: TmplAstBoundEvent,
+    inputConsumer: Pick<TcbDirectiveMetadata, 'name' | 'isComponent' | 'ref'>,
+    outputConsumer: Pick<TcbDirectiveMetadata, 'name' | 'isComponent' | 'ref'> | TmplAstElement,
+  ): void;
+
+  /** Reports required inputs that haven't been bound. */
+  missingRequiredInputs(
+    id: TypeCheckId,
+    element: TmplAstElement | TmplAstTemplate | TmplAstComponent | TmplAstDirective,
+    directiveName: string,
+    isComponent: boolean,
+    inputAliases: string[],
+  ): void;
+
+  /**
+   * Reports accesses of properties that aren't available in a `for` block's tracking expression.
+   */
+  illegalForLoopTrackAccess(
+    id: TypeCheckId,
+    block: TmplAstForLoopBlock,
+    access: PropertyRead,
+  ): void;
+
+  /**
+   * Reports deferred triggers that cannot access the element they're referring to.
+   */
+  inaccessibleDeferredTriggerElement(
+    id: TypeCheckId,
+    trigger:
+      | TmplAstHoverDeferredTrigger
+      | TmplAstInteractionDeferredTrigger
+      | TmplAstViewportDeferredTrigger,
+  ): void;
+
+  /**
+   * Reports cases where control flow nodes prevent content projection.
+   */
+  controlFlowPreventingContentProjection(
+    id: TypeCheckId,
+    category: OutOfBadDiagnosticCategory,
+    projectionNode: TmplAstElement | TmplAstTemplate,
+    componentName: string,
+    slotSelector: string,
+    controlFlowNode:
+      | TmplAstIfBlockBranch
+      | TmplAstSwitchBlockCase
+      | TmplAstForLoopBlock
+      | TmplAstForLoopBlockEmpty,
+    preservesWhitespaces: boolean,
+  ): void;
+
+  /** Reports cases where users are writing to `@let` declarations. */
+  illegalWriteToLetDeclaration(id: TypeCheckId, node: AST, target: TmplAstLetDeclaration): void;
+
+  /** Reports cases where users are accessing an `@let` before it is defined.. */
+  letUsedBeforeDefinition(id: TypeCheckId, node: PropertyRead, target: TmplAstLetDeclaration): void;
+
+  /**
+   * Reports a `@let` declaration that conflicts with another symbol in the same scope.
+   *
+   * @param id the type-checking ID of the template which contains the declaration.
+   * @param current the `TmplAstLetDeclaration` which is invalid.
+   */
+  conflictingDeclaration(id: TypeCheckId, current: TmplAstLetDeclaration): void;
+
+  /**
+   * Reports that a named template dependency (e.g. `<Missing/>`) is not available.
+   * @param id Type checking ID of the template in which the dependency is declared.
+   * @param node Node that declares the dependency.
+   */
+  missingNamedTemplateDependency(id: TypeCheckId, node: TmplAstComponent | TmplAstDirective): void;
+
+  /**
+   * Reports that a templace dependency of the wrong kind has been referenced at a specific position
+   * (e.g. `<SomeDirective/>`).
+   * @param id Type checking ID of the template in which the dependency is declared.
+   * @param node Node that declares the dependency.
+   */
+  incorrectTemplateDependencyType(id: TypeCheckId, node: TmplAstComponent | TmplAstDirective): void;
+
+  /**
+   * Reports a binding inside directive syntax that does not match any of the inputs/outputs of
+   * the directive.
+   * @param id Type checking ID of the template in which the directive was defined.
+   * @param directive Directive that contains the binding.
+   * @param node Node declaring the binding.
+   */
+  unclaimedDirectiveBinding(
+    id: TypeCheckId,
+    directive: TmplAstDirective,
+    node: TmplAstBoundAttribute | TmplAstTextAttribute | TmplAstBoundEvent,
+  ): void;
+
+  /**
+   * Reports that an implicit deferred trigger is set on a block that does not have a placeholder.
+   */
+  deferImplicitTriggerMissingPlaceholder(
+    id: TypeCheckId,
+    trigger:
+      | TmplAstHoverDeferredTrigger
+      | TmplAstInteractionDeferredTrigger
+      | TmplAstViewportDeferredTrigger,
+  ): void;
+
+  /**
+   * Reports that an implicit deferred trigger is set on a block whose placeholder is not set up
+   * correctly (e.g. more than one root node).
+   */
+  deferImplicitTriggerInvalidPlaceholder(
+    id: TypeCheckId,
+    trigger:
+      | TmplAstHoverDeferredTrigger
+      | TmplAstInteractionDeferredTrigger
+      | TmplAstViewportDeferredTrigger,
+  ): void;
+
+  /**
+   * Reports an unsupported binding on a form `FormField` node.
+   */
+  formFieldUnsupportedBinding(
+    id: TypeCheckId,
+    node: TmplAstBoundAttribute | TmplAstTextAttribute,
+  ): void;
+
+  /**
+   * Reports that multiple components in the compilation scope match a given element.
+   */
+  multipleMatchingComponents(
+    id: TypeCheckId,
+    element: TmplAstElement,
+    componentNames: string[],
+  ): void;
+}

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

@@ -18,7 +18,7 @@ import {
 import MagicString from 'magic-string';
 import ts from 'typescript';
 
-import {ErrorCode, ngErrorCode} from '../../../../src/ngtsc/diagnostics';
+import {ErrorCode, makeDiagnostic, ngErrorCode} from '../../../../src/ngtsc/diagnostics';
 import {absoluteFromSourceFile, AbsoluteFsPath} from '../../file_system';
 import {Reference, ReferenceEmitter} from '../../imports';
 import {PerfEvent, PerfRecorder} from '../../perf';
@@ -36,13 +36,14 @@ import {
   TypeCheckingConfig,
   TypeCtorMetadata,
   TemplateContext,
+  OutOfBandDiagnosticRecorder,
 } from '../api';
 import {makeTemplateDiagnostic} from '../diagnostics';
 
 import {adaptTypeCheckBlockMetadata} from './tcb_adapter';
 import {DomSchemaChecker, RegistryDomSchemaChecker} from './dom';
 import {Environment} from './environment';
-import {OutOfBandDiagnosticRecorder, OutOfBandDiagnosticRecorderImpl} from './oob';
+import {OutOfBandDiagnosticRecorderImpl} from './oob';
 import {ReferenceEmitEnvironment} from './reference_emit_environment';
 import {TypeCheckShimGenerator} from './shim';
 import {DirectiveSourceManager} from './source';
@@ -128,7 +129,7 @@ export interface PendingShimData {
   /**
    * Recorder for out-of-band diagnostics which are raised during generation.
    */
-  oobRecorder: OutOfBandDiagnosticRecorder;
+  oobRecorder: OutOfBandDiagnosticRecorder<TemplateDiagnostic>;
 
   /**
    * The `DomSchemaChecker` in use for this template, which records any schema-related diagnostics.
@@ -144,6 +145,11 @@ export interface PendingShimData {
    * Map of `TypeCheckId` to information collected about the template as it's ingested.
    */
   data: Map<TypeCheckId, TypeCheckData>;
+
+  /**
+   * Diagnostics produced during shim creation.
+   */
+  shimDiagnostics: TemplateDiagnostic[] | null;
 }
 
 /**
@@ -335,7 +341,16 @@ export class TypeCheckContextImpl implements TypeCheckContext {
       // and inlining would be required.
 
       // Record diagnostics to indicate the issues with this template.
-      shimData.oobRecorder.requiresInlineTcb(id, ref.node);
+      shimData.shimDiagnostics ??= [];
+      shimData.shimDiagnostics.push({
+        ...makeDiagnostic(
+          ErrorCode.INLINE_TCB_REQUIRED,
+          ref.node.name,
+          `This component requires inline template type-checking, which is not supported by the current environment.`,
+        ),
+        sourceFile: ref.node.getSourceFile(),
+        typeCheckId: id,
+      });
 
       // Checking this template would be unsupported, so don't try.
       this.perf.eventCount(PerfEvent.SkipGenerateTcbNoInline);
@@ -516,11 +531,17 @@ export class TypeCheckContextImpl implements TypeCheckContext {
     for (const [sfPath, pendingFileData] of this.fileMap) {
       // For each input file, consider generation operations for each of its shims.
       for (const pendingShimData of pendingFileData.shimData.values()) {
+        const genesisDiagnostics = [
+          ...pendingShimData.domSchemaChecker.diagnostics,
+          ...pendingShimData.oobRecorder.diagnostics,
+        ];
+
+        if (pendingShimData.shimDiagnostics !== null) {
+          genesisDiagnostics.unshift(...pendingShimData.shimDiagnostics);
+        }
+
         this.host.recordShimData(sfPath, {
-          genesisDiagnostics: [
-            ...pendingShimData.domSchemaChecker.diagnostics,
-            ...pendingShimData.oobRecorder.diagnostics,
-          ],
+          genesisDiagnostics,
           hasInlines: pendingFileData.hasInlines,
           path: pendingShimData.file.fileName,
           data: pendingShimData.data,
@@ -579,6 +600,7 @@ export class TypeCheckContextImpl implements TypeCheckContext {
           this.compilerHost,
         ),
         data: new Map<TypeCheckId, TypeCheckData>(),
+        shimDiagnostics: null,
       });
     }
     return fileData.shimData.get(shimPath)!;
@@ -657,7 +679,7 @@ class InlineTcbOp implements Op {
     readonly config: TypeCheckingConfig,
     readonly reflector: ReflectionHost,
     readonly domSchemaChecker: DomSchemaChecker,
-    readonly oobRecorder: OutOfBandDiagnosticRecorder,
+    readonly oobRecorder: OutOfBandDiagnosticRecorder<unknown>,
   ) {}
 
   /**