From 6e5cfd2cd232305e4d5441fa45b4192d18174b01 Mon Sep 17 00:00:00 2001
From: Andrew Kushnir <akushnir@google.com>
Date: Wed, 15 Jan 2020 14:46:49 -0800
Subject: [PATCH] fix(ivy): catch FatalDiagnosticError thrown from preanalysis
 phase (#34801)

Component's decorator handler exposes `preanalyze` method to preload async resources (templates, stylesheets). The logic in preanalysis phase may throw `FatalDiagnosticError` errors that contain useful information regarding the origin of the problem. However these errors from preanalysis phase were not intercepted in TraitCompiler, resulting in just error message text be displayed. This commit updates the logic to handle FatalDiagnosticError and transform it before throwing, so that the result diagnostic errors contain the necessary info.

PR Close #34801
---
 .../src/ngtsc/transform/src/compilation.ts    | 22 +++++++--
 .../compiler-cli/test/ngtsc/ngtsc_spec.ts     | 48 +++++++++++++++++++
 2 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts
index 4f567a20f4..e103aadd31 100644
--- a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts
+++ b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts
@@ -19,7 +19,7 @@ import {getSourceFile, isExported} from '../../util/src/typescript';
 
 import {AnalysisOutput, CompileResult, DecoratorHandler, HandlerFlags, HandlerPrecedence, ResolveResult} from './api';
 import {DtsTransformRegistry} from './declaration';
-import {Trait, TraitState} from './trait';
+import {PendingTrait, Trait, TraitState} from './trait';
 
 
 /**
@@ -198,7 +198,8 @@ export class TraitCompiler {
     this.fileToClasses.get(sf) !.add(record.node);
   }
 
-  private scanClassForTraits(clazz: ClassDeclaration): Trait<unknown, unknown, unknown>[]|null {
+  private scanClassForTraits(clazz: ClassDeclaration):
+      PendingTrait<unknown, unknown, unknown>[]|null {
     if (!this.compileNonExportedClasses && !isExported(clazz)) {
       return null;
     }
@@ -209,9 +210,9 @@ export class TraitCompiler {
   }
 
   protected detectTraits(clazz: ClassDeclaration, decorators: Decorator[]|null):
-      Trait<unknown, unknown, unknown>[]|null {
+      PendingTrait<unknown, unknown, unknown>[]|null {
     let record: ClassRecord|null = this.recordFor(clazz);
-    let foundTraits: Trait<unknown, unknown, unknown>[] = [];
+    let foundTraits: PendingTrait<unknown, unknown, unknown>[] = [];
 
     for (const handler of this.handlers) {
       const result = handler.detect(clazz, decorators);
@@ -302,7 +303,18 @@ export class TraitCompiler {
 
       let preanalysis: Promise<void>|null = null;
       if (preanalyzeQueue !== null && trait.handler.preanalyze !== undefined) {
-        preanalysis = trait.handler.preanalyze(clazz, trait.detected.metadata) || null;
+        // Attempt to run preanalysis. This could fail with a `FatalDiagnosticError`; catch it if it
+        // does.
+        try {
+          preanalysis = trait.handler.preanalyze(clazz, trait.detected.metadata) || null;
+        } catch (err) {
+          if (err instanceof FatalDiagnosticError) {
+            trait.toErrored([err.toDiagnostic()]);
+            return;
+          } else {
+            throw err;
+          }
+        }
       }
       if (preanalysis !== null) {
         preanalyzeQueue !.push(preanalysis.then(analyze));
diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
index 14c85acb07..1937c75f6d 100644
--- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
+++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
@@ -3552,6 +3552,54 @@ runInEachFileSystem(os => {
       });
     });
 
+    // Run checks that are present in preanalysis phase in both sync and async mode, to make sure
+    // the error messages are consistently thrown from `analyzeSync` and `analyzeAsync` functions.
+    ['sync', 'async'].forEach(mode => {
+      describe(`preanalysis phase checks [${mode}]`, () => {
+        let driveDiagnostics: () => Promise<ReadonlyArray<ts.Diagnostic>>;
+        beforeEach(() => {
+          if (mode === 'async') {
+            env.enablePreloading();
+            driveDiagnostics = () => env.driveDiagnosticsAsync();
+          } else {
+            driveDiagnostics = () => Promise.resolve(env.driveDiagnostics());
+          }
+        });
+
+        it('should throw if @Component is missing a template', async() => {
+          env.write('test.ts', `
+            import {Component} from '@angular/core';
+
+            @Component({
+              selector: 'test',
+            })
+            export class TestCmp {}
+          `);
+
+          const diags = await driveDiagnostics();
+          expect(diags[0].messageText).toBe('component is missing a template');
+          expect(diags[0].file !.fileName).toBe(absoluteFrom('/test.ts'));
+        });
+
+        it('should throw if `styleUrls` is defined incorrectly in @Component', async() => {
+          env.write('test.ts', `
+            import {Component} from '@angular/core';
+
+            @Component({
+              selector: 'test',
+              template: '...',
+              styleUrls: '...'
+            })
+            export class TestCmp {}
+          `);
+
+          const diags = await driveDiagnostics();
+          expect(diags[0].messageText).toBe('styleUrls must be an array of strings');
+          expect(diags[0].file !.fileName).toBe(absoluteFrom('/test.ts'));
+        });
+      });
+    });
+
     describe('flat module indices', () => {
       it('should generate a basic flat module index', () => {
         env.tsconfig({