refactor(core): better error message for undecorated-classes-with-di …

…migration (#33315)

Currently if one of the project targets could not be analyzed
due to AOT compiler program failures, we gracefully proceed
with the migration. This is expected, but we should not
print a message at the end of the migration that the migration
was _successful_. The migration was only done partially, hence
it's potentially incomplete and we should make it clear that once
the failures are resolved, the migration should be re-run.

PR Close #33315

packages/core/schematics/migrations/undecorated-classes-with-di/index.ts

@@ -35,6 +35,7 @@ export default function(): Rule {
     const {buildPaths} = getProjectTsConfigPaths(tree);
     const basePath = process.cwd();
     const failures: string[] = [];
+    let programError = false;
 
     if (!buildPaths.length) {
       throw new SchematicsException(
@@ -43,10 +44,23 @@ export default function(): Rule {
     }
 
     for (const tsconfigPath of buildPaths) {
-      failures.push(...runUndecoratedClassesMigration(tree, tsconfigPath, basePath, ctx.logger));
+      const result = runUndecoratedClassesMigration(tree, tsconfigPath, basePath, ctx.logger);
+      failures.push(...result.failures);
+      programError = programError || !!result.programError;
     }
 
-    if (failures.length) {
+    if (programError) {
+      ctx.logger.info('Could not migrate all undecorated classes that use dependency');
+      ctx.logger.info('injection. Some project targets could not be analyzed due to');
+      ctx.logger.info('TypeScript program failures.\n');
+      ctx.logger.info(`${MIGRATION_RERUN_MESSAGE}\n`);
+
+      if (failures.length) {
+        ctx.logger.info('Please manually fix the following failures and re-run the');
+        ctx.logger.info('migration once the TypeScript program failures are resolved.');
+        failures.forEach(message => ctx.logger.warn(`⮑   ${message}`));
+      }
+    } else if (failures.length) {
       ctx.logger.info('Could not migrate all undecorated classes that use dependency');
       ctx.logger.info('injection. Please manually fix the following failures:');
       failures.forEach(message => ctx.logger.warn(`⮑   ${message}`));
@@ -55,13 +69,14 @@ export default function(): Rule {
 }
 
 function runUndecoratedClassesMigration(
-    tree: Tree, tsconfigPath: string, basePath: string, logger: logging.LoggerApi): string[] {
+    tree: Tree, tsconfigPath: string, basePath: string,
+    logger: logging.LoggerApi): {failures: string[], programError?: boolean} {
   const failures: string[] = [];
   const programData = gracefullyCreateProgram(tree, basePath, tsconfigPath, logger);
 
   // Gracefully exit if the program could not be created.
   if (programData === null) {
-    return [];
+    return {failures: [], programError: true};
   }
 
   const {program, compiler} = programData;
@@ -101,7 +116,7 @@ function runUndecoratedClassesMigration(
   // file in order to avoid shifted character offsets.
   updateRecorders.forEach(recorder => recorder.commitUpdate());
 
-  return failures;
+  return {failures};
 
   /** Gets the update recorder for the specified source file. */
   function getUpdateRecorder(sourceFile: ts.SourceFile): UpdateRecorder {
@@ -153,7 +168,6 @@ function gracefullyCreateProgram(
           `\nTypeScript project "${tsconfigPath}" has syntactical errors which could cause ` +
           `an incomplete migration. Please fix the following failures and rerun the migration:`);
       logger.error(ts.formatDiagnostics(syntacticDiagnostics, host));
-      logger.info(MIGRATION_RERUN_MESSAGE);
       return null;
     }
 
@@ -165,7 +179,6 @@ function gracefullyCreateProgram(
   } catch (e) {
     logger.warn(`\n${MIGRATION_AOT_FAILURE}. The following project failed: ${tsconfigPath}\n`);
     logger.error(`${e.toString()}\n`);
-    logger.info(MIGRATION_RERUN_MESSAGE);
     return null;
   }
 }

packages/core/schematics/test/undecorated_classes_with_di_migration_spec.ts

@@ -21,6 +21,7 @@ describe('Undecorated classes with DI migration', () => {
   let previousWorkingDir: string;
   let warnOutput: string[];
   let errorOutput: string[];
+  let infoOutput: string[];
 
   beforeEach(() => {
     runner = new SchematicTestRunner('test', require.resolve('../migrations.json'));
@@ -39,11 +40,14 @@ describe('Undecorated classes with DI migration', () => {
 
     warnOutput = [];
     errorOutput = [];
+    infoOutput = [];
     runner.logger.subscribe(logEntry => {
       if (logEntry.level === 'warn') {
         warnOutput.push(logEntry.message);
       } else if (logEntry.level === 'error') {
         errorOutput.push(logEntry.message);
+      } else if (logEntry.level === 'info') {
+        infoOutput.push(logEntry.message);
       }
     });
 
@@ -112,6 +116,10 @@ describe('Undecorated classes with DI migration', () => {
     expect(errorOutput.length).toBe(0);
     expect(warnOutput.length).toBe(1);
     expect(warnOutput[0]).toMatch(/Class needs to declare an explicit constructor./);
+    expect(infoOutput.join(' '))
+        .toContain(
+            'Could not migrate all undecorated classes that use ' +
+            'dependency injection. Please manually fix the following failures');
   });
 
   it('should add @Directive() decorator to extended base class', async() => {
@@ -1442,6 +1450,10 @@ describe('Undecorated classes with DI migration', () => {
               /ensure there are no AOT compilation errors and rerun the migration.*project failed: tsconfig\.json/);
       expect(errorOutput.length).toBe(1);
       expect(errorOutput[0]).toMatch(/Cannot determine the module for class TestComp/);
+      expect(infoOutput.join(' '))
+          .toContain(
+              'Some project targets could not be analyzed due to ' +
+              'TypeScript program failures');
     });
 
     it('should gracefully exit migration if project fails with syntactical diagnostic', async() => {
@@ -1456,6 +1468,10 @@ describe('Undecorated classes with DI migration', () => {
           .toMatch(/project "tsconfig.json" has syntactical errors which could cause/);
       expect(errorOutput.length).toBe(1);
       expect(errorOutput[0]).toMatch(/error TS1005: 'from' expected/);
+      expect(infoOutput.join(' '))
+          .toContain(
+              'Some project targets could not be analyzed due to ' +
+              'TypeScript program failures');
     });
 
     it('should not throw if resources could not be read', async() => {