From 167bc0d1638ffd6fe91bcb40f96c2ab90f3e01cb Mon Sep 17 00:00:00 2001
From: Andrew Scott <atscott@google.com>
Date: Thu, 1 Dec 2022 11:42:33 -0800
Subject: [PATCH] fix(compiler-cli): Produce diagnostic rather than crash when
 using invalid hostDirective (#48314)

Because the language service uses the compiler, we try to produce as
much useful information as possible rather than throwing hard errors.
Hard errors cause the compiler to crash. While this can be acceptable
when compiling a program as part of a regular build, this is undesirable
for the language service.

PR Close #48314
---
 .../metadata/src/host_directives_resolver.ts   |  7 ++-----
 .../src/ngtsc/metadata/src/inheritance.ts      |  4 ++--
 .../src/ngtsc/scope/src/typecheck.ts           |  8 +++++++-
 .../test/ngtsc/host_directives_spec.ts         | 18 ++++++++++++++++++
 4 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/host_directives_resolver.ts b/packages/compiler-cli/src/ngtsc/metadata/src/host_directives_resolver.ts
index 16a8929373a53..eebe3a2763c93 100644
--- a/packages/compiler-cli/src/ngtsc/metadata/src/host_directives_resolver.ts
+++ b/packages/compiler-cli/src/ngtsc/metadata/src/host_directives_resolver.ts
@@ -43,12 +43,9 @@ export class HostDirectivesResolver {
     for (const current of directives) {
       const hostMeta = flattenInheritedDirectiveMetadata(this.metaReader, current.directive);
 
-      // This case has been checked for already, but we keep the assertion here so that the
-      // user gets a better error message than "Cannot read property foo of null" in case
-      // something slipped through.
+      // This case has been checked for already and produces a diagnostic
       if (hostMeta === null) {
-        throw new Error(
-            `Could not resolve host directive metadata of ${current.directive.debugName}`);
+        continue;
       }
 
       if (hostMeta.hostDirectives) {
diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts b/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts
index 097e1ef422515..9ed774f1d7ff7 100644
--- a/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts
+++ b/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts
@@ -21,10 +21,10 @@ import {ClassPropertyMapping, ClassPropertyName} from './property_mapping';
  * followed.
  */
 export function flattenInheritedDirectiveMetadata(
-    reader: MetadataReader, dir: Reference<ClassDeclaration>): DirectiveMeta {
+    reader: MetadataReader, dir: Reference<ClassDeclaration>): DirectiveMeta|null {
   const topMeta = reader.getDirectiveMetadata(dir);
   if (topMeta === null) {
-    throw new Error(`Metadata not found for directive: ${dir.debugName}`);
+    return null;
   }
   if (topMeta.baseClass === null) {
     return topMeta;
diff --git a/packages/compiler-cli/src/ngtsc/scope/src/typecheck.ts b/packages/compiler-cli/src/ngtsc/scope/src/typecheck.ts
index 5cab1f9beb353..ace986ea88da5 100644
--- a/packages/compiler-cli/src/ngtsc/scope/src/typecheck.ts
+++ b/packages/compiler-cli/src/ngtsc/scope/src/typecheck.ts
@@ -99,6 +99,9 @@ export class TypeCheckScopeRegistry {
     for (const meta of dependencies) {
       if (meta.kind === MetaKind.Directive && meta.selector !== null) {
         const extMeta = this.getTypeCheckDirectiveMetadata(meta.ref);
+        if (extMeta === null) {
+          continue;
+        }
         matcher.addSelectables(
             CssSelector.parse(meta.selector),
             [...this.hostDirectivesResolver.resolve(extMeta), extMeta]);
@@ -125,13 +128,16 @@ export class TypeCheckScopeRegistry {
     return typeCheckScope;
   }
 
-  getTypeCheckDirectiveMetadata(ref: Reference<ClassDeclaration>): DirectiveMeta {
+  getTypeCheckDirectiveMetadata(ref: Reference<ClassDeclaration>): DirectiveMeta|null {
     const clazz = ref.node;
     if (this.flattenedDirectiveMetaCache.has(clazz)) {
       return this.flattenedDirectiveMetaCache.get(clazz)!;
     }
 
     const meta = flattenInheritedDirectiveMetadata(this.metaReader, ref);
+    if (meta === null) {
+      return null;
+    }
     this.flattenedDirectiveMetaCache.set(clazz, meta);
     return meta;
   }
diff --git a/packages/compiler-cli/test/ngtsc/host_directives_spec.ts b/packages/compiler-cli/test/ngtsc/host_directives_spec.ts
index cbade7b9e2aa6..95beaf66dbf64 100644
--- a/packages/compiler-cli/test/ngtsc/host_directives_spec.ts
+++ b/packages/compiler-cli/test/ngtsc/host_directives_spec.ts
@@ -451,6 +451,24 @@ runInEachFileSystem(() => {
         expect(messages).toEqual(['Host directive HostDir must be standalone']);
       });
 
+      it('should produce a diagnostic if a host directive is not a directive', () => {
+        env.write('test.ts', `
+          import {Directive, Pipe, Component, NgModule} from '@angular/core';
+
+          @Pipe({name: 'hostDir'})
+          export class HostDir {}
+
+          @Directive({
+            hostDirectives: [HostDir],
+          })
+          export class Dir {}
+        `);
+
+        const messages = env.driveDiagnostics().map(extractMessage);
+        expect(messages).toEqual(
+            ['HostDir must be a standalone directive to be used as a host directive']);
+      });
+
       it('should produce a diagnostic if a host directive is a component', () => {
         env.write('test.ts', `
           import {Directive, Component, NgModule} from '@angular/core';