From c0ad47a3fb43aba86458d2ff99ff0a35e15e48f5 Mon Sep 17 00:00:00 2001
From: Paul Gschwendtner <paulgschwendtner@gmail.com>
Date: Mon, 4 Nov 2019 12:42:13 +0100
Subject: [PATCH] fix(core): undecorated-classes-with-di migration should
 report config errors (#33567)

Currently TypeScript projects with an invalid tsconfig configuration,
cause the undecorated-classes-with-di migration to throw. Instead we
should gracefully exit the migration (like we do for syntactical
diagnostics), but report that there are configuration issues.

This issue surfaced when testing this migration in combination
with the Angular CLI migrations. One of the CLI migrations currently
causes invalid tsconfig files which then cause this issue in the
undecorated-classes-with-di migration.

PR Close #33567
---
 .../undecorated-classes-with-di/index.ts      | 24 ++++++++++++++---
 ...ecorated_classes_with_di_migration_spec.ts | 27 +++++++++++++++++++
 2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/packages/core/schematics/migrations/undecorated-classes-with-di/index.ts b/packages/core/schematics/migrations/undecorated-classes-with-di/index.ts
index 7ffce12ee6039..801910df1ca2f 100644
--- a/packages/core/schematics/migrations/undecorated-classes-with-di/index.ts
+++ b/packages/core/schematics/migrations/undecorated-classes-with-di/index.ts
@@ -9,6 +9,7 @@
 import {logging} from '@angular-devkit/core';
 import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics';
 import {AotCompiler} from '@angular/compiler';
+import {Diagnostic as NgDiagnostic} from '@angular/compiler-cli';
 import {PartialEvaluator} from '@angular/compiler-cli/src/ngtsc/partial_evaluator';
 import {TypeScriptReflectionHost} from '@angular/compiler-cli/src/ngtsc/reflection';
 import {relative} from 'path';
@@ -84,10 +85,11 @@ function runUndecoratedClassesMigration(
   const partialEvaluator =
       new PartialEvaluator(new TypeScriptReflectionHost(typeChecker), typeChecker);
   const declarationCollector = new NgDeclarationCollector(typeChecker, partialEvaluator);
-  const rootSourceFiles = program.getRootFileNames().map(f => program.getSourceFile(f) !);
+  const sourceFiles = program.getSourceFiles().filter(
+      s => !s.isDeclarationFile && !program.isSourceFileFromExternalLibrary(s));
 
   // Analyze source files by detecting all directives, components and providers.
-  rootSourceFiles.forEach(sourceFile => declarationCollector.visitNode(sourceFile));
+  sourceFiles.forEach(sourceFile => declarationCollector.visitNode(sourceFile));
 
   const {decoratedDirectives, decoratedProviders, undecoratedDeclarations} = declarationCollector;
   const transform =
@@ -151,14 +153,28 @@ function runUndecoratedClassesMigration(
   }
 }
 
+function getErrorDiagnostics(diagnostics: ReadonlyArray<ts.Diagnostic|NgDiagnostic>) {
+  return <ts.Diagnostic[]>diagnostics.filter(d => d.category === ts.DiagnosticCategory.Error);
+}
+
 function gracefullyCreateProgram(
     tree: Tree, basePath: string, tsconfigPath: string,
     logger: logging.LoggerApi): {compiler: AotCompiler, program: ts.Program}|null {
   try {
     const {ngcProgram, host, program, compiler} = createNgcProgram(
         (options) => createMigrationCompilerHost(tree, options, basePath), tsconfigPath);
-    const syntacticDiagnostics = ngcProgram.getTsSyntacticDiagnostics();
-    const structuralDiagnostics = ngcProgram.getNgStructuralDiagnostics();
+    const syntacticDiagnostics = getErrorDiagnostics(ngcProgram.getTsSyntacticDiagnostics());
+    const structuralDiagnostics = getErrorDiagnostics(ngcProgram.getNgStructuralDiagnostics());
+    const configDiagnostics = getErrorDiagnostics(
+        [...program.getOptionsDiagnostics(), ...ngcProgram.getNgOptionDiagnostics()]);
+
+    if (configDiagnostics.length) {
+      logger.warn(
+          `\nTypeScript project "${tsconfigPath}" has configuration errors. This could cause ` +
+          `an incomplete migration. Please fix the following failures and rerun the migration:`);
+      logger.error(ts.formatDiagnostics(configDiagnostics, host));
+      return null;
+    }
 
     // Syntactic TypeScript errors can throw off the query analysis and therefore we want
     // to notify the developer that we couldn't analyze parts of the project. Developers
diff --git a/packages/core/schematics/test/undecorated_classes_with_di_migration_spec.ts b/packages/core/schematics/test/undecorated_classes_with_di_migration_spec.ts
index 15ea12c7f2b8e..beebaf81b4ff8 100644
--- a/packages/core/schematics/test/undecorated_classes_with_di_migration_spec.ts
+++ b/packages/core/schematics/test/undecorated_classes_with_di_migration_spec.ts
@@ -1495,5 +1495,32 @@ describe('Undecorated classes with DI migration', () => {
       expect(warnOutput.length).toBe(0);
       expect(errorOutput.length).toBe(0);
     });
+
+    it('should not throw if tsconfig references non-existent source file', async() => {
+      writeFile('/tsconfig.json', JSON.stringify({
+        compilerOptions: {
+          lib: ['es2015'],
+        },
+        files: [
+          './non-existent.ts',
+        ]
+      }));
+
+      let failed = false;
+      try {
+        await runMigration();
+      } catch (e) {
+        failed = true;
+      }
+
+      expect(failed).toBe(false, 'Expected the migration not to fail.');
+      expect(warnOutput.length).toBe(1);
+      expect(errorOutput.length).toBe(1);
+      expect(warnOutput[0])
+          .toContain(
+              'TypeScript project "tsconfig.json" has configuration errors. This could cause an ' +
+              'incomplete migration. Please fix the following failures and rerun the migration:');
+      expect(errorOutput[0]).toMatch(/non-existent\.ts' not found/);
+    });
   });
 });