refactor(core): add tslint rule entry-point for static-query migration (#29258)

In order to be able to use the static-query migration logic within
Google, we need to provide a TSLint rule entry-point that wires up
the schematic logic and provides reporting and automatic fixes.

PR Close #29258

package.json

@@ -97,6 +97,7 @@
     "systemjs": "0.18.10",
     "tsickle": "0.34.3",
     "tslib": "^1.9.0",
+    "tslint": "5.7.0",
     "typescript": "~3.3.3333",
     "xhr2": "0.1.4",
     "yargs": "9.0.1",
@@ -144,7 +145,6 @@
     "rewire": "2.5.2",
     "sauce-connect": "https://saucelabs.com/downloads/sc-4.5.1-linux.tar.gz",
     "semver": "5.4.1",
-    "tslint": "5.7.0",
     "tslint-eslint-rules": "4.1.1",
     "tsutils": "2.27.2",
     "universal-analytics": "0.4.15",

packages/core/schematics/migrations/static-queries/BUILD.bazel

@@ -2,13 +2,11 @@ load("//tools:defaults.bzl", "ts_library")
 
 ts_library(
     name = "static-queries",
-    srcs = glob(
+    srcs = glob(["**/*.ts"]),
-        ["**/*.ts"],
-        exclude = ["index_spec.ts"],
-    ),
     tsconfig = "//packages/core/schematics:tsconfig.json",
     visibility = [
         "//packages/core/schematics:__pkg__",
+        "//packages/core/schematics/migrations/static-queries/google3:__pkg__",
         "//packages/core/schematics/test:__pkg__",
     ],
     deps = [

packages/core/schematics/migrations/static-queries/google3/BUILD.bazel

@@ -0,0 +1,12 @@
+load("//tools:defaults.bzl", "ts_library")
+
+ts_library(
+    name = "google3",
+    srcs = glob(["**/*.ts"]),
+    tsconfig = "//packages/core/schematics:tsconfig.json",
+    visibility = ["//packages/core/schematics/test:__pkg__"],
+    deps = [
+        "//packages/core/schematics/migrations/static-queries",
+        "@npm//tslint",
+    ],
+)

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

@@ -0,0 +1,66 @@
+/**
+ * @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 {Replacement, RuleFailure, Rules} from 'tslint';
+import * as ts from 'typescript';
+
+import {analyzeNgQueryUsage} from '../angular/analyze_query_usage';
+import {NgQueryResolveVisitor} from '../angular/ng_query_visitor';
+import {QueryTiming} from '../angular/query-definition';
+import {getTransformedQueryCallExpr} from '../transform';
+
+/**
+ * Rule that reports if an Angular "ViewChild" or "ContentChild" query is not explicitly
+ * specifying its timing. The rule also provides TSLint automatic replacements that can
+ * be applied in order to automatically migrate to the explicit query timing API.
+ */
+export class Rule extends Rules.TypedRule {
+  applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] {
+    const typeChecker = program.getTypeChecker();
+    const queryVisitor = new NgQueryResolveVisitor(program.getTypeChecker());
+    const rootSourceFiles = program.getRootFileNames().map(f => program.getSourceFile(f) !);
+    const printer = ts.createPrinter();
+    const failures: RuleFailure[] = [];
+
+    // Analyze source files by detecting queries and class relations.
+    rootSourceFiles.forEach(sourceFile => queryVisitor.visitNode(sourceFile));
+
+    const {resolvedQueries, classMetadata} = queryVisitor;
+    const queries = resolvedQueries.get(sourceFile);
+
+    // No queries detected for the given source file.
+    if (!queries) {
+      return [];
+    }
+
+    // Compute the query usage for all resolved queries and update the
+    // query definitions to explicitly declare the query timing (static or dynamic)
+    queries.forEach(q => {
+      const queryExpr = q.decorator.node.expression;
+      const timing = analyzeNgQueryUsage(q, classMetadata, typeChecker);
+      const transformedNode = getTransformedQueryCallExpr(q, timing);
+
+      if (!transformedNode) {
+        return;
+      }
+
+      const newText = printer.printNode(ts.EmitHint.Unspecified, transformedNode, sourceFile);
+
+      // Replace the existing query decorator call expression with the
+      // updated call expression node.
+      const fix = new Replacement(queryExpr.getStart(), queryExpr.getWidth(), newText);
+      const timingStr = timing === QueryTiming.STATIC ? 'static' : 'dynamic';
+
+      failures.push(new RuleFailure(
+          sourceFile, queryExpr.getStart(), queryExpr.getWidth(),
+          `Query is not explicitly marked as "${timingStr}"`, this.ruleName, fix));
+    });
+
+    return failures;
+  }
+}

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

@@ -7,8 +7,16 @@
  */
 
 import {Rule, SchematicsException, Tree} from '@angular-devkit/schematics';
+import {dirname, relative} from 'path';
+import * as ts from 'typescript';
+
 import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
-import {runStaticQueryMigration} from './migration';
+
+import {analyzeNgQueryUsage} from './angular/analyze_query_usage';
+import {NgQueryResolveVisitor} from './angular/ng_query_visitor';
+import {getTransformedQueryCallExpr} from './transform';
+import {parseTsconfigFile} from './typescript/tsconfig';
+
 
 /** Entry point for the V8 static-query migration. */
 export default function(): Rule {
@@ -27,3 +35,63 @@ export default function(): Rule {
     }
   };
 }
+
+/**
+ * Runs the static query migration for the given TypeScript project. The schematic
+ * analyzes all queries within the project and sets up the query timing based on
+ * the current usage of the query property. e.g. a view query that is not used in any
+ * lifecycle hook does not need to be static and can be set up with "static: false".
+ */
+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);
+  const rootSourceFiles = program.getRootFileNames().map(f => program.getSourceFile(f) !);
+  const printer = ts.createPrinter();
+
+  // Analyze source files by detecting queries and class relations.
+  rootSourceFiles.forEach(sourceFile => queryVisitor.visitNode(sourceFile));
+
+  const {resolvedQueries, classMetadata} = queryVisitor;
+
+  // Walk through all source files that contain resolved queries and update
+  // the source files if needed. Note that we need to update multiple queries
+  // within a source file within the same recorder in order to not throw off
+  // the TypeScript node offsets.
+  resolvedQueries.forEach((queries, sourceFile) => {
+    const update = tree.beginUpdate(relative(basePath, sourceFile.fileName));
+
+    // Compute the query usage for all resolved queries and update the
+    // query definitions to explicitly declare the query timing (static or dynamic)
+    queries.forEach(q => {
+      const queryExpr = q.decorator.node.expression;
+      const timing = analyzeNgQueryUsage(q, classMetadata, typeChecker);
+      const transformedNode = getTransformedQueryCallExpr(q, timing);
+
+      if (!transformedNode) {
+        return;
+      }
+
+      const newText = printer.printNode(ts.EmitHint.Unspecified, transformedNode, 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);
+    });
+
+    tree.commitUpdate(update);
+  });
+}

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

