diff --git a/package.json b/package.json index c80ed56309..01cebc7c1f 100644 --- a/package.json +++ b/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", diff --git a/packages/core/schematics/migrations/static-queries/BUILD.bazel b/packages/core/schematics/migrations/static-queries/BUILD.bazel index 7c2f0e9c63..d5211ce3b1 100644 --- a/packages/core/schematics/migrations/static-queries/BUILD.bazel +++ b/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( - ["**/*.ts"], - exclude = ["index_spec.ts"], - ), + srcs = glob(["**/*.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 = [ diff --git a/packages/core/schematics/migrations/static-queries/google3/BUILD.bazel b/packages/core/schematics/migrations/static-queries/google3/BUILD.bazel new file mode 100644 index 0000000000..05a86b63ad --- /dev/null +++ b/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", + ], +) diff --git a/packages/core/schematics/migrations/static-queries/google3/explicitQueryTimingRule.ts b/packages/core/schematics/migrations/static-queries/google3/explicitQueryTimingRule.ts new file mode 100644 index 0000000000..ea3bfa9b1b --- /dev/null +++ b/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; + } +} diff --git a/packages/core/schematics/migrations/static-queries/index.ts b/packages/core/schematics/migrations/static-queries/index.ts index e0149a4fb2..9cf086eadc 100644 --- a/packages/core/schematics/migrations/static-queries/index.ts +++ b/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); + }); +} diff --git a/packages/core/schematics/migrations/static-queries/migration.ts b/packages/core/schematics/migrations/static-queries/migration.ts deleted file mode 100644 index 57b7822119..0000000000 --- a/packages/core/schematics/migrations/static-queries/migration.ts +++ /dev/null @@ -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); -} diff --git a/packages/core/schematics/migrations/static-queries/transform.ts b/packages/core/schematics/migrations/static-queries/transform.ts new file mode 100644 index 0000000000..29cb187ae3 --- /dev/null +++ b/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])]); +} diff --git a/packages/core/schematics/test/BUILD.bazel b/packages/core/schematics/test/BUILD.bazel index fd1877312e..962ff56adc 100644 --- a/packages/core/schematics/test/BUILD.bazel +++ b/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", ], ) diff --git a/packages/core/schematics/test/google3/explicit_query_timing_rule_spec.ts b/packages/core/schematics/test/google3/explicit_query_timing_rule_spec.ts new file mode 100644 index 0000000000..97b7c99466 --- /dev/null +++ b/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: ''}) + 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: ''}) + 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: ''}) + 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"'); + }); +}); diff --git a/packages/core/schematics/utils/BUILD.bazel b/packages/core/schematics/utils/BUILD.bazel index df0004600e..02482f4f02 100644 --- a/packages/core/schematics/utils/BUILD.bazel +++ b/packages/core/schematics/utils/BUILD.bazel @@ -2,10 +2,7 @@ load("//tools:defaults.bzl", "ts_library") ts_library( name = "utils", - srcs = glob( - ["**/*.ts"], - exclude = ["**/*_spec.ts"], - ), + srcs = glob(["**/*.ts"]), tsconfig = "//packages/core/schematics:tsconfig.json", visibility = ["//packages/core/schematics:__subpackages__"], deps = [