fixup! feat(core): update schematic to migrate to explicit query timing

Properly handle chained class inheritances

packages/core/schematics/migrations/static-queries/angular/analyze_query_usage.ts

@@ -27,43 +27,37 @@ const STATIC_QUERY_LIFECYCLE_HOOKS = {
 export function analyzeNgQueryUsage(
     query: NgQueryDefinition, derivedClassesMap: DerivedClassesMap,
     typeChecker: ts.TypeChecker): QueryTiming {
-  const classDecl = query.container;
-
-  // List of classes that derive from the query container and need to be analyzed as well.
-  // e.g. a ViewQuery could be used statically in a derived class.
-  const derivedClasses = derivedClassesMap.get(classDecl);
-  let isStatic = isQueryUsedStatically(classDecl, query, typeChecker);
-
-  // We don't need to check the derived classes if the container class already
-  // uses the query statically. This improves performances for a large chain of
-  // derived classes.
-  if (derivedClasses && !isStatic) {
-    isStatic = derivedClasses.some(
-        derivedClass => isQueryUsedStatically(derivedClass, query, typeChecker));
-  }
-
-  return isStatic ? QueryTiming.STATIC : QueryTiming.DYNAMIC;
+  return isQueryUsedStatically(query.container, query, derivedClassesMap, typeChecker) ?
+      QueryTiming.STATIC :
+      QueryTiming.DYNAMIC;
 }
 
-/** Checks whether the given class uses the specified query statically. */
+/** Checks whether a given class or it's derived classes use the specified query statically. */
 function isQueryUsedStatically(
-    classDecl: ts.ClassDeclaration, query: NgQueryDefinition,
+    classDecl: ts.ClassDeclaration, query: NgQueryDefinition, derivedClassesMap: DerivedClassesMap,
     typeChecker: ts.TypeChecker): boolean {
+  const usageVisitor = new DeclarationUsageVisitor(query.property, typeChecker);
   const staticQueryHooks = classDecl.members.filter(
       m => ts.isMethodDeclaration(m) &&
           (ts.isStringLiteralLike(m.name) || ts.isIdentifier(m.name)) &&
           STATIC_QUERY_LIFECYCLE_HOOKS[query.type].indexOf(m.name.text) !== -1);
 
-  // In case there are no lifecycle hooks defined which could access a query
-  // statically, we can consider the query as dynamic as nothing in the class declaration
-  // could reasonably access the query in a static way.
-  if (!staticQueryHooks.length) {
-    return false;
+  // In case there lifecycle hooks defined which could access this type of query
+  // statically, we look if the query declaration is statically accessed within
+  // one of the lifecycle hook declarations.
+  if (staticQueryHooks.length &&
+      staticQueryHooks.some(hookNode => usageVisitor.isUsedInNode(hookNode))) {
+    return true;
   }
 
-  const usageVisitor = new DeclarationUsageVisitor(query.property, typeChecker);
+  // List of classes that derive from the query container and need to be analyzed as well.
+  // e.g. a ViewQuery could be used statically in a derived class.
+  const derivedClasses = derivedClassesMap.get(classDecl);
+
+  if (!derivedClasses) {
+    return false;
+  }
 
-  // Visit each defined lifecycle hook and check whether the query property is used
-  // inside the method declaration.
-  return staticQueryHooks.some(hookDeclNode => usageVisitor.isUsedInNode(hookDeclNode));
+  return derivedClasses.some(
+      derivedClass => isQueryUsedStatically(derivedClass, query, derivedClassesMap, typeChecker));
 }

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

@@ -328,5 +328,30 @@ describe('static-queries migration', () => {
       expect(tree.readContent('/index.ts'))
           .toContain(`@${queryType}('test', { static: true }) query: any;`);
     });
+
+    it('should detect static queries within nested inheritance', () => {
+      writeFile('/index.ts', `
+        import {Component, ${queryType}} from '@angular/core';
+        
+        @Component({template: '<span #test></span>'})
+        export class MyComp {
+          @${queryType}('test') query: any;
+        }
+        
+        export class A extends MyComp {}
+        export class B extends A {
+        
+          ngOnInit() {
+            this.query.testFn();
+          }
+        
+        }
+      `);
+
+      runMigration();
+
+      expect(tree.readContent('/index.ts'))
+          .toContain(`@${queryType}('test', { static: true }) query: any;`);
+    });
   }
 });

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

@@ -13,6 +13,7 @@ import * as ts from 'typescript';
 import {analyzeNgQueryUsage} from './angular/analyze_query_usage';
 import {NgQueryResolveVisitor} from './angular/ng_query_visitor';
 import {NgQueryDefinition, QueryTiming} from './angular/query-definition';
+import {getPropertyNameText} from './typescript/property_name';
 import {parseTsconfigFile} from './typescript/tsconfig';
 
 /**
@@ -74,8 +75,7 @@ function recordQueryUsageTransformation(
     // 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 && (ts.isIdentifier(p.name) || ts.isStringLiteralLike(p.name)) &&
-                p.name.text === 'static')) {
+            p => !!p.name && getPropertyNameText(p.name) === 'static')) {
       return;
     }
 

packages/core/schematics/migrations/static-queries/typescript/property_name.ts

@@ -0,0 +1,20 @@
+/**
+ * @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 * as ts from 'typescript';
+
+/**
+ * Gets the text of the given property name. Returns null if the property
+ * name couldn't be determined statically.
+ */
+export function getPropertyNameText(node: ts.PropertyName): string|null {
+  if (ts.isIdentifier(node) || ts.isStringLiteralLike(node)) {
+    return node.text;
+  }
+  return null;
+}