fix(ngcc): do not attempt compilation when analysis fails (#34889)

In #34288, ngtsc was refactored to separate the result of the analysis
and resolve phase for more granular incremental rebuilds. In this model,
any errors in one phase transition the trait into an error state, which
prevents it from being ran through subsequent phases. The ngcc compiler
on the other hand did not adopt this strict error model, which would
cause incomplete metadata—due to errors in earlier phases—to be offered
for compilation that could result in a hard crash.

This commit updates ngcc to take advantage of ngtsc's `TraitCompiler`,
that internally manages all Ivy classes that are part of the
compilation. This effectively replaces ngcc's own `AnalyzedFile` and
`AnalyzedClass` types, together with all of the logic to drive the
`DecoratorHandler`s. All of this is now handled in the `TraitCompiler`,
benefiting from its explicit state transitions of `Trait`s so that the
ngcc crash is a thing of the past.

Fixes #34500
Resolves FW-1788

PR Close #34889

packages/compiler-cli/src/ngtsc/annotations/src/component.ts

@@ -109,6 +109,7 @@ export class ComponentDecoratorHandler implements
     if (decorator !== undefined) {
       return {
         trigger: decorator.node,
+        decorator,
         metadata: decorator,
       };
     } else {

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

@@ -70,10 +70,11 @@ export class DirectiveDecoratorHandler implements
         }
         return false;
       });
-      return angularField ? {trigger: angularField.node, metadata: null} : undefined;
+      return angularField ? {trigger: angularField.node, decorator: null, metadata: null} :
+                            undefined;
     } else {
       const decorator = findAngularDecorator(decorators, 'Directive', this.isCore);
-      return decorator ? {trigger: decorator.node, metadata: decorator} : undefined;
+      return decorator ? {trigger: decorator.node, decorator, metadata: decorator} : undefined;
     }
   }
 

packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts

