refactor(core): static query schematic should handle asynchronous query usages properly

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

@@ -53,22 +53,25 @@ function isQueryUsedStatically(
   // access it statically. e.g.
   //  (1) queries used in the "ngOnInit" lifecycle hook are static.
   //  (2) inputs with setters can access queries statically.
-  const possibleStaticQueryNodes: ts.Node[] = classDecl.members.filter(m => {
-    if (ts.isMethodDeclaration(m) && hasPropertyNameText(m.name) &&
-        STATIC_QUERY_LIFECYCLE_HOOKS[query.type].indexOf(m.name.text) !== -1) {
-      return true;
-    } else if (
-        knownInputNames && ts.isSetAccessor(m) && hasPropertyNameText(m.name) &&
-        knownInputNames.indexOf(m.name.text) !== -1) {
-      return true;
-    }
-    return false;
-  });
+  const possibleStaticQueryNodes: ts.Node[] =
+      classDecl.members
+          .filter(m => {
+            if (ts.isMethodDeclaration(m) && m.body && hasPropertyNameText(m.name) &&
+                STATIC_QUERY_LIFECYCLE_HOOKS[query.type].indexOf(m.name.text) !== -1) {
+              return true;
+            } else if (
+                knownInputNames && ts.isSetAccessor(m) && m.body && hasPropertyNameText(m.name) &&
+                knownInputNames.indexOf(m.name.text) !== -1) {
+              return true;
+            }
+            return false;
+          })
+          .map((member: ts.SetAccessorDeclaration | ts.MethodDeclaration) => member.body !);
 
   // In case nodes that can possibly access a query statically have been found, check
-  // if the query declaration is used within any of these nodes.
+  // if the query declaration is synchronously used within any of these nodes.
   if (possibleStaticQueryNodes.length &&
-      possibleStaticQueryNodes.some(hookNode => usageVisitor.isUsedInNode(hookNode))) {
+      possibleStaticQueryNodes.some(n => usageVisitor.isSynchronouslyUsedInNode(n))) {
     return true;
   }
 

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

@@ -7,6 +7,7 @@
  */
 
 import * as ts from 'typescript';
+import {isFunctionLikeDeclaration, unwrapExpression} from '../typescript/functions';
 
 /**
  * Class that can be used to determine if a given TypeScript node is used within
@@ -26,19 +27,34 @@ export class DeclarationUsageVisitor {
   }
 
   private addJumpExpressionToQueue(node: ts.Expression, nodeQueue: ts.Node[]) {
-    const callExprSymbol = this.typeChecker.getSymbolAtLocation(node);
+    // In case the given expression is already referring to a function-like declaration,
+    // we don't need to resolve the symbol of the expression as the jump expression is
+    // defined inline and we can just add the given node to the queue.
+    if (isFunctionLikeDeclaration(node) && node.body) {
+      nodeQueue.push(node.body);
+      return;
+    }
+
+    const callExprType = this.typeChecker.getTypeAtLocation(node);
+    const callExprSymbol = callExprType.getSymbol();
+
+    if (!callExprSymbol || !callExprSymbol.valueDeclaration ||
+        !isFunctionLikeDeclaration(callExprSymbol.valueDeclaration)) {
+      return;
+    }
 
-    // Note that we should not add previously visited symbols to the queue as this
-    // could cause cycles.
-    if (callExprSymbol && callExprSymbol.valueDeclaration &&
-        !this.visitedJumpExprSymbols.has(callExprSymbol)) {
+    const expressionDecl = callExprSymbol.valueDeclaration;
+
+    // Note that we should not add previously visited symbols to the queue as
+    // this could cause cycles.
+    if (expressionDecl.body && !this.visitedJumpExprSymbols.has(callExprSymbol)) {
       this.visitedJumpExprSymbols.add(callExprSymbol);
-      nodeQueue.push(callExprSymbol.valueDeclaration);
+      nodeQueue.push(expressionDecl.body);
     }
   }
 
   private addNewExpressionToQueue(node: ts.NewExpression, nodeQueue: ts.Node[]) {
-    const newExprSymbol = this.typeChecker.getSymbolAtLocation(node.expression);
+    const newExprSymbol = this.typeChecker.getSymbolAtLocation(unwrapExpression(node.expression));
 
     // Only handle new expressions which resolve to classes. Technically "new" could
     // also call void functions or objects with a constructor signature. Also note that
@@ -50,15 +66,15 @@ export class DeclarationUsageVisitor {
     }
 
     const targetConstructor =
-        newExprSymbol.valueDeclaration.members.find(d => ts.isConstructorDeclaration(d));
+        newExprSymbol.valueDeclaration.members.find(ts.isConstructorDeclaration);
 
-    if (targetConstructor) {
+    if (targetConstructor && targetConstructor.body) {
       this.visitedJumpExprSymbols.add(newExprSymbol);
-      nodeQueue.push(targetConstructor);
+      nodeQueue.push(targetConstructor.body);
     }
   }
 
-  isUsedInNode(searchNode: ts.Node): boolean {
+  isSynchronouslyUsedInNode(searchNode: ts.Node): boolean {
     const nodeQueue: ts.Node[] = [searchNode];
     this.visitedJumpExprSymbols.clear();
 
@@ -72,7 +88,7 @@ export class DeclarationUsageVisitor {
       // Handle call expressions within TypeScript nodes that cause a jump in control
       // flow. We resolve the call expression value declaration and add it to the node queue.
       if (ts.isCallExpression(node)) {
-        this.addJumpExpressionToQueue(node.expression, nodeQueue);
+        this.addJumpExpressionToQueue(unwrapExpression(node.expression), nodeQueue);
       }
 
       // Handle new expressions that cause a jump in control flow. We resolve the
@@ -81,7 +97,12 @@ export class DeclarationUsageVisitor {
         this.addNewExpressionToQueue(node, nodeQueue);
       }
 
-      nodeQueue.push(...node.getChildren());
+      // Do not visit nodes that declare a block of statements but are not executed
+      // synchronously (e.g. function declarations). We only want to check TypeScript
+      // nodes which are synchronously executed in the control flow.
+      if (!isFunctionLikeDeclaration(node)) {
+        nodeQueue.push(...node.getChildren());
+      }
     }
     return false;
   }

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

@@ -25,6 +25,16 @@ import {parseTsconfigFile} from './typescript/tsconfig';
 export function runStaticQueryMigration(tree: Tree, tsconfigPath: string, basePath: string) {
   const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
   const host = ts.createCompilerHost(parsed.options, true);
+
+  // We need to overwrite the host "readFile" method, as we want the TypeScript
+  // program to be based on the file contents in the virtual file tree. Otherwise
+  // if we run the migration for multiple tsconfig files which have intersecting
+  // source files, it can end up updating query definitions multiple times.
+  host.readFile = fileName => {
+    const buffer = tree.read(relative(basePath, fileName));
+    return buffer ? buffer.toString() : undefined;
+  };
+
   const program = ts.createProgram(parsed.fileNames, parsed.options, host);
   const typeChecker = program.getTypeChecker();
   const queryVisitor = new NgQueryResolveVisitor(typeChecker);

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

@@ -0,0 +1,29 @@
+/**
+ * @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';
+
+/** Checks whether a given node is a function like declaration. */
+export function isFunctionLikeDeclaration(node: ts.Node): node is ts.FunctionLikeDeclaration {
+  return ts.isFunctionDeclaration(node) || ts.isMethodDeclaration(node) ||
+      ts.isArrowFunction(node) || ts.isFunctionExpression(node) ||
+      ts.isGetAccessorDeclaration(node) || ts.isSetAccessorDeclaration(node);
+}
+
+/**
+ * Unwraps a given expression TypeScript node. Expressions can be wrapped within multiple
+ * parentheses. e.g. "(((({exp}))))()". The function should return the TypeScript node
+ * referring to the inner expression. e.g "exp".
+ */
+export function unwrapExpression(node: ts.Expression | ts.ParenthesizedExpression): ts.Expression {
+  if (ts.isParenthesizedExpression(node)) {
+    return unwrapExpression(node.expression);
+  } else {
+    return node;
+  }
+}

packages/core/schematics/test/project_tsconfig_paths_spec.ts

@@ -22,7 +22,7 @@ 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)).toEqual(['my-custom-config.json']);
   });
 
   it('should detect test tsconfig path inside of .angular.json file', () => {
@@ -32,7 +32,7 @@ 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)).toEqual(['my-test-config.json']);
   });
 
   it('should detect common tsconfigs if no workspace config could be found', () => {
@@ -41,7 +41,16 @@ describe('project tsconfig paths', () => {
     testTree.create('/src/tsconfig.app.json', '');
 
     expect(getProjectTsConfigPaths(testTree)).toEqual([
-      './tsconfig.json', './src/tsconfig.json', './src/tsconfig.app.json'
+      'tsconfig.json', 'src/tsconfig.json', 'src/tsconfig.app.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'}}}}}
+    }));
+
+    expect(getProjectTsConfigPaths(testTree)).toEqual(['tsconfig.json']);
+  });
 });