Skip to content

Commit

Permalink
feat(core): template-var-assignment update schematic (angular#29608)
Browse files Browse the repository at this point in the history
Introduces a new update schematic called "template-var-assignment"
that is responsible for analyzing template files in order to warn
developers if template variables are assigned to values.

The schematic also comes with a driver for `tslint` so that the
check can be used wtihin Google.

PR Close angular#29608
  • Loading branch information
devversion authored and wKoza committed Apr 17, 2019
1 parent 375f64a commit a629c13
Show file tree
Hide file tree
Showing 17 changed files with 849 additions and 2 deletions.
5 changes: 5 additions & 0 deletions packages/core/schematics/migrations.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
}
Original file line number Diff line number Diff line change
@@ -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",
],
)
Original file line number Diff line number Diff line change
@@ -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));
}
Original file line number Diff line number Diff line change
@@ -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<string, ResolvedTemplate>();

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),
});
}
});
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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",
],
)
Original file line number Diff line number Diff line change
@@ -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;
}
}
Loading

0 comments on commit a629c13

Please sign in to comment.