@@ -54,6 +54,7 @@ export class InjectableDecoratorHandler implements
     if (decorator !== undefined) {
       return {
         trigger: decorator.node,
+        decorator: decorator,
         metadata: decorator,
       };
     } else {

packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts

@@ -71,6 +71,7 @@ export class NgModuleDecoratorHandler implements
     if (decorator !== undefined) {
       return {
         trigger: decorator.node,
+        decorator: decorator,
         metadata: decorator,
       };
     } else {

packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts

@@ -44,6 +44,7 @@ export class PipeDecoratorHandler implements DecoratorHandler<Decorator, PipeHan
     if (decorator !== undefined) {
       return {
         trigger: decorator.node,
+        decorator: decorator,
         metadata: decorator,
       };
     } else {

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

@@ -85,11 +85,6 @@ export enum ErrorCode {
    */
   NGMODULE_DECLARATION_NOT_UNIQUE = 6007,
 
-  /**
-   * Raised when ngcc tries to inject a synthetic decorator over one that already exists.
-   */
-  NGCC_MIGRATION_DECORATOR_INJECTION_ERROR = 7001,
-
   /**
    * An element name failed validation against the DOM schema.
    */

packages/compiler-cli/src/ngtsc/transform/index.ts

@@ -9,4 +9,5 @@
 export * from './src/api';
 export {ClassRecord, TraitCompiler} from './src/compilation';
 export {declarationTransformFactory, DtsTransformRegistry, IvyDeclarationDtsTransform, ReturnTypeTransform} from './src/declaration';
+export {AnalyzedTrait, ErroredTrait, PendingTrait, ResolvedTrait, SkippedTrait, Trait, TraitState} from './src/trait';
 export {ivyTransformFactory} from './src/transform';

packages/compiler-cli/src/ngtsc/transform/src/api.ts

@@ -153,8 +153,26 @@ export interface DecoratorHandler<D, A, R> {
       constantPool: ConstantPool): CompileResult|CompileResult[];
 }
 
+/**
+ * The output of detecting a trait for a declaration as the result of the first phase of the
+ * compilation pipeline.
+ */
 export interface DetectResult<M> {
+  /**
+   * The node that triggered the match, which is typically a decorator.
+   */
   trigger: ts.Node|null;
+
+  /**
+   * Refers to the decorator that was recognized for this detection, if any. This can be a concrete
+   * decorator that is actually present in a file, or a synthetic decorator as inserted
+   * programmatically.
+   */
+  decorator: Decorator|null;
+
+  /**
+   * An arbitrary object to carry over from the detection phase into the analysis phase.
+   */
   metadata: Readonly<M>;
 }
 

packages/compiler-cli/src/ngtsc/transform/src/compilation.ts

@@ -6,20 +6,18 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ConstantPool, Type} from '@angular/compiler';
+import {ConstantPool} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
-import {ImportRewriter} from '../../imports';
 import {IncrementalBuild} from '../../incremental/api';
 import {IndexingContext} from '../../indexer';
-import {ModuleWithProvidersScanner} from '../../modulewithproviders';
 import {PerfRecorder} from '../../perf';
-import {ClassDeclaration, ReflectionHost, isNamedClassDeclaration} from '../../reflection';
+import {ClassDeclaration, Decorator, ReflectionHost} from '../../reflection';
 import {TypeCheckContext} from '../../typecheck';
 import {getSourceFile, isExported} from '../../util/src/typescript';
 
-import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from './api';
+import {AnalysisOutput, CompileResult, DecoratorHandler, HandlerFlags, HandlerPrecedence, ResolveResult} from './api';
 import {DtsTransformRegistry} from './declaration';
 import {Trait, TraitState} from './trait';
 
@@ -80,7 +78,7 @@ export class TraitCompiler {
    * Maps source files to any class declaration(s) within them which have been discovered to contain
    * Ivy traits.
    */
-  private fileToClasses = new Map<ts.SourceFile, Set<ClassDeclaration>>();
+  protected fileToClasses = new Map<ts.SourceFile, Set<ClassDeclaration>>();
 
   private reexportMap = new Map<string, Map<string, [string, string]>>();
 
@@ -123,7 +121,7 @@ export class TraitCompiler {
     }
 
     const visit = (node: ts.Node): void => {
-      if (isNamedClassDeclaration(node)) {
+      if (this.reflector.isClass(node)) {
         this.analyzeClass(node, preanalyze ? promises : null);
       }
       ts.forEachChild(node, visit);
@@ -138,6 +136,14 @@ export class TraitCompiler {
     }
   }
 
+  recordFor(clazz: ClassDeclaration): ClassRecord|null {
+    if (this.classes.has(clazz)) {
+      return this.classes.get(clazz) !;
+    } else {
+      return null;
+    }
+  }
+
   recordsFor(sf: ts.SourceFile): ClassRecord[]|null {
     if (!this.fileToClasses.has(sf)) {
       return null;
@@ -192,14 +198,20 @@ export class TraitCompiler {
     this.fileToClasses.get(sf) !.add(record.node);
   }
 
-  private scanClassForTraits(clazz: ClassDeclaration): ClassRecord|null {
+  private scanClassForTraits(clazz: ClassDeclaration): Trait<unknown, unknown, unknown>[]|null {
     if (!this.compileNonExportedClasses && !isExported(clazz)) {
       return null;
     }
 
     const decorators = this.reflector.getDecoratorsOfDeclaration(clazz);
 
-    let record: ClassRecord|null = null;
+    return this.detectTraits(clazz, decorators);
+  }
+
+  protected detectTraits(clazz: ClassDeclaration, decorators: Decorator[]|null):
+      Trait<unknown, unknown, unknown>[]|null {
+    let record: ClassRecord|null = this.recordFor(clazz);
+    let foundTraits: Trait<unknown, unknown, unknown>[] = [];
 
     for (const handler of this.handlers) {
       const result = handler.detect(clazz, decorators);
@@ -207,11 +219,12 @@ export class TraitCompiler {
         continue;
       }
 
-
       const isPrimaryHandler = handler.precedence === HandlerPrecedence.PRIMARY;
       const isWeakHandler = handler.precedence === HandlerPrecedence.WEAK;
       const trait = Trait.pending(handler, result);
 
+      foundTraits.push(trait);
+
       if (record === null) {
         // This is the first handler to match this class. This path is a fast path through which
         // most classes will flow.
@@ -262,8 +275,8 @@ export class TraitCompiler {
             length: clazz.getWidth(),
             messageText: 'Two incompatible decorators on class',
           }];
-          record.traits = [];
-          return record;
+          record.traits = foundTraits = [];
+          break;
         }
 
         // Otherwise, it's safe to accept the multiple decorators here. Update some of the metadata
@@ -273,18 +286,18 @@ export class TraitCompiler {
       }
     }
 
-    return record;
+    return foundTraits.length > 0 ? foundTraits : null;
   }
 
-  private analyzeClass(clazz: ClassDeclaration, preanalyzeQueue: Promise<void>[]|null): void {
-    const record = this.scanClassForTraits(clazz);
+  protected analyzeClass(clazz: ClassDeclaration, preanalyzeQueue: Promise<void>[]|null): void {
+    const traits = this.scanClassForTraits(clazz);
 
-    if (record === null) {
+    if (traits === null) {
       // There are no Ivy traits on the class, so it can safely be skipped.
       return;
     }
 
-    for (const trait of record.traits) {
+    for (const trait of traits) {
       const analyze = () => this.analyzeTrait(clazz, trait);
 
       let preanalysis: Promise<void>|null = null;
@@ -299,7 +312,9 @@ export class TraitCompiler {
     }
   }
 
-  private analyzeTrait(clazz: ClassDeclaration, trait: Trait<unknown, unknown, unknown>): void {
+  protected analyzeTrait(
+      clazz: ClassDeclaration, trait: Trait<unknown, unknown, unknown>,
+      flags?: HandlerFlags): void {
     if (trait.state !== TraitState.PENDING) {
       throw new Error(
           `Attempt to analyze trait of ${clazz.name.text} in state ${TraitState[trait.state]} (expected DETECTED)`);
@@ -308,7 +323,7 @@ export class TraitCompiler {
     // Attempt analysis. This could fail with a `FatalDiagnosticError`; catch it if it does.
     let result: AnalysisOutput<unknown>;
     try {
-      result = trait.handler.analyze(clazz, trait.detected.metadata);
+      result = trait.handler.analyze(clazz, trait.detected.metadata, flags);
     } catch (err) {
       if (err instanceof FatalDiagnosticError) {
         trait = trait.toErrored([err.toDiagnostic()]);
@@ -425,7 +440,7 @@ export class TraitCompiler {
 
   compile(clazz: ts.Declaration, constantPool: ConstantPool): CompileResult[]|null {
     const original = ts.getOriginalNode(clazz) as typeof clazz;
-    if (!isNamedClassDeclaration(clazz) || !isNamedClassDeclaration(original) ||
+    if (!this.reflector.isClass(clazz) || !this.reflector.isClass(original) ||
         !this.classes.has(original)) {
       return null;
     }
@@ -465,7 +480,7 @@ export class TraitCompiler {
 
   decoratorsFor(node: ts.Declaration): ts.Decorator[] {
     const original = ts.getOriginalNode(node) as typeof node;
-    if (!isNamedClassDeclaration(original) || !this.classes.has(original)) {
+    if (!this.reflector.isClass(original) || !this.classes.has(original)) {
       return [];
     }