fix(ivy): don't crash on an unknown localref target (#33454)

Previously the template binder would crash when encountering an unknown
localref (# reference) such as `<div #ref="foo">` when no directive has
`exportAs: "foo"`.

With this commit, the compiler instead generates a template diagnostic error
informing the user about the invalid reference.

PR Close #33454

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

@@ -84,6 +84,11 @@ export enum ErrorCode {
    * An element's attribute name failed validation against the DOM schema.
    */
   SCHEMA_INVALID_ATTRIBUTE = 8002,
+
+  /**
+   * No matching directive was found for a `#ref="target"` expression.
+   */
+  MISSING_REFERENCE_TARGET = 8003,
 }
 
 export function ngErrorCode(code: ErrorCode): number {

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

@@ -19,6 +19,7 @@ import {shouldReportDiagnostic, translateDiagnostic} from './diagnostics';
 import {DomSchemaChecker, RegistryDomSchemaChecker} from './dom';
 import {Environment} from './environment';
 import {TypeCheckProgramHost} from './host';
+import {OutOfBandDiagnosticRecorder, OutOfBandDiagnosticRecorderImpl} from './oob';
 import {TcbSourceManager} from './source';
 import {generateTypeCheckBlock, requiresInlineTypeCheckBlock} from './type_check_block';
 import {TypeCheckFile} from './type_check_file';
@@ -58,6 +59,8 @@ export class TypeCheckContext {
 
   private domSchemaChecker = new RegistryDomSchemaChecker(this.sourceManager);
 
+  private oobRecorder = new OutOfBandDiagnosticRecorderImpl(this.sourceManager);
+
   /**
    * Record a template for the given component `node`, with a `SelectorMatcher` for directive
    * matching.
@@ -102,7 +105,8 @@ export class TypeCheckContext {
       this.addInlineTypeCheckBlock(ref, tcbMetadata);
     } else {
       // The class can be type-checked externally as normal.
-      this.typeCheckFile.addTypeCheckBlock(ref, tcbMetadata, this.domSchemaChecker);
+      this.typeCheckFile.addTypeCheckBlock(
+          ref, tcbMetadata, this.domSchemaChecker, this.oobRecorder);
     }
   }
 
@@ -219,6 +223,7 @@ export class TypeCheckContext {
     }
 
     diagnostics.push(...this.domSchemaChecker.diagnostics);
+    diagnostics.push(...this.oobRecorder.diagnostics);
 
     return {
       diagnostics,
@@ -234,7 +239,7 @@ export class TypeCheckContext {
       this.opMap.set(sf, []);
     }
     const ops = this.opMap.get(sf) !;
-    ops.push(new TcbOp(ref, tcbMeta, this.config, this.domSchemaChecker));
+    ops.push(new TcbOp(ref, tcbMeta, this.config, this.domSchemaChecker, this.oobRecorder));
   }
 }
 
@@ -266,7 +271,8 @@ class TcbOp implements Op {
   constructor(
       readonly ref: Reference<ClassDeclaration<ts.ClassDeclaration>>,
       readonly meta: TypeCheckBlockMetadata, readonly config: TypeCheckingConfig,
-      readonly domSchemaChecker: DomSchemaChecker) {}
+      readonly domSchemaChecker: DomSchemaChecker,
+      readonly oobRecorder: OutOfBandDiagnosticRecorder) {}
 
   /**
    * Type check blocks are inserted immediately after the end of the component class.
@@ -277,7 +283,8 @@ class TcbOp implements Op {
       string {
     const env = new Environment(this.config, im, refEmitter, sf);
     const fnName = ts.createIdentifier(`_tcb_${this.ref.node.pos}`);
-    const fn = generateTypeCheckBlock(env, this.ref, fnName, this.meta, this.domSchemaChecker);
+    const fn = generateTypeCheckBlock(
+        env, this.ref, fnName, this.meta, this.domSchemaChecker, this.oobRecorder);
     return printer.printNode(ts.EmitHint.Unspecified, fn, sf);
   }
 }

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

@@ -0,0 +1,55 @@
+/**
+ * @license
+ * Copyright Google Inc. 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.io/license
+ */
+
+import {TmplAstReference} from '@angular/compiler';
+import * as ts from 'typescript';
+
+import {ErrorCode, ngErrorCode} from '../../diagnostics';
+
+import {TcbSourceResolver, makeTemplateDiagnostic} from './diagnostics';
+
+/**
+ * Collects `ts.Diagnostic`s 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 `ts.Diagnostic`s which are stored by the
+ * recorder for later display.
+ */
+export interface OutOfBandDiagnosticRecorder {
+  readonly diagnostics: ReadonlyArray<ts.Diagnostic>;
+
+  /**
+   * Reports a `#ref="target"` expression in the template for which a target directive could not be
+   * found.
+   *
+   * @param templateId the template type-checking ID of the template which contains the broken
+   * reference.
+   * @param ref the `TmplAstReference` which could not be matched to a directive.
+   */
+  missingReferenceTarget(templateId: string, ref: TmplAstReference): void;
+}
+
+export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder {
+  private _diagnostics: ts.Diagnostic[] = [];
+
+  constructor(private resolver: TcbSourceResolver) {}
+
+  get diagnostics(): ReadonlyArray<ts.Diagnostic> { return this._diagnostics; }
+
+  missingReferenceTarget(templateId: string, ref: TmplAstReference): void {
+    const mapping = this.resolver.getSourceMapping(templateId);
+    const value = ref.value.trim();
+
+    const errorMsg = `No directive found with exportAs '${value}'.`;
+    this._diagnostics.push(makeTemplateDiagnostic(
+        mapping, ref.valueSpan || ref.sourceSpan, ts.DiagnosticCategory.Error,
+        ngErrorCode(ErrorCode.MISSING_REFERENCE_TARGET), errorMsg));
+  }
+}

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

@@ -17,6 +17,7 @@ import {addParseSpanInfo, addSourceId, toAbsoluteSpan, wrapForDiagnostics} from
 import {DomSchemaChecker} from './dom';
 import {Environment} from './environment';
 import {NULL_AS_ANY, astToTypescript} from './expression';
+import {OutOfBandDiagnosticRecorder} from './oob';
 import {checkIfClassIsExported, checkIfGenericTypesAreUnbound, tsCallMethod, tsCastToAny, tsCreateElement, tsCreateVariable, tsDeclareVariable} from './ts_util';
 
 
@@ -28,15 +29,27 @@ import {checkIfClassIsExported, checkIfGenericTypesAreUnbound, tsCallMethod, tsC
  * When passed through TypeScript's TypeChecker, type errors that arise within the type check block
  * function indicate issues in the template itself.
  *
- * @param node the TypeScript node for the component class.
+ * As a side effect of generating a TCB for the component, `ts.Diagnostic`s may also be produced
+ * directly for issues within the template which are identified during generation. These issues are
+ * recorded in either the `domSchemaChecker` (which checks usage of DOM elements and bindings) as
+ * well as the `oobRecorder` (which records errors when the type-checking code generator is unable
+ * to sufficiently understand a template).
+ *
+ * @param env an `Environment` into which type-checking code will be generated.
+ * @param ref a `Reference` to the component class which should be type-checked.
+ * @param name a `ts.Identifier` to use for the generated `ts.FunctionDeclaration`.
  * @param meta metadata about the component's template and the function being generated.
- * @param importManager an `ImportManager` for the file into which the TCB will be written.
+ * @param domSchemaChecker used to check and record errors regarding improper usage of DOM elements
+ * and bindings.
+ * @param oobRecorder used to record errors regarding template elements which could not be correctly
+ * translated into types during TCB generation.
  */
 export function generateTypeCheckBlock(
     env: Environment, ref: Reference<ClassDeclaration<ts.ClassDeclaration>>, name: ts.Identifier,
-    meta: TypeCheckBlockMetadata, domSchemaChecker: DomSchemaChecker): ts.FunctionDeclaration {
+    meta: TypeCheckBlockMetadata, domSchemaChecker: DomSchemaChecker,
-  const tcb =
+    oobRecorder: OutOfBandDiagnosticRecorder): ts.FunctionDeclaration {
-      new Context(env, domSchemaChecker, meta.id, meta.boundTarget, meta.pipes, meta.schemas);
+  const tcb = new Context(
+      env, domSchemaChecker, oobRecorder, meta.id, meta.boundTarget, meta.pipes, meta.schemas);
   const scope = Scope.forNodes(tcb, null, tcb.boundTarget.target.template !);
   const ctxRawType = env.referenceType(ref);
   if (!ts.isTypeReferenceNode(ctxRawType)) {
@@ -562,7 +575,8 @@ export class Context {
   private nextId = 1;
 
   constructor(
-      readonly env: Environment, readonly domSchemaChecker: DomSchemaChecker, readonly id: string,
+      readonly env: Environment, readonly domSchemaChecker: DomSchemaChecker,
+      readonly oobRecorder: OutOfBandDiagnosticRecorder, readonly id: string,
       readonly boundTarget: BoundTarget<TypeCheckableDirectiveMeta>,
       private pipes: Map<string, Reference<ClassDeclaration<ts.ClassDeclaration>>>,
       readonly schemas: SchemaMetadata[]) {}
@@ -797,6 +811,7 @@ class Scope {
       for (const child of node.children) {
         this.appendNode(child);
       }
+      this.checkReferencesOfNode(node);
     } else if (node instanceof TmplAstTemplate) {
       // Template children are rendered in a child scope.
       this.appendDirectivesAndInputsOfNode(node);
@@ -806,11 +821,20 @@ class Scope {
         this.templateCtxOpMap.set(node, ctxIndex);
         this.opQueue.push(new TcbTemplateBodyOp(this.tcb, this, node));
       }
+      this.checkReferencesOfNode(node);
     } else if (node instanceof TmplAstBoundText) {
       this.opQueue.push(new TcbTextInterpolationOp(this.tcb, this, node));
     }
   }
 
+  private checkReferencesOfNode(node: TmplAstElement|TmplAstTemplate): void {
+    for (const ref of node.references) {
+      if (this.tcb.boundTarget.getReferenceTarget(ref) === null) {
+        this.tcb.oobRecorder.missingReferenceTarget(this.tcb.id, ref);
+      }
+    }
+  }
+
   private appendDirectivesAndInputsOfNode(node: TmplAstElement|TmplAstTemplate): void {
     // Collect all the inputs on the element.
     const claimedInputs = new Set<string>();
@@ -1025,7 +1049,10 @@ class TcbExpressionTranslator {
     } else if (binding instanceof TmplAstReference) {
       const target = this.tcb.boundTarget.getReferenceTarget(binding);
       if (target === null) {
-        throw new Error(`Unbound reference? ${binding.name}`);
+        // This reference is unbound. Traversal of the `TmplAstReference` itself should have
+        // recorded the error in the `OutOfBandDiagnosticRecorder`.
+        // Still check the rest of the expression if possible by using an `any` value.
+        return NULL_AS_ANY;
       }
 
       // The reference is either to an element, an <ng-template> node, or to a directive on an

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

@@ -15,9 +15,11 @@ import {ImportManager} from '../../translator';
 import {TypeCheckBlockMetadata, TypeCheckingConfig} from './api';
 import {DomSchemaChecker} from './dom';
 import {Environment} from './environment';
+import {OutOfBandDiagnosticRecorder} from './oob';
 import {generateTypeCheckBlock} from './type_check_block';
 
 
+
 /**
  * An `Environment` representing the single type-checking file into which most (if not all) Type
  * Check Blocks (TCBs) will be generated.
@@ -38,9 +40,9 @@ export class TypeCheckFile extends Environment {
 
   addTypeCheckBlock(
       ref: Reference<ClassDeclaration<ts.ClassDeclaration>>, meta: TypeCheckBlockMetadata,
-      domSchemaChecker: DomSchemaChecker): void {
+      domSchemaChecker: DomSchemaChecker, oobRecorder: OutOfBandDiagnosticRecorder): void {
     const fnId = ts.createIdentifier(`_tcb${this.nextTcbId++}`);
-    const fn = generateTypeCheckBlock(this, ref, fnId, meta, domSchemaChecker);
+    const fn = generateTypeCheckBlock(this, ref, fnId, meta, domSchemaChecker, oobRecorder);
     this.tcbStatements.push(fn);
   }
 

packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {CssSelector, ParseSourceFile, ParseSourceSpan, R3TargetBinder, SchemaMetadata, SelectorMatcher, TmplAstElement, Type, parseTemplate} from '@angular/compiler';
+import {CssSelector, ParseSourceFile, ParseSourceSpan, R3TargetBinder, SchemaMetadata, SelectorMatcher, TmplAstElement, TmplAstReference, Type, parseTemplate} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {AbsoluteFsPath, LogicalFileSystem, absoluteFrom} from '../../file_system';
@@ -19,6 +19,7 @@ import {TemplateSourceMapping, TypeCheckBlockMetadata, TypeCheckableDirectiveMet
 import {TypeCheckContext} from '../src/context';
 import {DomSchemaChecker} from '../src/dom';
 import {Environment} from '../src/environment';
+import {OutOfBandDiagnosticRecorder} from '../src/oob';
 import {generateTypeCheckBlock} from '../src/type_check_block';
 
 export function typescriptLibDts(): TestFile {
@@ -220,7 +221,7 @@ export function tcb(
 
   const tcb = generateTypeCheckBlock(
       FakeEnvironment.newFake(config), new Reference(clazz), ts.createIdentifier('Test_TCB'), meta,
-      new NoopSchemaChecker());
+      new NoopSchemaChecker(), new NoopOobRecorder());
 
   const removeComments = !options.emitSpans;
   const res = ts.createPrinter({removeComments}).printNode(ts.EmitHint.Unspecified, tcb, sf);
@@ -382,3 +383,8 @@ export class NoopSchemaChecker implements DomSchemaChecker {
       id: string, element: TmplAstElement, name: string, span: ParseSourceSpan,
       schemas: SchemaMetadata[]): void {}
 }
+
+export class NoopOobRecorder implements OutOfBandDiagnosticRecorder {
+  get diagnostics(): ReadonlyArray<ts.Diagnostic> { return []; }
+  missingReferenceTarget(): void {}
+}

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

@@ -3137,6 +3137,24 @@ runInEachFileSystem(os => {
       });
     });
 
+    describe('local refs', () => {
+      it('should not generate an error when a local ref is unresolved' +
+             ' (outside of template type-checking)',
+         () => {
+
+           env.write('test.ts', `
+          import {Component} from '@angular/core';
+  
+          @Component({
+            template: '<div #ref="unknownTarget"></div>',
+          })
+          export class TestCmp {}
+        `);
+           const diags = env.driveDiagnostics();
+           expect(diags.length).toBe(0);
+         });
+    });
+
     describe('multiple local refs', () => {
       const getComponentScript = (template: string): string => `
       import {Component, Directive, NgModule} from '@angular/core';

packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts

@@ -714,6 +714,27 @@ export declare class AnimationEvent {
       env.driveMain();
     });
 
+    it('should report an error with an unknown local ref target', () => {
+      env.write('test.ts', `
+        import {Component, NgModule} from '@angular/core';
+
+        @Component({
+          selector: 'test',
+          template: '<div #ref="unknownTarget"></div>',
+        })
+        class TestCmp {}
+
+        @NgModule({
+          declarations: [TestCmp],
+        })
+        class Module {}
+      `);
+      const diags = env.driveDiagnostics();
+      expect(diags.length).toBe(1);
+      expect(diags[0].messageText).toBe(`No directive found with exportAs 'unknownTarget'.`);
+      expect(getSourceCodeForDiagnostic(diags[0])).toBe('unknownTarget');
+    });
+
     it('should report an error with pipe bindings', () => {
       env.write('test.ts', `
     import {CommonModule} from '@angular/common';

packages/compiler/src/render3/view/t2_binder.ts

@@ -260,16 +260,16 @@ class DirectiveBinder<DirectiveT extends DirectiveMeta> implements Visitor {
         // This could be a reference to a component if there is one.
         dirTarget = directives.find(dir => dir.isComponent) || null;
       } else {
-        // This is a reference to a directive exported via exportAs. One should exist.
+        // This should be a reference to a directive exported via exportAs.
         dirTarget =
             directives.find(
                 dir => dir.exportAs !== null && dir.exportAs.some(value => value === ref.value)) ||
             null;
-
+        // Check if a matching directive was found.
-        // Check if a matching directive was found, and error if it wasn't.
         if (dirTarget === null) {
-          // TODO(alxhub): Return an error value here that can be used for template validation.
+          // No matching directive was found - this reference points to an unknown target. Leave it
-          throw new Error(`Assertion error: failed to find directive with exportAs: ${ref.value}`);
+          // unmapped.
+          return;
         }
       }