diff --git a/packages/core/schematics/migrations.json b/packages/core/schematics/migrations.json index 8293d9f2e8..f4fe65f4b6 100644 --- a/packages/core/schematics/migrations.json +++ b/packages/core/schematics/migrations.json @@ -4,6 +4,11 @@ "version": "8", "description": "Migrates ViewChild and ContentChild to explicit query timing", "factory": "./migrations/static-queries/index" + }, + "migration-v8-template-local-variables": { + "version": "8", + "description": "Warns developers if values are assigned to template variables", + "factory": "./migrations/template-var-assignment/index" } } } diff --git a/packages/core/schematics/migrations/template-var-assignment/BUILD.bazel b/packages/core/schematics/migrations/template-var-assignment/BUILD.bazel new file mode 100644 index 0000000000..a45b58f070 --- /dev/null +++ b/packages/core/schematics/migrations/template-var-assignment/BUILD.bazel @@ -0,0 +1,19 @@ +load("//tools:defaults.bzl", "ts_library") + +ts_library( + name = "template-var-assignment", + srcs = glob(["**/*.ts"]), + tsconfig = "//packages/core/schematics:tsconfig.json", + visibility = [ + "//packages/core/schematics:__pkg__", + "//packages/core/schematics/migrations/template-var-assignment/google3:__pkg__", + "//packages/core/schematics/test:__pkg__", + ], + deps = [ + "//packages/compiler", + "//packages/core/schematics/utils", + "@npm//@angular-devkit/schematics", + "@npm//@types/node", + "@npm//typescript", + ], +) diff --git a/packages/core/schematics/migrations/template-var-assignment/analyze_template.ts b/packages/core/schematics/migrations/template-var-assignment/analyze_template.ts new file mode 100644 index 0000000000..36c2cc9b25 --- /dev/null +++ b/packages/core/schematics/migrations/template-var-assignment/analyze_template.ts @@ -0,0 +1,51 @@ +/** + * @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 {PropertyWrite, parseTemplate} from '@angular/compiler'; +import {Variable, visitAll} from '@angular/compiler/src/render3/r3_ast'; + +import {ResolvedTemplate} from './angular/ng_component_template'; +import {PropertyAssignment, PropertyWriteHtmlVisitor} from './angular/property_write_html_visitor'; + +export interface TemplateVariableAssignment { + node: PropertyWrite; + start: number; + end: number; +} + +/** + * Analyzes a given resolved template by looking for property assignments to local + * template variables within bound events. + */ +export function analyzeResolvedTemplate( + filePath: string, template: ResolvedTemplate): TemplateVariableAssignment[]|null { + try { + const templateNodes = parseTemplate(template.content, filePath).nodes; + const visitor = new PropertyWriteHtmlVisitor(); + + // Analyze the Angular Render3 HTML AST and collect all property assignments and + // template variables. + visitAll(visitor, templateNodes); + + return filterTemplateVariableAssignments(visitor.propertyAssignments, visitor.templateVariables) + .map(({node, start, end}) => ({node, start: start + node.span.start, end})); + } catch { + // Do nothing if the template couldn't be parsed. We don't want to throw any + // exception if a template is syntactically not valid. e.g. template could be + // using preprocessor syntax. + return null; + } +} + +/** + * Returns all template variable assignments by looking if a given property + * assignment is setting the value for one of the specified template variables. + */ +function filterTemplateVariableAssignments(writes: PropertyAssignment[], variables: Variable[]) { + return writes.filter(propertyWrite => !!variables.find(v => v.name === propertyWrite.node.name)); +} diff --git a/packages/core/schematics/migrations/template-var-assignment/angular/ng_component_template.ts b/packages/core/schematics/migrations/template-var-assignment/angular/ng_component_template.ts new file mode 100644 index 0000000000..0296ed238f --- /dev/null +++ b/packages/core/schematics/migrations/template-var-assignment/angular/ng_component_template.ts @@ -0,0 +1,126 @@ +/** + * @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 {existsSync, readFileSync} from 'fs'; +import {dirname, resolve} from 'path'; +import * as ts from 'typescript'; + +import {computeLineStartsMap, getLineAndCharacterFromPosition} from '../../../utils/line_mappings'; +import {getAngularDecorators} from '../../../utils/ng_decorators'; +import {unwrapExpression} from '../../../utils/typescript/functions'; +import {getPropertyNameText} from '../../../utils/typescript/property_name'; + +export interface ResolvedTemplate { + /** File content of the given template. */ + content: string; + /** Start offset of the template content (e.g. in the inline source file) */ + start: number; + /** Whether the given template is inline or not. */ + inline: boolean; + /** + * Gets the character and line of a given position index in the template. + * If the template is declared inline within a TypeScript source file, the line and + * character are based on the full source file content. + */ + getCharacterAndLineOfPosition: (pos: number) => { character: number, line: number }; +} + +/** + * Visitor that can be used to determine Angular templates referenced within given + * TypeScript source files (inline templates or external referenced templates) + */ +export class NgComponentTemplateVisitor { + resolvedTemplates = new Map(); + + constructor(public typeChecker: ts.TypeChecker) {} + + visitNode(node: ts.Node) { + switch (node.kind) { + case ts.SyntaxKind.ClassDeclaration: + this.visitClassDeclaration(node as ts.ClassDeclaration); + break; + } + + ts.forEachChild(node, node => this.visitNode(node)); + } + + private visitClassDeclaration(node: ts.ClassDeclaration) { + if (!node.decorators || !node.decorators.length) { + return; + } + + const ngDecorators = getAngularDecorators(this.typeChecker, node.decorators); + const componentDecorator = ngDecorators.find(dec => dec.name === 'Component'); + + // In case no "@Component" decorator could be found on the current class, skip. + if (!componentDecorator) { + return; + } + + const decoratorCall = componentDecorator.node.expression; + + // In case the component decorator call is not valid, skip this class declaration. + if (decoratorCall.arguments.length !== 1) { + return; + } + + const componentMetadata = unwrapExpression(decoratorCall.arguments[0]); + + // Ensure that the component metadata is an object literal expression. + if (!ts.isObjectLiteralExpression(componentMetadata)) { + return; + } + + const sourceFile = node.getSourceFile(); + const sourceFileName = sourceFile.fileName; + + // Walk through all component metadata properties and determine the referenced + // HTML templates (either external or inline) + componentMetadata.properties.forEach(property => { + if (!ts.isPropertyAssignment(property)) { + return; + } + + const propertyName = getPropertyNameText(property.name); + + // In case there is an inline template specified, ensure that the value is statically + // analyzable by checking if the initializer is a string literal-like node. + if (propertyName === 'template' && ts.isStringLiteralLike(property.initializer)) { + // Need to add an offset of one to the start because the template quotes are + // not part of the template content. + const templateStartIdx = property.initializer.getStart() + 1; + this.resolvedTemplates.set(resolve(sourceFileName), { + content: property.initializer.text, + inline: true, + start: templateStartIdx, + getCharacterAndLineOfPosition: + pos => ts.getLineAndCharacterOfPosition(sourceFile, pos + templateStartIdx) + }); + } + if (propertyName === 'templateUrl' && ts.isStringLiteralLike(property.initializer)) { + const templatePath = resolve(dirname(sourceFileName), property.initializer.text); + + // In case the template does not exist in the file system, skip this + // external template. + if (!existsSync(templatePath)) { + return; + } + + const fileContent = readFileSync(templatePath, 'utf8'); + const lineStartsMap = computeLineStartsMap(fileContent); + + this.resolvedTemplates.set(templatePath, { + content: fileContent, + inline: false, + start: 0, + getCharacterAndLineOfPosition: pos => getLineAndCharacterFromPosition(lineStartsMap, pos), + }); + } + }); + } +} diff --git a/packages/core/schematics/migrations/template-var-assignment/angular/property_write_html_visitor.ts b/packages/core/schematics/migrations/template-var-assignment/angular/property_write_html_visitor.ts new file mode 100644 index 0000000000..333edddb70 --- /dev/null +++ b/packages/core/schematics/migrations/template-var-assignment/angular/property_write_html_visitor.ts @@ -0,0 +1,61 @@ +/** + * @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 {ParseSourceSpan, PropertyWrite, RecursiveAstVisitor} from '@angular/compiler'; +import {BoundEvent, Element, NullVisitor, Template, Variable, visitAll} from '@angular/compiler/src/render3/r3_ast'; + +export interface PropertyAssignment { + start: number; + end: number; + node: PropertyWrite; +} + +/** + * AST visitor that traverses the Render3 HTML AST in order to find all declared + * template variables and property assignments within bound events. + */ +export class PropertyWriteHtmlVisitor extends NullVisitor { + templateVariables: Variable[] = []; + propertyAssignments: PropertyAssignment[] = []; + + private expressionAstVisitor = new ExpressionAstVisitor(this.propertyAssignments); + + visitElement(element: Element): void { + visitAll(this, element.outputs); + visitAll(this, element.children); + } + + visitTemplate(template: Template): void { + // Visit all children of the template. The template proxies the outputs of the + // immediate child elements, so we just ignore outputs on the "Template" in order + // to not visit similar bound events twice. + visitAll(this, template.children); + + // Keep track of all declared local template variables. + this.templateVariables.push(...template.variables); + } + + visitBoundEvent(node: BoundEvent) { + node.handler.visit(this.expressionAstVisitor, node.handlerSpan); + } +} + +/** AST visitor that resolves all property assignments with a given expression AST. */ +class ExpressionAstVisitor extends RecursiveAstVisitor { + constructor(private propertyAssignments: PropertyAssignment[]) { super(); } + + visitPropertyWrite(node: PropertyWrite, span: ParseSourceSpan) { + this.propertyAssignments.push({ + node: node, + start: span.start.offset, + end: span.end.offset, + }); + + super.visitPropertyWrite(node, span); + } +} diff --git a/packages/core/schematics/migrations/template-var-assignment/google3/BUILD.bazel b/packages/core/schematics/migrations/template-var-assignment/google3/BUILD.bazel new file mode 100644 index 0000000000..ff26637f69 --- /dev/null +++ b/packages/core/schematics/migrations/template-var-assignment/google3/BUILD.bazel @@ -0,0 +1,13 @@ +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/template-var-assignment", + "//packages/core/schematics/utils/tslint", + "@npm//tslint", + ], +) diff --git a/packages/core/schematics/migrations/template-var-assignment/google3/noTemplateVariableAssignmentRule.ts b/packages/core/schematics/migrations/template-var-assignment/google3/noTemplateVariableAssignmentRule.ts new file mode 100644 index 0000000000..a89e0ea30c --- /dev/null +++ b/packages/core/schematics/migrations/template-var-assignment/google3/noTemplateVariableAssignmentRule.ts @@ -0,0 +1,53 @@ +/** + * @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 {RuleFailure, Rules} from 'tslint'; +import * as ts from 'typescript'; + +import {createHtmlSourceFile} from '../../../utils/tslint/tslint_html_source_file'; +import {analyzeResolvedTemplate} from '../analyze_template'; +import {NgComponentTemplateVisitor} from '../angular/ng_component_template'; + +const FAILURE_MESSAGE = 'Found assignment to template variable. This does not work with Ivy and ' + + 'needs to be updated.'; + +/** + * Rule that reports if an Angular template contains property assignments to template variables. + */ +export class Rule extends Rules.TypedRule { + applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] { + const typeChecker = program.getTypeChecker(); + const templateVisitor = new NgComponentTemplateVisitor(typeChecker); + const failures: RuleFailure[] = []; + + // Analyze the current source files by detecting all referenced HTML templates. + templateVisitor.visitNode(sourceFile); + + const {resolvedTemplates} = templateVisitor; + + // Analyze each resolved template and print a warning for property writes to + // template variables. + resolvedTemplates.forEach((template, filePath) => { + const nodes = analyzeResolvedTemplate(filePath, template); + const templateFile = + template.inline ? sourceFile : createHtmlSourceFile(filePath, template.content); + + if (!nodes) { + return; + } + + nodes.forEach(n => { + failures.push(new RuleFailure( + templateFile, template.start + n.start, template.start + n.end, FAILURE_MESSAGE, + this.ruleName)); + }); + }); + + return failures; + } +} diff --git a/packages/core/schematics/migrations/template-var-assignment/index.ts b/packages/core/schematics/migrations/template-var-assignment/index.ts new file mode 100644 index 0000000000..2adc588d11 --- /dev/null +++ b/packages/core/schematics/migrations/template-var-assignment/index.ts @@ -0,0 +1,84 @@ +/** + * @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 {logging, normalize} from '@angular-devkit/core'; +import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics'; +import {dirname, relative} from 'path'; +import * as ts from 'typescript'; + +import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths'; +import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig'; + +import {analyzeResolvedTemplate} from './analyze_template'; +import {NgComponentTemplateVisitor} from './angular/ng_component_template'; + +type Logger = logging.LoggerApi; + +/** Entry point for the V8 template variable assignment schematic. */ +export default function(): Rule { + return (tree: Tree, context: SchematicContext) => { + const projectTsConfigPaths = getProjectTsConfigPaths(tree); + const basePath = process.cwd(); + + if (!projectTsConfigPaths.length) { + throw new SchematicsException( + 'Could not find any tsconfig file. Cannot check templates for template variable ' + + 'assignments.'); + } + + for (const tsconfigPath of projectTsConfigPaths) { + runTemplateVariableAssignmentCheck(tree, tsconfigPath, basePath, context.logger); + } + }; +} + +/** + * Runs the template variable assignment check. Warns developers + * if values are assigned to template variables within output bindings. + */ +function runTemplateVariableAssignmentCheck( + tree: Tree, tsconfigPath: string, basePath: string, logger: Logger) { + 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. + 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 templateVisitor = new NgComponentTemplateVisitor(typeChecker); + const rootSourceFiles = program.getRootFileNames().map(f => program.getSourceFile(f) !); + + // Analyze source files by detecting HTML templates. + rootSourceFiles.forEach(sourceFile => templateVisitor.visitNode(sourceFile)); + + const {resolvedTemplates} = templateVisitor; + + // Analyze each resolved template and print a warning for property writes to + // template variables. + resolvedTemplates.forEach((template, filePath) => { + const nodes = analyzeResolvedTemplate(filePath, template); + + if (!nodes) { + return; + } + + const displayFilePath = normalize(relative(basePath, filePath)); + + nodes.forEach(n => { + const {line, character} = template.getCharacterAndLineOfPosition(n.start); + logger.warn( + `${displayFilePath}@${line + 1}:${character + 1}: Found assignment to template ` + + `variable. This does not work with Ivy and needs to be updated.`); + }); + }); +} diff --git a/packages/core/schematics/test/BUILD.bazel b/packages/core/schematics/test/BUILD.bazel index 962ff56adc..b1dd0643d4 100644 --- a/packages/core/schematics/test/BUILD.bazel +++ b/packages/core/schematics/test/BUILD.bazel @@ -11,6 +11,8 @@ ts_library( deps = [ "//packages/core/schematics/migrations/static-queries", "//packages/core/schematics/migrations/static-queries/google3", + "//packages/core/schematics/migrations/template-var-assignment", + "//packages/core/schematics/migrations/template-var-assignment/google3", "//packages/core/schematics/utils", "@npm//@angular-devkit/schematics", "@npm//@types/shelljs", diff --git a/packages/core/schematics/test/google3/no_template_variable_assignment_rule_spec.ts b/packages/core/schematics/test/google3/no_template_variable_assignment_rule_spec.ts new file mode 100644 index 0000000000..1f878e64a7 --- /dev/null +++ b/packages/core/schematics/test/google3/no_template_variable_assignment_rule_spec.ts @@ -0,0 +1,110 @@ +/** + * @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 {writeFileSync} from 'fs'; +import {dirname, join} from 'path'; +import * as shx from 'shelljs'; +import {Configuration, Linter} from 'tslint'; + +describe('Google3 noTemplateVariableAssignment TSLint rule', () => { + const rulesDirectory = dirname(require.resolve( + '../../migrations/template-var-assignment/google3/noTemplateVariableAssignmentRule')); + + 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 no-template-variable TSLint rule.*/ + function runTSLint() { + const program = Linter.createProgram(join(tmpDir, 'tsconfig.json')); + const linter = new Linter({fix: false, rulesDirectory: [rulesDirectory]}, program); + const config = Configuration.parseConfigFile( + {rules: {'no-template-variable-assignment': 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); + } + + it('should create failure for detected two-way data binding assignment', () => { + writeFile('index.ts', ` + import {Component} from '@angular/core'; + + @Component({template: ''}) + export class MyComp {} + `); + + const linter = runTSLint(); + const failures = linter.getResult().failures; + + expect(failures.length).toBe(1); + expect(failures[0].getFileName()).toContain('index.ts'); + expect(failures[0].getStartPosition().getLineAndCharacter()).toEqual({line: 3, character: 68}); + expect(failures[0].getEndPosition().getLineAndCharacter()).toEqual({line: 3, character: 69}); + expect(failures[0].getFailure()).toMatch(/^Found assignment to template variable./); + }); + + it('should create failure with correct offsets for external templates', () => { + writeFile('index.ts', ` + import {Component} from '@angular/core'; + + @Component({templateUrl: './my-tmpl.html'}) + export class MyComp {} + `); + + writeFile(`my-tmpl.html`, ` + + `); + + const linter = runTSLint(); + const failures = linter.getResult().failures; + + expect(failures.length).toBe(1); + expect(failures[0].getFileName()).toContain('my-tmpl.html'); + expect(failures[0].getStartPosition().getLineAndCharacter()).toEqual({line: 1, character: 50}); + expect(failures[0].getEndPosition().getLineAndCharacter()).toEqual({line: 1, character: 56}); + expect(failures[0].getFailure()).toMatch(/^Found assignment to template variable./); + }); + + it('should create failure for template variable assignment within output', () => { + writeFile('index.ts', ` + import {Component} from '@angular/core'; + + @Component({templateUrl: './my-tmpl.html'}) + export class MyComp {} + `); + + writeFile(`my-tmpl.html`, ` + + + `); + + const linter = runTSLint(); + const failures = linter.getResult().failures; + + expect(failures.length).toBe(1); + expect(failures[0].getFileName()).toContain('my-tmpl.html'); + expect(failures[0].getStartPosition().getLineAndCharacter()).toEqual({line: 2, character: 52}); + expect(failures[0].getEndPosition().getLineAndCharacter()).toEqual({line: 2, character: 65}); + expect(failures[0].getFailure()).toMatch(/^Found assignment to template variable./); + }); +}); diff --git a/packages/core/schematics/test/line_mappings_spec.ts b/packages/core/schematics/test/line_mappings_spec.ts new file mode 100644 index 0000000000..a9aa66fb5b --- /dev/null +++ b/packages/core/schematics/test/line_mappings_spec.ts @@ -0,0 +1,30 @@ +/** + * @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 {computeLineStartsMap, getLineAndCharacterFromPosition} from '../utils/line_mappings'; + +describe('line mappings', () => { + + it('should properly compute line starts', + () => { + expect(computeLineStartsMap(` + 1 + 2`)).toEqual([0, 1, 9, 16]); + }); + + it('should properly get line and character from line starts', () => { + const lineStarts = computeLineStartsMap(` + 1 + 2`); + + expect(getLineAndCharacterFromPosition(lineStarts, 8)).toEqual({ + line: 1, + character: 7, + }); + }); +}); diff --git a/packages/core/schematics/test/template_var_assignment_migration_spec.ts b/packages/core/schematics/test/template_var_assignment_migration_spec.ts new file mode 100644 index 0000000000..918459a2da --- /dev/null +++ b/packages/core/schematics/test/template_var_assignment_migration_spec.ts @@ -0,0 +1,177 @@ +/** + * @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 {getSystemPath, normalize, virtualFs} from '@angular-devkit/core'; +import {TempScopedNodeJsSyncHost} from '@angular-devkit/core/node/testing'; +import {HostTree} from '@angular-devkit/schematics'; +import {SchematicTestRunner, UnitTestTree} from '@angular-devkit/schematics/testing'; +import * as shx from 'shelljs'; + +describe('template variable assignment migration', () => { + let runner: SchematicTestRunner; + let host: TempScopedNodeJsSyncHost; + let tree: UnitTestTree; + let tmpDirPath: string; + let previousWorkingDir: string; + let consoleOutput: string[]; + + beforeEach(() => { + runner = new SchematicTestRunner('test', require.resolve('../migrations.json')); + host = new TempScopedNodeJsSyncHost(); + tree = new UnitTestTree(new HostTree(host)); + + writeFile('/tsconfig.json', JSON.stringify({ + compilerOptions: { + lib: ['es2015'], + } + })); + + consoleOutput = []; + runner.logger.subscribe(logEntry => consoleOutput.push(logEntry.message)); + + previousWorkingDir = shx.pwd(); + tmpDirPath = getSystemPath(host.root); + + // Switch into the temporary directory path. This allows us to run + // the schematic against our custom unit test tree. + shx.cd(tmpDirPath); + }); + + afterEach(() => { + shx.cd(previousWorkingDir); + shx.rm('-r', tmpDirPath); + }); + + function writeFile(filePath: string, contents: string) { + host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents)); + } + + function runMigration() { + runner.runSchematic('migration-v8-template-local-variables', {}, tree); + } + + it('should warn for two-way data binding variable assignment', () => { + writeFile('/index.ts', ` + import {Component} from '@angular/core'; + + @Component({ + template: '', + }) + export class MyComp {} + `); + + runMigration(); + + expect(consoleOutput.length).toBe(1); + expect(consoleOutput[0]).toMatch(/^index.ts@5:69: Found assignment/); + }); + + it('should warn for two-way data binding assigning to "as" variable', () => { + writeFile('/index.ts', ` + import {Component} from '@angular/core'; + + @Component({ + templateUrl: './tmpl.html', + }) + export class MyComp {} + `); + + writeFile('/tmpl.html', ` +
+ +
+ `); + + runMigration(); + + expect(consoleOutput.length).toBe(1); + expect(consoleOutput).toMatch(/^tmpl.html@3:31: Found assignment/); + }); + + it('should warn for bound event assignments to "as" variable', () => { + writeFile('/index.ts', ` + import {Component} from '@angular/core'; + + @Component({ + templateUrl: './sub_dir/tmpl.html', + }) + export class MyComp {} + `); + + writeFile('/sub_dir/tmpl.html', ` +
+
Hide
+
Show
+
+ `); + + runMigration(); + + expect(consoleOutput.length).toBe(2); + expect(consoleOutput[0]).toMatch(/^sub_dir\/tmpl.html@3:25: Found assignment/); + expect(consoleOutput[1]).toMatch(/^sub_dir\/tmpl.html@4:25: Found assignment/); + }); + + it('should warn for bound event assignments to template "let" variables', () => { + writeFile('/index.ts', ` + import {Component} from '@angular/core'; + + @Component({ + templateUrl: './sub_dir/tmpl.html', + }) + export class MyComp {} + `); + + writeFile('/sub_dir/tmpl.html', ` + +
Hide
+
Show
+ + `); + + runMigration(); + + expect(consoleOutput.length).toBe(2); + expect(consoleOutput[0]).toMatch(/^sub_dir\/tmpl.html@3:25: Found assignment/); + expect(consoleOutput[1]).toMatch(/^sub_dir\/tmpl.html@4:25: Found assignment/); + }); + + it('should not warn for bound event assignments to component property', () => { + writeFile('/index.ts', ` + import {Component} from '@angular/core'; + + @Component({ + templateUrl: './sub_dir/tmpl.html', + }) + export class MyComp {} + `); + + writeFile('/sub_dir/tmpl.html', ``); + + runMigration(); + + expect(consoleOutput.length).toBe(0); + }); + + it('should not throw an error if a detected template fails parsing', () => { + writeFile('/index.ts', ` + import {Component} from '@angular/core'; + + @Component({ + templateUrl: './sub_dir/tmpl.html', + }) + export class MyComp {} + `); + + writeFile('/sub_dir/tmpl.html', ``); + + runMigration(); + + expect(consoleOutput.length).toBe(0); + }); +}); diff --git a/packages/core/schematics/tsconfig.json b/packages/core/schematics/tsconfig.json index 6e6c7ac264..4674d61f7b 100644 --- a/packages/core/schematics/tsconfig.json +++ b/packages/core/schematics/tsconfig.json @@ -3,6 +3,14 @@ "strictNullChecks": true, "noImplicitReturns": true, "lib": ["es2015"], - "types": [] + "types": [], + "baseUrl": ".", + "paths": { + "@angular/compiler": ["../../compiler"], + "@angular/compiler/*": ["../../compiler/*"] + } + }, + "bazelOptions": { + "suppressTsconfigOverrideWarnings": true } } diff --git a/packages/core/schematics/utils/line_mappings.ts b/packages/core/schematics/utils/line_mappings.ts new file mode 100644 index 0000000000..8ff8f10cec --- /dev/null +++ b/packages/core/schematics/utils/line_mappings.ts @@ -0,0 +1,63 @@ +/** + * @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 + */ + +const LF_CHAR = 10; +const CR_CHAR = 13; +const LINE_SEP_CHAR = 8232; +const PARAGRAPH_CHAR = 8233; + +/** Gets the line and character for the given position from the line starts map. */ +export function getLineAndCharacterFromPosition(lineStartsMap: number[], position: number) { + const lineIndex = findClosestLineStartPosition(lineStartsMap, position); + return {character: position - lineStartsMap[lineIndex], line: lineIndex}; +} + +/** + * Computes the line start map of the given text. This can be used in order to + * retrieve the line and character of a given text position index. + */ +export function computeLineStartsMap(text: string): number[] { + const result: number[] = [0]; + let pos = 0; + while (pos < text.length) { + const char = text.charCodeAt(pos++); + // Handles the "CRLF" line break. In that case we peek the character + // after the "CR" and check if it is a line feed. + if (char === CR_CHAR) { + if (text.charCodeAt(pos) === LF_CHAR) { + pos++; + } + result.push(pos); + } else if (char === LF_CHAR || char === LINE_SEP_CHAR || char === PARAGRAPH_CHAR) { + result.push(pos); + } + } + result.push(pos); + return result; +} + +/** Finds the closest line start for the given position. */ +function findClosestLineStartPosition( + linesMap: T[], position: T, low = 0, high = linesMap.length - 1) { + while (low <= high) { + const pivotIdx = Math.floor((low + high) / 2); + const pivotEl = linesMap[pivotIdx]; + + if (pivotEl === position) { + return pivotIdx; + } else if (position > pivotEl) { + low = pivotIdx + 1; + } else { + high = pivotIdx - 1; + } + } + + // In case there was no exact match, return the closest "lower" line index. We also + // subtract the index by one because want the index of the previous line start. + return low - 1; +} diff --git a/packages/core/schematics/utils/ng_decorators.ts b/packages/core/schematics/utils/ng_decorators.ts index 550e2897b0..7e14a83e91 100644 --- a/packages/core/schematics/utils/ng_decorators.ts +++ b/packages/core/schematics/utils/ng_decorators.ts @@ -11,7 +11,7 @@ import {getCallDecoratorImport} from './typescript/decorators'; export type CallExpressionDecorator = ts.Decorator & { expression: ts.CallExpression; -} +}; export interface NgDecorator { name: string; diff --git a/packages/core/schematics/utils/tslint/BUILD.bazel b/packages/core/schematics/utils/tslint/BUILD.bazel new file mode 100644 index 0000000000..9f70c9f1e5 --- /dev/null +++ b/packages/core/schematics/utils/tslint/BUILD.bazel @@ -0,0 +1,8 @@ +load("//tools:defaults.bzl", "ts_library") + +ts_library( + name = "tslint", + srcs = glob(["**/*.ts"]), + tsconfig = "//packages/core/schematics:tsconfig.json", + visibility = ["//packages/core/schematics/migrations/template-var-assignment/google3:__pkg__"], +) diff --git a/packages/core/schematics/utils/tslint/tslint_html_source_file.ts b/packages/core/schematics/utils/tslint/tslint_html_source_file.ts new file mode 100644 index 0000000000..2b1db7372b --- /dev/null +++ b/packages/core/schematics/utils/tslint/tslint_html_source_file.ts @@ -0,0 +1,37 @@ +/** + * @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'; + +/** + * Creates a fake TypeScript source file that can contain content of templates or stylesheets. + * The fake TypeScript source file then can be passed to TSLint in combination with a rule failure. + */ +export function createHtmlSourceFile(filePath: string, content: string): ts.SourceFile { + const sourceFile = ts.createSourceFile(filePath, `\`${content}\``, ts.ScriptTarget.ES5); + + // Subtract two characters because the string literal quotes are only needed for parsing + // and are not part of the actual source file. + sourceFile.end = sourceFile.end - 2; + + // Note: This does not affect the way TSLint applies replacements for external resource files. + // At the time of writing, TSLint loads files manually if the actual rule source file is not + // equal to the source file of the replacement. This means that the replacements need proper + // offsets without the string literal quote symbols. + sourceFile.getFullText = function() { + return sourceFile.text.substring(1, sourceFile.text.length - 1); + }; + + sourceFile.getText = sourceFile.getFullText; + + // Update the "text" property to be set to the template content without quotes. This is + // necessary so that the TypeScript line starts map is properly computed. + sourceFile.text = sourceFile.getFullText(); + + return sourceFile; +}