fix(ngcc): report diagnostics from migrations (#34014)

When ngcc is analyzing synthetically inserted decorators from a
migration, it is typically not expected that any diagnostics are
produced. In the situation where a diagnostic is produced, however, the
diagnostic would not be reported at all. This commit ensures that
diagnostics in migrations are reported.

PR Close #34014

packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts

@@ -160,7 +160,7 @@ export class DecorationAnalyzer {
   protected applyMigrations(analyzedFiles: AnalyzedFile[]): void {
     const migrationHost = new DefaultMigrationHost(
         this.reflectionHost, this.fullMetaReader, this.evaluator, this.handlers,
-        this.bundle.entryPoint.path, analyzedFiles);
+        this.bundle.entryPoint.path, analyzedFiles, this.diagnosticHandler);
 
     this.migrations.forEach(migration => {
       analyzedFiles.forEach(analyzedFile => {

packages/compiler-cli/ngcc/src/analysis/migration_host.ts

@@ -27,7 +27,8 @@ export class DefaultMigrationHost implements MigrationHost {
   constructor(
       readonly reflectionHost: NgccReflectionHost, readonly metadata: MetadataReader,
       readonly evaluator: PartialEvaluator, private handlers: DecoratorHandler<any, any>[],
-      private entryPointPath: AbsoluteFsPath, private analyzedFiles: AnalyzedFile[]) {}
+      private entryPointPath: AbsoluteFsPath, private analyzedFiles: AnalyzedFile[],
+      private diagnosticHandler: (error: ts.Diagnostic) => void) {}
 
   injectSyntheticDecorator(clazz: ClassDeclaration, decorator: Decorator, flags?: HandlerFlags):
       void {
@@ -37,6 +38,12 @@ export class DefaultMigrationHost implements MigrationHost {
       return;
     }
 
+    if (newAnalyzedClass.diagnostics !== undefined) {
+      for (const diagnostic of newAnalyzedClass.diagnostics) {
+        this.diagnosticHandler(createMigrationDiagnostic(diagnostic, clazz, decorator));
+      }
+    }
+
     const analyzedFile = getOrCreateAnalyzedFile(this.analyzedFiles, clazz.getSourceFile());
     const oldAnalyzedClass = analyzedFile.analyzedClasses.find(c => c.declaration === clazz);
     if (oldAnalyzedClass === undefined) {
@@ -102,3 +109,43 @@ function mergeAnalyzedClasses(oldClass: AnalyzedClass, newClass: AnalyzedClass)
     }
   }
 }
+
+/**
+ * Creates a diagnostic from another one, containing additional information about the synthetic
+ * decorator.
+ */
+function createMigrationDiagnostic(
+    diagnostic: ts.Diagnostic, source: ts.Node, decorator: Decorator): ts.Diagnostic {
+  const clone = {...diagnostic};
+
+  const chain: ts.DiagnosticMessageChain[] = [{
+    messageText: `Occurs for @${decorator.name} decorator inserted by an automatic migration`,
+    category: ts.DiagnosticCategory.Message,
+    code: 0,
+  }];
+
+  if (decorator.args !== null) {
+    const args = decorator.args.map(arg => arg.getText()).join(', ');
+    chain.push({
+      messageText: `@${decorator.name}(${args})`,
+      category: ts.DiagnosticCategory.Message,
+      code: 0,
+    });
+  }
+
+  if (typeof clone.messageText === 'string') {
+    clone.messageText = {
+      messageText: clone.messageText,
+      category: diagnostic.category,
+      code: diagnostic.code,
+      next: chain,
+    };
+  } else {
+    if (clone.messageText.next === undefined) {
+      clone.messageText.next = chain;
+    } else {
+      clone.messageText.next.push(...chain);
+    }
+  }
+  return clone;
+}

packages/compiler-cli/ngcc/test/analysis/migration_host_spec.ts

@@ -5,7 +5,9 @@
  * 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 {ErrorCode} from '../../../src/ngtsc/diagnostics';
+import * as ts from 'typescript';
+
+import {ErrorCode, makeDiagnostic} from '../../../src/ngtsc/diagnostics';
 import {AbsoluteFsPath, absoluteFrom} from '../../../src/ngtsc/file_system';
 import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
 import {ClassDeclaration, Decorator} from '../../../src/ngtsc/reflection';
@@ -13,6 +15,7 @@ import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPr
 import {DefaultMigrationHost} from '../../src/analysis/migration_host';
 import {AnalyzedClass, AnalyzedFile} from '../../src/analysis/types';
 import {NgccClassSymbol} from '../../src/host/ngcc_host';
+import {createComponentDecorator} from '../../src/migrations/utils';
 
 runInEachFileSystem(() => {
   describe('DefaultMigrationHost', () => {
@@ -23,6 +26,7 @@ runInEachFileSystem(() => {
     let mockEvaluator: any = {};
     let mockClazz: any;
     let mockDecorator: any = {name: 'MockDecorator'};
+    let diagnosticHandler = () => {};
     beforeEach(() => {
       _ = absoluteFrom;
       entryPointPath = _('/node_modules/some-package/entry-point');
@@ -51,7 +55,8 @@ runInEachFileSystem(() => {
         const handler1 = new TestHandler('handler1', log);
         const handler2 = new TestHandler('handler2', log);
         const host = new DefaultMigrationHost(
-            mockHost, mockMetadata, mockEvaluator, [handler1, handler2], entryPointPath, []);
+            mockHost, mockMetadata, mockEvaluator, [handler1, handler2], entryPointPath, [],
+            diagnosticHandler);
         host.injectSyntheticDecorator(mockClazz, mockDecorator);
         expect(log).toEqual([
           `handler1:detect:MockClazz:MockDecorator`,
@@ -67,7 +72,7 @@ runInEachFileSystem(() => {
            const handler3 = new TestHandler('handler3', log);
            const host = new DefaultMigrationHost(
                mockHost, mockMetadata, mockEvaluator, [handler1, handler2, handler3],
-               entryPointPath, []);
+               entryPointPath, [], diagnosticHandler);
            host.injectSyntheticDecorator(mockClazz, mockDecorator);
            expect(log).toEqual([
              `handler1:detect:MockClazz:MockDecorator`,
@@ -82,7 +87,8 @@ runInEachFileSystem(() => {
         const handler = new AlwaysDetectHandler('handler', log);
         const analyzedFiles: AnalyzedFile[] = [];
         const host = new DefaultMigrationHost(
-            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
+            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles,
+            diagnosticHandler);
         host.injectSyntheticDecorator(mockClazz, mockDecorator);
         expect(analyzedFiles.length).toEqual(1);
         expect(analyzedFiles[0].analyzedClasses.length).toEqual(1);
@@ -99,7 +105,8 @@ runInEachFileSystem(() => {
           analyzedClasses: [DUMMY_CLASS_1, DUMMY_CLASS_2],
         }];
         const host = new DefaultMigrationHost(
-            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
+            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles,
+            diagnosticHandler);
         host.injectSyntheticDecorator(mockClazz, mockDecorator);
         expect(analyzedFiles.length).toEqual(1);
         expect(analyzedFiles[0].analyzedClasses.length).toEqual(3);
@@ -120,7 +127,8 @@ runInEachFileSystem(() => {
           analyzedClasses: [analyzedClass],
         }];
         const host = new DefaultMigrationHost(
-            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
+            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles,
+            diagnosticHandler);
         host.injectSyntheticDecorator(mockClazz, mockDecorator);
         expect(analyzedFiles.length).toEqual(1);
         expect(analyzedFiles[0].analyzedClasses.length).toEqual(1);
@@ -144,7 +152,8 @@ runInEachFileSystem(() => {
              analyzedClasses: [analyzedClass],
            }];
            const host = new DefaultMigrationHost(
-               mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
+               mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles,
+               diagnosticHandler);
            host.injectSyntheticDecorator(mockClazz, mockDecorator);
            expect(analyzedFiles.length).toEqual(1);
            expect(analyzedFiles[0].analyzedClasses.length).toEqual(1);
@@ -167,11 +176,34 @@ runInEachFileSystem(() => {
           analyzedClasses: [analyzedClass],
         }];
         const host = new DefaultMigrationHost(
-            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
+            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles,
+            diagnosticHandler);
         expect(() => host.injectSyntheticDecorator(mockClazz, mockDecorator))
             .toThrow(jasmine.objectContaining(
                 {code: ErrorCode.NGCC_MIGRATION_DECORATOR_INJECTION_ERROR}));
       });
+
+      it('should report diagnostics from handlers', () => {
+        const log: string[] = [];
+        const handler = new DiagnosticProducingHandler('handler', log);
+        const analyzedFiles: AnalyzedFile[] = [];
+        const diagnostics: ts.Diagnostic[] = [];
+        const host = new DefaultMigrationHost(
+            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles,
+            diagnostic => diagnostics.push(diagnostic));
+        mockClazz.getStart = () => 0;
+        mockClazz.getWidth = () => 0;
+
+        const decorator = createComponentDecorator(mockClazz, {selector: 'comp', exportAs: null});
+        host.injectSyntheticDecorator(mockClazz, decorator);
+
+        expect(diagnostics.length).toBe(1);
+        expect(ts.flattenDiagnosticMessageText(diagnostics[0].messageText, '\n'))
+            .toEqual(
+                `test diagnostic\n` +
+                `  Occurs for @Component decorator inserted by an automatic migration\n` +
+                `  @Component({ template: "", selector: "comp" })`);
+      });
     });
 
     describe('getAllDecorators', () => {
@@ -180,7 +212,8 @@ runInEachFileSystem(() => {
         const handler = new AlwaysDetectHandler('handler', log);
         const analyzedFiles: AnalyzedFile[] = [];
         const host = new DefaultMigrationHost(
-            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
+            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles,
+            diagnosticHandler);
 
         const decorators = host.getAllDecorators(mockClazz);
         expect(decorators).toBeNull();
@@ -191,7 +224,8 @@ runInEachFileSystem(() => {
         const handler = new AlwaysDetectHandler('handler', log);
         const analyzedFiles: AnalyzedFile[] = [];
         const host = new DefaultMigrationHost(
-            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
+            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles,
+            diagnosticHandler);
 
         const sourceFile: any = {};
         const unrelatedClass: any = {
@@ -218,7 +252,8 @@ runInEachFileSystem(() => {
           analyzedClasses: [analyzedClass],
         }];
         const host = new DefaultMigrationHost(
-            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
+            mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles,
+            diagnosticHandler);
         host.injectSyntheticDecorator(mockClazz, mockDecorator);
 
         const decorators = host.getAllDecorators(mockClazz) !;
@@ -232,7 +267,8 @@ runInEachFileSystem(() => {
       it('should be true for nodes within the entry-point', () => {
         const analyzedFiles: AnalyzedFile[] = [];
         const host = new DefaultMigrationHost(
-            mockHost, mockMetadata, mockEvaluator, [], entryPointPath, analyzedFiles);
+            mockHost, mockMetadata, mockEvaluator, [], entryPointPath, analyzedFiles,
+            diagnosticHandler);
 
         const sourceFile: any = {
           fileName: _('/node_modules/some-package/entry-point/relative.js'),
@@ -246,7 +282,8 @@ runInEachFileSystem(() => {
       it('should be false for nodes outside the entry-point', () => {
         const analyzedFiles: AnalyzedFile[] = [];
         const host = new DefaultMigrationHost(
-            mockHost, mockMetadata, mockEvaluator, [], entryPointPath, analyzedFiles);
+            mockHost, mockMetadata, mockEvaluator, [], entryPointPath, analyzedFiles,
+            diagnosticHandler);
 
         const sourceFile: any = {
           fileName: _('/node_modules/some-package/other-entry/index.js'),
@@ -284,3 +321,10 @@ class AlwaysDetectHandler extends TestHandler {
     return {trigger: node, metadata: {}};
   }
 }
+
+class DiagnosticProducingHandler extends AlwaysDetectHandler {
+  analyze(node: ClassDeclaration): AnalysisOutput<any> {
+    super.analyze(node);
+    return {diagnostics: [makeDiagnostic(9999, node, 'test diagnostic')]};
+  }
+}