refactor(core): static-query migrations fails if options cannot be transformed (#30178)

Currently the `static-query` migrations fails at the final step of
updating a query when the query already specifies options which
cannot be transformed easily. e.g. the options are computed through
a function call: `@ViewChild(..., getQueryOpts());` or `@ViewChild(..., myOptionsVar)`.

In these cases we technically could add additionally logic to update
the query options, but given that this is an edge-case and it's
potentially over-engineering the migration schematic, we just
always add a TODO for the timing and print out the determined
query timing in the console. The developer in that case just needs
to manually update the logic for the query options to contain the
printed query timing.

Potentially related to: https://github.com/angular/angular-cli/issues/14298

PR Close #30178

packages/core/schematics/migrations/static-queries/google3/explicitQueryTimingRule.ts

@@ -64,13 +64,13 @@ export class Rule extends Rules.TypedRule {
     queries.forEach(q => {
       const queryExpr = q.decorator.node.expression;
       const {timing, message} = usageStrategy.detectTiming(q);
-      const transformedNode = getTransformedQueryCallExpr(q, timing, !!message);
+      const result = getTransformedQueryCallExpr(q, timing, !!message);
 
-      if (!transformedNode) {
+      if (!result) {
         return;
       }
 
-      const newText = printer.printNode(ts.EmitHint.Unspecified, transformedNode, sourceFile);
+      const newText = printer.printNode(ts.EmitHint.Unspecified, result.node, sourceFile);
 
       // Replace the existing query decorator call expression with the
       // updated call expression node.

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

@@ -192,23 +192,24 @@ async function runStaticQueryMigration(
     queries.forEach(q => {
       const queryExpr = q.decorator.node.expression;
       const {timing, message} = strategy.detectTiming(q);
-      const transformedNode = getTransformedQueryCallExpr(q, timing, !!message);
+      const result = getTransformedQueryCallExpr(q, timing, !!message);
 
-      if (!transformedNode) {
+      if (!result) {
         return;
       }
 
-      const newText = printer.printNode(ts.EmitHint.Unspecified, transformedNode, sourceFile);
+      const newText = printer.printNode(ts.EmitHint.Unspecified, result.node, sourceFile);
 
       // Replace the existing query decorator call expression with the updated
       // call expression node.
       update.remove(queryExpr.getStart(), queryExpr.getWidth());
       update.insertRight(queryExpr.getStart(), newText);
 
-      if (message) {
+      if (result.failureMessage || message) {
         const {line, character} =
             ts.getLineAndCharacterOfPosition(sourceFile, q.decorator.node.getStart());
-        failureMessages.push(`${relativePath}@${line + 1}:${character + 1}: ${message}`);
+        failureMessages.push(
+            `${relativePath}@${line + 1}:${character + 1}: ${result.failureMessage || message}`);
       }
     });
 

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

@@ -10,6 +10,13 @@ import * as ts from 'typescript';
 import {getPropertyNameText} from '../../utils/typescript/property_name';
 import {NgQueryDefinition, QueryTiming} from './angular/query-definition';
 
+export type TransformedQueryResult = null | {
+  /** Transformed call expression. */
+  node: ts.CallExpression;
+  /** Failure message which is set when the query could not be transformed successfully. */
+  failureMessage: string|null;
+};
+
 const TODO_COMMENT = 'TODO: add static flag';
 
 /**
@@ -17,8 +24,8 @@ const TODO_COMMENT = 'TODO: add static flag';
  * determined timing. The updated decorator call expression node will be returned.
  */
 export function getTransformedQueryCallExpr(
-    query: NgQueryDefinition, timing: QueryTiming | null, createTodo: boolean): ts.CallExpression|
-    null {
+    query: NgQueryDefinition, timing: QueryTiming | null,
+    createTodo: boolean): TransformedQueryResult {
   const queryExpr = query.decorator.node.expression;
   const queryArguments = queryExpr.arguments;
   const queryPropertyAssignments = timing === null ?
@@ -27,29 +34,52 @@ export function getTransformedQueryCallExpr(
           'static', timing === QueryTiming.STATIC ? ts.createTrue() : ts.createFalse())];
 
   // If the query decorator is already called with two arguments, we need to
-  // keep the existing options untouched and just add the new property if needed.
+  // keep the existing options untouched and just add the new property if possible.
   if (queryArguments.length === 2) {
-    const existingOptions = queryArguments[1] as ts.ObjectLiteralExpression;
+    const existingOptions = queryArguments[1];
+    const hasTodoComment = existingOptions.getFullText().includes(TODO_COMMENT);
+    let newOptionsNode: ts.Expression;
+    let failureMessage: string|null = null;
 
-    // In case the options already contains a property for the "static" flag, we just
-    // skip this query and leave it untouched.
-    if (existingOptions.properties.some(
-            p => !!p.name && getPropertyNameText(p.name) === 'static')) {
-      return null;
+    if (ts.isObjectLiteralExpression(existingOptions)) {
+      // In case the options already contains a property for the "static" flag,
+      // we just skip this query and leave it untouched.
+      if (existingOptions.properties.some(
+              p => !!p.name && getPropertyNameText(p.name) === 'static')) {
+        return null;
+      }
+
+      newOptionsNode = ts.updateObjectLiteral(
+          existingOptions, existingOptions.properties.concat(queryPropertyAssignments));
+
+      // 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 && !hasTodoComment) {
+        addQueryTimingTodoToNode(newOptionsNode);
+      }
+    } else {
+      // In case the options query parameter is not an object literal expression, and
+      // we want to set the query timing, we just preserve the existing query parameter.
+      newOptionsNode = existingOptions;
+      // We always want to add a TODO in case the query options cannot be updated.
+      if (!hasTodoComment) {
+        addQueryTimingTodoToNode(existingOptions);
+      }
+      // If there is a new explicit timing that has been determined for the given query,
+      // we create a transformation failure message that shows developers that they need
+      // to set the query timing manually to the determined query timing.
+      if (timing !== null) {
+        failureMessage = 'Cannot update query declaration to explicit timing. Please manually ' +
+            `set the query timing to: "{static: ${(timing === QueryTiming.STATIC).toString()}}"`;
+      }
     }
 
-    const updatedOptions = ts.updateObjectLiteral(
-        existingOptions, existingOptions.properties.concat(queryPropertyAssignments));
-
-    // 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);
-    }
-
-    return ts.updateCall(
-        queryExpr, queryExpr.expression, queryExpr.typeArguments,
-        [queryArguments[0], updatedOptions]);
+    return {
+      failureMessage,
+      node: ts.updateCall(
+          queryExpr, queryExpr.expression, queryExpr.typeArguments,
+          [queryArguments[0], newOptionsNode !])
+    };
   }
 
   const optionsNode = ts.createObjectLiteral(queryPropertyAssignments);
@@ -58,8 +88,12 @@ export function getTransformedQueryCallExpr(
     addQueryTimingTodoToNode(optionsNode);
   }
 
-  return ts.updateCall(
-      queryExpr, queryExpr.expression, queryExpr.typeArguments, [queryArguments[0], optionsNode]);
+  return {
+    failureMessage: null,
+    node: ts.updateCall(
+        queryExpr, queryExpr.expression, queryExpr.typeArguments,
+        [queryArguments[0], optionsNode])
+  };
 }
 
 /**

packages/core/schematics/test/static_queries_migration_template_spec.ts

@@ -492,6 +492,31 @@ describe('static-queries migration with template strategy', () => {
           .toMatch(/^⮑ {3}index.ts@6:11: Content queries cannot be migrated automatically\./);
     });
 
+    it('should add a todo if query options cannot be migrated inline', async() => {
+      writeFile('/index.ts', `
+        import {Component, NgModule, ViewChild} from '@angular/core';
+        
+        const myOptionsVar = {};
+
+        @Component({template: '<p #myRef></p>'})
+        export class MyComp {
+          @ViewChild('myRef', myOptionsVar) query: any;
+        }
+
+        @NgModule({declarations: [MyComp]})
+        export class MyModule {}
+      `);
+
+      await runMigration();
+
+      expect(tree.readContent('/index.ts'))
+          .toContain(`@ViewChild('myRef', /* TODO: add static flag */ myOptionsVar) query: any;`);
+      expect(warnOutput.length).toBe(1);
+      expect(warnOutput[0])
+          .toMatch(/^⮑ {3}index.ts@8:11: Cannot update query declaration to explicit timing./);
+      expect(warnOutput[0]).toMatch(/Please manually set the query timing to.*static: true/);
+    });
+
     it('should not normalize stylesheets which are referenced in component', async() => {
       writeFile('sub_dir/index.ts', `
         import {Component, NgModule, ContentChild} from '@angular/core';