@@ -1,107 +0,0 @@
-/**
- * @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 {Tree, UpdateRecorder} from '@angular-devkit/schematics';
-import {dirname, relative, resolve} from 'path';
-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';
-
-/**
- * Runs the static query migration for the given TypeScript project. The schematic
- * analyzes all queries within the project and sets up the query timing based on
- * the current usage of the query property. e.g. a view query that is not used in any
- * lifecycle hook does not need to be static and can be set up with "static: false".
- */
-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);
-  const rootSourceFiles = program.getRootFileNames().map(f => program.getSourceFile(f) !);
-  const printer = ts.createPrinter();
-
-  // Analyze source files by detecting queries and class relations.
-  rootSourceFiles.forEach(sourceFile => queryVisitor.visitNode(sourceFile));
-
-  const {resolvedQueries, classMetadata} = queryVisitor;
-
-  // Walk through all source files that contain resolved queries and update
-  // the source files if needed. Note that we need to update multiple queries
-  // within a source file within the same recorder in order to not throw off
-  // the TypeScript node offsets.
-  resolvedQueries.forEach((queries, sourceFile) => {
-    const update = tree.beginUpdate(relative(basePath, sourceFile.fileName));
-
-    // Compute the query usage for all resolved queries and update the
-    // query definitions to explicitly declare the query timing (static or dynamic)
-    queries.forEach(q => {
-      const timing = analyzeNgQueryUsage(q, classMetadata, typeChecker);
-      recordQueryUsageTransformation(q, update, timing, printer, sourceFile);
-    });
-
-    tree.commitUpdate(update);
-  });
-}
-
-/**
- * Transforms the query decorator by explicitly specifying the timing based on the
- * determined timing. The changes will be added to the specified update recorder.
- */
-function recordQueryUsageTransformation(
-    query: NgQueryDefinition, recorder: UpdateRecorder, timing: QueryTiming, printer: ts.Printer,
-    sourceFile: ts.SourceFile) {
-  const queryExpr = query.decorator.node.expression as ts.CallExpression;
-  const queryArguments = queryExpr.arguments;
-  const timingPropertyAssignment = ts.createPropertyAssignment(
-      'static', timing === QueryTiming.STATIC ? ts.createTrue() : ts.createFalse());
-  let newCallText = '';
-
-  // 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.
-  if (queryArguments.length === 2) {
-    const existingOptions = queryArguments[1] as ts.ObjectLiteralExpression;
-
-    // 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;
-    }
-
-    const updatedOptions = ts.updateObjectLiteral(
-        existingOptions, existingOptions.properties.concat(timingPropertyAssignment));
-    const updatedCall = ts.updateCall(
-        queryExpr, queryExpr.expression, queryExpr.typeArguments,
-        [queryArguments[0], updatedOptions]);
-    newCallText = printer.printNode(ts.EmitHint.Unspecified, updatedCall, sourceFile);
-  } else {
-    const newCall = ts.updateCall(
-        queryExpr, queryExpr.expression, queryExpr.typeArguments,
-        [queryArguments[0], ts.createObjectLiteral([timingPropertyAssignment])]);
-    newCallText = printer.printNode(ts.EmitHint.Unspecified, newCall, sourceFile);
-  }
-
-  recorder.remove(queryExpr.getStart(), queryExpr.getWidth());
-  recorder.insertRight(queryExpr.getStart(), newCallText);
-}

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

