feat(core): template-var-assignment update schematic (#29608)
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 #29608
This commit is contained in:
Paul Gschwendtner
committed by
Jason Aden
Jason Aden
parent
15eb1e0ce1
commit
7c8f4e3202
@ -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",
|
||||
|
||||
@ -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: '<span *ngFor="let i of options" [(a)]="i"></span>'})
|
||||
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`, `
|
||||
<span *ngFor="let option of options" [(a)]="option"></span>
|
||||
`);
|
||||
|
||||
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`, `
|
||||
<!-- Comment -->
|
||||
<span *ngFor="let option of options" (click)="option = true"></span>
|
||||
`);
|
||||
|
||||
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./);
|
||||
});
|
||||
});
|
||||
30
packages/core/schematics/test/line_mappings_spec.ts
Normal file
30
packages/core/schematics/test/line_mappings_spec.ts
Normal file
@ -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,
|
||||
});
|
||||
});
|
||||
});
|
||||
@ -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: '<cmp *ngFor="let optionName of options" [(opt)]="optionName"></cmp>',
|
||||
})
|
||||
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', `
|
||||
<div *ngIf="somePartner() | async as partner">
|
||||
<some-comp [(value)]="partner"></some-comp>
|
||||
</div>
|
||||
`);
|
||||
|
||||
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', `
|
||||
<div *ngIf="true as visible">
|
||||
<div (click)="visible=false">Hide</div>
|
||||
<div (click)="visible=true">Show</div>
|
||||
</div>
|
||||
`);
|
||||
|
||||
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', `
|
||||
<ng-template let-visible="false">
|
||||
<div (click)="visible=false">Hide</div>
|
||||
<div (click)="visible=true">Show</div>
|
||||
</ng-template>
|
||||
`);
|
||||
|
||||
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', `<button (click)="myProp = true"></button>`);
|
||||
|
||||
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', `<x (click)="<invalid-syntax>"></x>`);
|
||||
|
||||
runMigration();
|
||||
|
||||
expect(consoleOutput.length).toBe(0);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user