refactor(core): static-query migration should not fail for test files (#30034)

Currently when someone runs `ng update` with the static-query migration,
the migration can fail with an error saying that the `AOT` compiler could not
be created. This can happen if the CLI project contains a test `tsconfig.json`
that is picked up by the schematic.

Due to the fact that spec tsconfig files cannot be ran with NGC (e.g. test
components are not part of a module; not all source files are guaranteed to
be included), test `tsconfig` projects will now use a new `test` migration
strategy where all queries within tests are left untouched and a TODO is added.

PR Close #30034

packages/core/schematics/migrations/move-document/index.ts

@@ -19,15 +19,15 @@ import {addToImport, createImport, removeFromImport} from './move-import';
 /** Entry point for the V8 move-document migration. */
 export default function(): Rule {
   return (tree: Tree) => {
-    const projectTsConfigPaths = getProjectTsConfigPaths(tree);
+    const {buildPaths, testPaths} = getProjectTsConfigPaths(tree);
     const basePath = process.cwd();
 
-    if (!projectTsConfigPaths.length) {
+    if (!buildPaths.length && !testPaths.length) {
       throw new SchematicsException(`Could not find any tsconfig file. Cannot migrate DOCUMENT 
           to new import source.`);
     }
 
-    for (const tsconfigPath of projectTsConfigPaths) {
+    for (const tsconfigPath of [...buildPaths, ...testPaths]) {
       runMoveDocumentMigration(tree, tsconfigPath, basePath);
     }
   };

packages/core/schematics/migrations/static-queries/index.ts

@@ -19,10 +19,17 @@ import {TypeScriptVisitor, visitAllNodes} from '../../utils/typescript/visit_nod
 
 import {NgQueryResolveVisitor} from './angular/ng_query_visitor';
 import {QueryTemplateStrategy} from './strategies/template_strategy/template_strategy';
+import {QueryTestStrategy} from './strategies/test_strategy/test_strategy';
 import {TimingStrategy} from './strategies/timing-strategy';
 import {QueryUsageStrategy} from './strategies/usage_strategy/usage_strategy';
 import {getTransformedQueryCallExpr} from './transform';
 
+export enum SELECTED_STRATEGY {
+  TEMPLATE,
+  USAGE,
+  TESTS,
+}
+
 /** Entry point for the V8 static-query migration. */
 export default function(): Rule {
   return (tree: Tree, context: SchematicContext) => {
@@ -34,7 +41,7 @@ export default function(): Rule {
 
 /** Runs the V8 migration static-query migration for all determined TypeScript projects. */
 async function runMigration(tree: Tree, context: SchematicContext) {
-  const projectTsConfigPaths = getProjectTsConfigPaths(tree);
+  const {buildPaths, testPaths} = getProjectTsConfigPaths(tree);
   const basePath = process.cwd();
   const logger = context.logger;
 
@@ -44,7 +51,7 @@ async function runMigration(tree: Tree, context: SchematicContext) {
   logger.info('https://github.com/angular/angular/pull/28810');
   logger.info('');
 
-  if (!projectTsConfigPaths.length) {
+  if (!buildPaths.length && !testPaths.length) {
     throw new SchematicsException(
         'Could not find any tsconfig file. Cannot migrate queries ' +
         'to explicit timing.');
@@ -52,7 +59,7 @@ async function runMigration(tree: Tree, context: SchematicContext) {
 
   // In case prompts are supported, determine the desired migration strategy
   // by creating a choice prompt. By default the template strategy is used.
-  let isUsageStrategy = false;
+  let selectedStrategy: SELECTED_STRATEGY = SELECTED_STRATEGY.TEMPLATE;
   if (supportsPrompt()) {
     logger.info('There are two available migration strategies that can be selected:');
     logger.info('  • Template strategy  -  migration tool (short-term gains, rare corrections)');
@@ -70,17 +77,26 @@ async function runMigration(tree: Tree, context: SchematicContext) {
       default: 'template',
     });
     logger.info('');
-    isUsageStrategy = strategyName === 'usage';
+    selectedStrategy =
+        strategyName === 'usage' ? SELECTED_STRATEGY.USAGE : SELECTED_STRATEGY.TEMPLATE;
   } else {
     // In case prompts are not supported, we still want to allow developers to opt
     // into the usage strategy by specifying an environment variable. The tests also
     // use the environment variable as there is no headless way to select via prompt.
-    isUsageStrategy = !!process.env['NG_STATIC_QUERY_USAGE_STRATEGY'];
+    selectedStrategy = !!process.env['NG_STATIC_QUERY_USAGE_STRATEGY'] ? SELECTED_STRATEGY.USAGE :
+                                                                         SELECTED_STRATEGY.TEMPLATE;
   }
 
   const failures = [];
-  for (const tsconfigPath of projectTsConfigPaths) {
-    failures.push(...await runStaticQueryMigration(tree, tsconfigPath, basePath, isUsageStrategy));
+
+  for (const tsconfigPath of buildPaths) {
+    failures.push(...await runStaticQueryMigration(tree, tsconfigPath, basePath, selectedStrategy));
+  }
+  // For the "test" tsconfig projects we always want to use the test strategy as
+  // we can't detect the proper timing within spec files.
+  for (const tsconfigPath of testPaths) {
+    failures.push(
+        ...await runStaticQueryMigration(tree, tsconfigPath, basePath, SELECTED_STRATEGY.TESTS));
   }
 
   if (failures.length) {
@@ -99,7 +115,7 @@ async function runMigration(tree: Tree, context: SchematicContext) {
  * lifecycle hook does not need to be static and can be set up with "static: false".
  */
 async function runStaticQueryMigration(
-    tree: Tree, tsconfigPath: string, basePath: string, isUsageStrategy: boolean) {
+    tree: Tree, tsconfigPath: string, basePath: string, selectedStrategy: SELECTED_STRATEGY) {
   const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
   const host = ts.createCompilerHost(parsed.options, true);
 
@@ -119,10 +135,11 @@ async function runStaticQueryMigration(
   const rootSourceFiles = program.getRootFileNames().map(f => program.getSourceFile(f) !);
   const printer = ts.createPrinter();
   const analysisVisitors: TypeScriptVisitor[] = [queryVisitor];
+  const failureMessages: string[] = [];
 
   // If the "usage" strategy is selected, we also need to add the query visitor
   // to the analysis visitors so that query usage in templates can be also checked.
-  if (isUsageStrategy) {
+  if (selectedStrategy === SELECTED_STRATEGY.USAGE) {
     analysisVisitors.push(templateVisitor);
   }
 
@@ -136,7 +153,7 @@ async function runStaticQueryMigration(
   const {resolvedQueries, classMetadata} = queryVisitor;
   const {resolvedTemplates} = templateVisitor;
 
-  if (isUsageStrategy) {
+  if (selectedStrategy === SELECTED_STRATEGY.USAGE) {
     // Add all resolved templates to the class metadata if the usage strategy is used. This
     // is necessary in order to be able to check component templates for static query usage.
     resolvedTemplates.forEach(template => {
@@ -146,10 +163,14 @@ async function runStaticQueryMigration(
     });
   }
 
-  const strategy: TimingStrategy = isUsageStrategy ?
-      new QueryUsageStrategy(classMetadata, typeChecker) :
-      new QueryTemplateStrategy(tsconfigPath, classMetadata, host);
-  const failureMessages: string[] = [];
+  let strategy: TimingStrategy;
+  if (selectedStrategy === SELECTED_STRATEGY.USAGE) {
+    strategy = new QueryUsageStrategy(classMetadata, typeChecker);
+  } else if (selectedStrategy === SELECTED_STRATEGY.TESTS) {
+    strategy = new QueryTestStrategy();
+  } else {
+    strategy = new QueryTemplateStrategy(tsconfigPath, classMetadata, host);
+  }
 
   // In case the strategy could not be set up properly, we just exit the
   // migration. We don't want to throw an exception as this could mean

packages/core/schematics/migrations/static-queries/strategies/template_strategy/template_strategy.ts

@@ -184,6 +184,8 @@ export class QueryTemplateStrategy implements TimingStrategy {
     console.error('Could not create Angular AOT compiler to determine query timing.');
     console.error('The following diagnostics were detected:\n');
     console.error(diagnostics.map(d => d.messageText).join(`\n`));
+    console.error('Please make sure that there is no compilation failure. The migration');
+    console.error('can be rerun with: "ng update @angular/core --from 7 --to 8 --migrate-only"');
   }
 
   private _getViewQueryUniqueKey(filePath: string, className: string, propName: string) {

packages/core/schematics/migrations/static-queries/strategies/test_strategy/test_strategy.ts

@@ -0,0 +1,28 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * 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 {NgQueryDefinition} from '../../angular/query-definition';
+import {TimingResult, TimingStrategy} from '../timing-strategy';
+
+/**
+ * Query timing strategy that is used for queries used within test files. The query
+ * timing is not analyzed for test files as the template strategy cannot work within
+ * spec files (due to missing component modules) and the usage strategy is not capable
+ * of detecting the timing of queries based on how they are used in tests.
+ */
+export class QueryTestStrategy implements TimingStrategy {
+  setup() { return true; }
+
+  /**
+   * Detects the timing for a given query. For queries within tests, we always
+   * add a TODO and print a message saying that the timing can't be detected for tests.
+   */
+  detectTiming(query: NgQueryDefinition): TimingResult {
+    return {timing: null, message: 'Timing within tests cannot be detected.'};
+  }
+}

packages/core/schematics/migrations/static-queries/transform.ts

@@ -10,6 +10,8 @@ import * as ts from 'typescript';
 import {getPropertyNameText} from '../../utils/typescript/property_name';
 import {NgQueryDefinition, QueryTiming} from './angular/query-definition';
 
+const TODO_COMMENT = 'TODO: add static flag';
+
 /**
  * Transforms the given query decorator by explicitly specifying the timing based on the
  * determined timing. The updated decorator call expression node will be returned.
@@ -39,7 +41,9 @@ export function getTransformedQueryCallExpr(
     const updatedOptions = ts.updateObjectLiteral(
         existingOptions, existingOptions.properties.concat(queryPropertyAssignments));
 
-    if (createTodo) {
+    // In case we want to add a todo and the options do not have the todo
+    // yet, we add the query timing todo as synthetic multi-line comment.
+    if (createTodo && !existingOptions.getFullText().includes(TODO_COMMENT)) {
       addQueryTimingTodoToNode(updatedOptions);
     }
 
@@ -68,6 +72,6 @@ function addQueryTimingTodoToNode(node: ts.Node) {
                                    end: -1,
                                    hasTrailingNewLine: false,
                                    kind: ts.SyntaxKind.MultiLineCommentTrivia,
-                                   text: ' TODO: add static flag '
+                                   text: ` ${TODO_COMMENT} `
                                  }]);
 }

packages/core/schematics/migrations/template-var-assignment/index.ts

@@ -27,16 +27,16 @@ const FAILURE_MESSAGE = `Found assignment to template variable.`;
 /** Entry point for the V8 template variable assignment schematic. */
 export default function(): Rule {
   return (tree: Tree, context: SchematicContext) => {
-    const projectTsConfigPaths = getProjectTsConfigPaths(tree);
+    const {buildPaths, testPaths} = getProjectTsConfigPaths(tree);
     const basePath = process.cwd();
 
-    if (!projectTsConfigPaths.length) {
+    if (!buildPaths.length && !testPaths.length) {
       throw new SchematicsException(
           'Could not find any tsconfig file. Cannot check templates for template variable ' +
           'assignments.');
     }
 
-    for (const tsconfigPath of projectTsConfigPaths) {
+    for (const tsconfigPath of [...buildPaths, ...testPaths]) {
       runTemplateVariableAssignmentCheck(tree, tsconfigPath, basePath, context.logger);
     }
   };

packages/core/schematics/test/move_document_migration_spec.ts

@@ -29,6 +29,9 @@ describe('move-document migration', () => {
         lib: ['es2015'],
       }
     }));
+    writeFile('/angular.json', JSON.stringify({
+      projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}}
+    }));
 
     previousWorkingDir = shx.pwd();
     tmpDirPath = getSystemPath(host.root);

packages/core/schematics/test/project_tsconfig_paths_spec.ts

@@ -22,7 +22,16 @@ describe('project tsconfig paths', () => {
           {my_name: {architect: {build: {options: {tsConfig: './my-custom-config.json'}}}}}
     }));
 
-    expect(getProjectTsConfigPaths(testTree)).toEqual(['my-custom-config.json']);
+    expect(getProjectTsConfigPaths(testTree).buildPaths).toEqual(['my-custom-config.json']);
+  });
+
+  it('should detect test tsconfig path inside of angular.json file', () => {
+    testTree.create('/my-test-config.json', '');
+    testTree.create('/angular.json', JSON.stringify({
+      projects: {my_name: {architect: {test: {options: {tsConfig: './my-test-config.json'}}}}}
+    }));
+
+    expect(getProjectTsConfigPaths(testTree).testPaths).toEqual(['my-test-config.json']);
   });
 
   it('should detect test tsconfig path inside of .angular.json file', () => {
@@ -32,25 +41,23 @@ describe('project tsconfig paths', () => {
           {with_tests: {architect: {test: {options: {tsConfig: './my-test-config.json'}}}}}
     }));
 
-    expect(getProjectTsConfigPaths(testTree)).toEqual(['my-test-config.json']);
+    expect(getProjectTsConfigPaths(testTree).testPaths).toEqual(['my-test-config.json']);
   });
 
   it('should detect common tsconfigs if no workspace config could be found', () => {
-    testTree.create('/tsconfig.json', '');
-    testTree.create('/src/tsconfig.json', '');
     testTree.create('/src/tsconfig.app.json', '');
+    testTree.create('/src/tsconfig.spec.json', '');
 
-    expect(getProjectTsConfigPaths(testTree)).toEqual([
-      'tsconfig.json', 'src/tsconfig.json', 'src/tsconfig.app.json'
-    ]);
+    expect(getProjectTsConfigPaths(testTree).buildPaths).toEqual(['src/tsconfig.app.json']);
+    expect(getProjectTsConfigPaths(testTree).testPaths).toEqual(['src/tsconfig.spec.json']);
   });
 
   it('should not return duplicate tsconfig files', () => {
     testTree.create('/tsconfig.json', '');
     testTree.create('/.angular.json', JSON.stringify({
-      projects: {app: {architect: {test: {options: {tsConfig: 'tsconfig.json'}}}}}
+      projects: {app: {architect: {build: {options: {tsConfig: 'tsconfig.json'}}}}}
     }));
 
-    expect(getProjectTsConfigPaths(testTree)).toEqual(['tsconfig.json']);
+    expect(getProjectTsConfigPaths(testTree).buildPaths).toEqual(['tsconfig.json']);
   });
 });

packages/core/schematics/test/static_queries_migration_template_spec.ts

@@ -31,6 +31,9 @@ describe('static-queries migration with template strategy', () => {
         lib: ['es2015'],
       }
     }));
+    writeFile('/angular.json', JSON.stringify({
+      projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}}
+    }));
 
     warnOutput = [];
     runner.logger.subscribe(logEntry => {
@@ -515,5 +518,54 @@ describe('static-queries migration with template strategy', () => {
 
       expect(console.error).toHaveBeenCalledTimes(0);
     });
+
+    it('should always use the test migration strategy for test tsconfig files', async() => {
+      writeFile('/src/tsconfig.spec.json', JSON.stringify({
+        compilerOptions: {
+          experimentalDecorators: true,
+          lib: ['es2015'],
+        },
+        files: [
+          'test.ts',
+        ],
+      }));
+
+      writeFile('/src/test.ts', `
+        import {ViewChild} from '@angular/core';
+        import {AppComponent} from './app.component';
+        
+        @Component({template: '<span #test>Test</span>'})
+        class MyTestComponent {
+          @ViewChild('test') query: any;
+        }
+      `);
+
+      writeFile('/src/app.component.ts', `
+        import {Component, ViewChild} from '@angular/core';
+        
+        @Component({template: '<span #test></span>'})
+        export class AppComponent {
+          @ViewChild('test') query: any;
+        }
+      `);
+
+      writeFile('/src/app.module.ts', `
+        import {NgModule} from '@angular/core';
+        import {AppComponent} from './app.component';
+        
+        @NgModule({declarations: [AppComponent]})
+        export class MyModule {}
+      `);
+
+      spyOn(console, 'error').and.callThrough();
+
+      await runMigration();
+
+      expect(console.error).toHaveBeenCalledTimes(0);
+      expect(tree.readContent('/src/test.ts'))
+          .toContain(`@ViewChild('test', /* TODO: add static flag */ {}) query: any;`);
+      expect(tree.readContent('/src/app.component.ts'))
+          .toContain(`@ViewChild('test', { static: true }) query: any;`);
+    });
   });
 });

packages/core/schematics/test/static_queries_migration_usage_spec.ts

@@ -35,6 +35,9 @@ describe('static-queries migration with usage strategy', () => {
         lib: ['es2015'],
       }
     }));
+    writeFile('/angular.json', JSON.stringify({
+      projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}}
+    }));
 
     previousWorkingDir = shx.pwd();
     tmpDirPath = getSystemPath(host.root);
@@ -598,6 +601,9 @@ describe('static-queries migration with usage strategy', () => {
       // Move the tsconfig into a subdirectory. This ensures that the update is properly
       // recorded for TypeScript projects not at the schematic tree root.
       host.sync.rename(normalize('/tsconfig.json'), normalize('/src/tsconfig.json'));
+      writeFile('/angular.json', JSON.stringify({
+        projects: {t: {architect: {build: {options: {tsConfig: './src/tsconfig.json'}}}}}
+      }));
 
       await runMigration();
 

packages/core/schematics/test/template_var_assignment_migration_spec.ts

@@ -30,6 +30,9 @@ describe('template variable assignment migration', () => {
         lib: ['es2015'],
       }
     }));
+    writeFile('/angular.json', JSON.stringify({
+      projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}}
+    }));
 
     warnOutput = [];
     runner.logger.subscribe(logEntry => {

packages/core/schematics/utils/project_tsconfig_paths.ts

@@ -8,6 +8,7 @@
 
 import {normalize} from '@angular-devkit/core';
 import {Tree} from '@angular-devkit/schematics';
+import {WorkspaceProject} from '@schematics/angular/utility/workspace-models';
 
 /** Name of the default Angular CLI workspace configuration files. */
 const defaultWorkspaceConfigPaths = ['/angular.json', '/.angular.json'];
@@ -16,13 +17,11 @@ const defaultWorkspaceConfigPaths = ['/angular.json', '/.angular.json'];
  * Gets all tsconfig paths from a CLI project by reading the workspace configuration
  * and looking for common tsconfig locations.
  */
-export function getProjectTsConfigPaths(tree: Tree): string[] {
-  // Start with some tsconfig paths that are generally used within CLI projects.
-  const tsconfigPaths = new Set<string>([
-    'tsconfig.json',
-    'src/tsconfig.json',
-    'src/tsconfig.app.json',
-  ]);
+export function getProjectTsConfigPaths(tree: Tree): {buildPaths: string[], testPaths: string[]} {
+  // Start with some tsconfig paths that are generally used within CLI projects. Note
+  // that we are not interested in IDE-specific tsconfig files (e.g. /tsconfig.json)
+  const buildPaths = new Set<string>(['src/tsconfig.app.json']);
+  const testPaths = new Set<string>(['src/tsconfig.spec.json']);
 
   // Add any tsconfig directly referenced in a build or test task of the angular.json workspace.
   const workspace = getWorkspaceConfigGracefully(tree);
@@ -30,23 +29,38 @@ export function getProjectTsConfigPaths(tree: Tree): string[] {
   if (workspace) {
     const projects = Object.keys(workspace.projects).map(name => workspace.projects[name]);
     for (const project of projects) {
-      ['build', 'test'].forEach(targetName => {
-        if (project.targets && project.targets[targetName] && project.targets[targetName].options &&
-            project.targets[targetName].options.tsConfig) {
-          tsconfigPaths.add(normalize(project.targets[targetName].options.tsConfig));
-        }
+      const buildPath = getTargetTsconfigPath(project, 'build');
+      const testPath = getTargetTsconfigPath(project, 'test');
 
-        if (project.architect && project.architect[targetName] &&
-            project.architect[targetName].options &&
-            project.architect[targetName].options.tsConfig) {
-          tsconfigPaths.add(normalize(project.architect[targetName].options.tsConfig));
-        }
-      });
+      if (buildPath) {
+        buildPaths.add(buildPath);
+      }
+
+      if (testPath) {
+        testPaths.add(testPath);
+      }
     }
   }
 
   // Filter out tsconfig files that don't exist in the CLI project.
-  return Array.from(tsconfigPaths).filter(p => tree.exists(p));
+  return {
+    buildPaths: Array.from(buildPaths).filter(p => tree.exists(p)),
+    testPaths: Array.from(testPaths).filter(p => tree.exists(p)),
+  };
+}
+
+/** Gets the tsconfig path from the given target within the specified project. */
+function getTargetTsconfigPath(project: WorkspaceProject, targetName: string): string|null {
+  if (project.targets && project.targets[targetName] && project.targets[targetName].options &&
+      project.targets[targetName].options.tsConfig) {
+    return normalize(project.targets[targetName].options.tsConfig);
+  }
+
+  if (project.architect && project.architect[targetName] && project.architect[targetName].options &&
+      project.architect[targetName].options.tsConfig) {
+    return normalize(project.architect[targetName].options.tsConfig);
+  }
+  return null;
 }
 
 /**