@@ -0,0 +1,48 @@
+/**
+ * @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';
+
+import {NgQueryDefinition, QueryTiming} from './angular/query-definition';
+import {getPropertyNameText} from './typescript/property_name';
+
+
+/**
+ * Transforms the given query decorator by explicitly specifying the timing based on the
+ * determined timing. The updated decorator call expression node will be returned.
+ */
+export function getTransformedQueryCallExpr(
+    query: NgQueryDefinition, timing: QueryTiming): ts.CallExpression|null {
+  const queryExpr = query.decorator.node.expression as ts.CallExpression;
+  const queryArguments = queryExpr.arguments;
+  const timingPropertyAssignment = ts.createPropertyAssignment(
+      '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.
+  if (queryArguments.length === 2) {
+    const existingOptions = queryArguments[1] as ts.ObjectLiteralExpression;
+
+    // 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;
+    }
+
+    const updatedOptions = ts.updateObjectLiteral(
+        existingOptions, existingOptions.properties.concat(timingPropertyAssignment));
+    return ts.updateCall(
+        queryExpr, queryExpr.expression, queryExpr.typeArguments,
+        [queryArguments[0], updatedOptions]);
+  }
+
+  return ts.updateCall(
+      queryExpr, queryExpr.expression, queryExpr.typeArguments,
+      [queryArguments[0], ts.createObjectLiteral([timingPropertyAssignment])]);
+}

packages/core/schematics/test/BUILD.bazel

@@ -10,9 +10,11 @@ ts_library(
     ],
     deps = [
         "//packages/core/schematics/migrations/static-queries",
+        "//packages/core/schematics/migrations/static-queries/google3",
         "//packages/core/schematics/utils",
         "@npm//@angular-devkit/schematics",
         "@npm//@types/shelljs",
+        "@npm//tslint",
     ],
 )
 

packages/core/schematics/test/google3/explicit_query_timing_rule_spec.ts

