diff --git a/packages/core/schematics/migrations/template-var-assignment/analyze_template.ts b/packages/core/schematics/migrations/template-var-assignment/analyze_template.ts index c140770232..fbd2b69580 100644 --- a/packages/core/schematics/migrations/template-var-assignment/analyze_template.ts +++ b/packages/core/schematics/migrations/template-var-assignment/analyze_template.ts @@ -7,12 +7,10 @@ */ import {PropertyWrite} from '@angular/compiler'; -import {Variable, visitAll} from '@angular/compiler/src/render3/r3_ast'; - +import {visitAll} from '@angular/compiler/src/render3/r3_ast'; import {ResolvedTemplate} from '../../utils/ng_component_template'; import {parseHtmlGracefully} from '../../utils/parse_html'; - -import {PropertyAssignment, PropertyWriteHtmlVisitor} from './angular/property_write_html_visitor'; +import {HtmlVariableAssignmentVisitor} from './angular/html_variable_assignment_visitor'; export interface TemplateVariableAssignment { node: PropertyWrite; @@ -32,20 +30,11 @@ export function analyzeResolvedTemplate(template: ResolvedTemplate): TemplateVar return null; } - const visitor = new PropertyWriteHtmlVisitor(); + const visitor = new HtmlVariableAssignmentVisitor(); - // Analyze the Angular Render3 HTML AST and collect all property assignments and - // template variables. + // Analyze the Angular Render3 HTML AST and collect all template variable assignments. visitAll(visitor, templateNodes); - return filterTemplateVariableAssignments(visitor.propertyAssignments, visitor.templateVariables) - .map(({node, start, end}) => ({node, start: start + node.span.start, end})); -} - -/** - * 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)); + return visitor.variableAssignments.map( + ({node, start, end}) => ({node, start: start + node.span.start, end})); } diff --git a/packages/core/schematics/migrations/template-var-assignment/angular/html_variable_assignment_visitor.ts b/packages/core/schematics/migrations/template-var-assignment/angular/html_variable_assignment_visitor.ts new file mode 100644 index 0000000000..07973ef2ee --- /dev/null +++ b/packages/core/schematics/migrations/template-var-assignment/angular/html_variable_assignment_visitor.ts @@ -0,0 +1,79 @@ +/** + * @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 {ImplicitReceiver, ParseSourceSpan, PropertyWrite, RecursiveAstVisitor} from '@angular/compiler'; +import {BoundEvent, Element, NullVisitor, Template, Variable, visitAll} from '@angular/compiler/src/render3/r3_ast'; + +export interface TemplateVariableAssignment { + start: number; + end: number; + node: PropertyWrite; +} + +/** + * HTML AST visitor that traverses the Render3 HTML AST in order to find all + * expressions that write to local template variables within bound events. + */ +export class HtmlVariableAssignmentVisitor extends NullVisitor { + variableAssignments: TemplateVariableAssignment[] = []; + + private currentVariables: Variable[] = []; + private expressionAstVisitor = + new ExpressionAstVisitor(this.variableAssignments, this.currentVariables); + + visitElement(element: Element): void { + visitAll(this, element.outputs); + visitAll(this, element.children); + } + + visitTemplate(template: Template): void { + // Keep track of the template variables which can be accessed by the template + // child nodes through the implicit receiver. + this.currentVariables.push(...template.variables); + + // 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); + + // Remove all previously added variables since all children that could access + // these have been visited already. + template.variables.forEach(v => { + const variableIdx = this.currentVariables.indexOf(v); + + if (variableIdx !== -1) { + this.currentVariables.splice(variableIdx, 1); + } + }); + } + + visitBoundEvent(node: BoundEvent) { + node.handler.visit(this.expressionAstVisitor, node.handlerSpan); + } +} + +/** AST visitor that resolves all variable assignments within a given expression AST. */ +class ExpressionAstVisitor extends RecursiveAstVisitor { + constructor( + private variableAssignments: TemplateVariableAssignment[], + private currentVariables: Variable[]) { + super(); + } + + visitPropertyWrite(node: PropertyWrite, span: ParseSourceSpan) { + if (node.receiver instanceof ImplicitReceiver && + this.currentVariables.some(v => v.name === node.name)) { + this.variableAssignments.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/angular/property_write_html_visitor.ts b/packages/core/schematics/migrations/template-var-assignment/angular/property_write_html_visitor.ts deleted file mode 100644 index 333edddb70..0000000000 --- a/packages/core/schematics/migrations/template-var-assignment/angular/property_write_html_visitor.ts +++ /dev/null @@ -1,61 +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 {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/test/template_var_assignment_migration_spec.ts b/packages/core/schematics/test/template_var_assignment_migration_spec.ts index 87a3ba4f9e..810d187c9e 100644 --- a/packages/core/schematics/test/template_var_assignment_migration_spec.ts +++ b/packages/core/schematics/test/template_var_assignment_migration_spec.ts @@ -181,6 +181,55 @@ describe('template variable assignment migration', () => { expect(warnOutput.length).toBe(0); }); + it('should not warn for property writes with template variable name but different receiver', + () => { + writeFile('/index.ts', ` + import {Component} from '@angular/core'; + + @Component({ + templateUrl: './sub_dir/tmpl.html', + }) + export class MyComp { + someProp = { + element: 'someValue', + }; + } + `); + + writeFile('/sub_dir/tmpl.html', ` + + `); + + runMigration(); + + expect(warnOutput.length).toBe(0); + }); + + it('should not warn for property writes with template variable name but different scope', () => { + writeFile('/index.ts', ` + import {Component} from '@angular/core'; + + @Component({ + templateUrl: './sub_dir/tmpl.html', + }) + export class MyComp { + element = 'someValue'; + } + `); + + writeFile('/sub_dir/tmpl.html', ` + + + `); + + runMigration(); + + expect(warnOutput.length).toBe(0); + }); + + it('should not throw an error if a detected template fails parsing', () => { writeFile('/index.ts', ` import {Component} from '@angular/core';