@@ -0,0 +1,131 @@
+/**
+ * @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 {readFileSync, writeFileSync} from 'fs';
+import {dirname, join} from 'path';
+import * as shx from 'shelljs';
+import {Configuration, Linter} from 'tslint';
+
+describe('Google3 explicitQueryTiming TSLint rule', () => {
+
+  /**
+   * Path to the static-query schematic rules directory. The path needs to be resolved through
+   * the Bazel runfiles, because on Windows runfiles are not symlinked into the working directory.
+   */
+  const rulesDirectory =
+      dirname(require.resolve('../../migrations/static-queries/google3/explicitQueryTimingRule'));
+
+  let tmpDir: string;
+
+  beforeEach(() => {
+    tmpDir = join(process.env['TEST_TMPDIR'] !, 'google3-test');
+    shx.mkdir('-p', tmpDir);
+
+    writeFile('tsconfig.json', JSON.stringify({compilerOptions: {module: 'es2015'}}));
+  });
+
+  afterEach(() => shx.rm('-r', tmpDir));
+
+  /**
+   * Runs TSLint with the static-query timing TSLint rule. By default the rule fixes
+   * are automatically applied.
+   */
+  function runTSLint(fix = true) {
+    const program = Linter.createProgram(join(tmpDir, 'tsconfig.json'));
+    const linter = new Linter({fix, rulesDirectory: [rulesDirectory]}, program);
+    const config = Configuration.parseConfigFile(
+        {rules: {'explicit-query-timing': true}, linterOptions: {typeCheck: true}});
+
+    program.getRootFileNames().forEach(fileName => {
+      linter.lint(fileName, program.getSourceFile(fileName) !.getFullText(), config);
+    });
+
+    return linter;
+  }
+
+  /** Writes a file to the current temporary directory. */
+  function writeFile(fileName: string, content: string) {
+    writeFileSync(join(tmpDir, fileName), content);
+  }
+
+  /** Expects a given file in the temporary directory to contain the specified string. */
+  function expectFileToContain(fileName: string, match: string) {
+    expect(readFileSync(join(tmpDir, fileName), 'utf8')).toContain(match);
+  }
+
+  it('should properly apply query timing replacements', () => {
+    writeFile('index.ts', `
+      import {Component, ViewChild} from '@angular/core';
+      
+      @Component({template: '<span #test></span>'})
+      export class MyComp {
+        @ViewChild('test') query: any;
+        @ViewChild('test') query2: any;
+        @ViewChild('test') query3: any;
+        
+        ngAfterContentInit() {
+          this.query.classList.add('test');
+        }
+      }
+    `);
+
+    writeFile('external.ts', `
+      import {MyComp} from './index';
+    
+      export class Test extends MyComp {
+        ngOnInit() {
+          this.query3.doSomething();
+        }
+      }
+    `);
+
+    runTSLint();
+
+    expectFileToContain('index.ts', `@ViewChild('test', { static: true }) query: any;`);
+    expectFileToContain('index.ts', `@ViewChild('test', { static: false }) query2: any;`);
+    expectFileToContain('index.ts', `@ViewChild('test', { static: true }) query3: any;`);
+  });
+
+  it('should report non-explicit static query definitions', () => {
+    writeFile('index.ts', `
+      import {Component, ViewChild} from '@angular/core';
+      
+      @Component({template: '<span #test></span>'})
+      export class MyComp {
+        @ViewChild('test') query: any;
+        
+        ngAfterContentInit() {
+          this.query.classList.add('test');
+        }
+      }
+    `);
+
+    const linter = runTSLint(false);
+    const failures = linter.getResult().failures;
+
+    expect(failures.length).toBe(1);
+    expect(failures[0].getFailure()).toContain('Query is not explicitly marked as "static"');
+  });
+
+  it('should report non-explicit dynamic query definitions', () => {
+    writeFile('index.ts', `
+      import {Component, ContentChild} from '@angular/core';
+      
+      @Component({template: '<span #test></span>'})
+      export class MyComp {
+        @ContentChild('test') query: any;
+      }
+    `);
+
+    const linter = runTSLint(false);
+    const failures = linter.getResult().failures;
+
+    expect(failures.length).toBe(1);
+    expect(failures[0].getFailure()).toContain('Query is not explicitly marked as "dynamic"');
+  });
+});

packages/core/schematics/utils/BUILD.bazel

@@ -2,10 +2,7 @@ load("//tools:defaults.bzl", "ts_library")
 
 ts_library(
     name = "utils",
-    srcs = glob(
+    srcs = glob(["**/*.ts"]),
-        ["**/*.ts"],
-        exclude = ["**/*_spec.ts"],
-    ),
     tsconfig = "//packages/core/schematics:tsconfig.json",
     visibility = ["//packages/core/schematics:__subpackages__"],
     deps = [