refactor(compiler-cli): add ExtendedTemplateChecker (#43107)

Change the current way to run template checks to the
`ExtendedTemplateChecker` instead of just the
`getExtendedTemplateDiagnosticsForComponent` function. Refactored the
tests that used the previous function to use the new class.

Refs #42966

PR Close #43107
This commit is contained in:
Daniel Trevino 2021-08-17 16:50:10 +00:00 committed by Alex Rickabaugh
parent bed121c34f
commit 3932480ddb
5 changed files with 82 additions and 49 deletions

View file

@ -0,0 +1,19 @@
/**
* @license
* Copyright Google LLC 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';
/**
* Interface to generate extended template diangostics from the component tempaltes.
*/
export interface ExtendedTemplateChecker {
/**
* Run `TemplateCheck`s for a component and return the generated `ts.Diagnostic`s.
*/
getExtendedTemplateDiagnosticsForComponent(component: ts.ClassDeclaration): ts.Diagnostic[];
}

View file

@ -0,0 +1,10 @@
/**
* @license
* Copyright Google LLC 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
*/
export * from './api';
export * from './extended_template_checker';

View file

@ -7,39 +7,42 @@
*/
import * as ts from 'typescript';
import {ErrorCode} from '../../../diagnostics';
import {TemplateTypeChecker} from '../../api';
import {TemplateCheck, TemplateContext} from '../api/api';
import {ExtendedTemplateChecker, TemplateCheck, TemplateContext} from '../api';
/**
* Run all `TemplateChecks` for a component and return the generated `ts.Diagnostic`s.
* @param component the `@Component()` class from which the template is obtained
* @param templateTypeChecker interface to get information about template nodes
* @param typeChecker program's type checker
* @param templateChecks specific checks to be run
* @returns generated `ts.Diagnostic[]`
*/
export function getExtendedTemplateDiagnosticsForComponent(
component: ts.ClassDeclaration, templateTypeChecker: TemplateTypeChecker,
typeChecker: ts.TypeChecker, templateChecks: TemplateCheck<ErrorCode>[]): ts.Diagnostic[] {
const template = templateTypeChecker.getTemplate(component);
// Skip checks if component has no template. This can happen if the user writes a
// `@Component()` but doesn't add the template, could happen in the language service
// when users are in the middle of typing code.
if (template === null) {
return [];
export class ExtendedTemplateCheckerImpl implements ExtendedTemplateChecker {
constructor(
private readonly templateTypeChecker: TemplateTypeChecker,
private readonly typeChecker: ts.TypeChecker,
private readonly templateChecks: TemplateCheck<ErrorCode>[]) {}
getExtendedTemplateDiagnosticsForComponent(component: ts.ClassDeclaration): ts.Diagnostic[] {
const template = this.templateTypeChecker.getTemplate(component);
// Skip checks if component has no template. This can happen if the user writes a
// `@Component()` but doesn't add the template, could happen in the language service
// when users are in the middle of typing code.
if (template === null) {
return [];
}
const diagnostics: ts.Diagnostic[] = [];
const ctx = {
templateTypeChecker: this.templateTypeChecker,
typeChecker: this.typeChecker,
component
} as TemplateContext;
for (const check of this.templateChecks) {
diagnostics.push(...deduplicateDiagnostics(check.run(ctx, template)));
}
return diagnostics;
}
const diagnostics: ts.Diagnostic[] = [];
const ctx = {templateTypeChecker, typeChecker, component} as TemplateContext;
for (const check of templateChecks) {
diagnostics.push(...deduplicateDiagnostics(check.run(ctx, template)));
}
return diagnostics;
}
// Filter out duplicated diagnostics, this is possible due to the way the compiler
// handles desugaring and produces `AST`s. Ex.
//

View file

@ -9,7 +9,7 @@
import {TmplAstBoundEvent, TmplAstNode, TmplAstRecursiveVisitor} from '@angular/compiler';
import * as ts from 'typescript';
import {ErrorCode} from '../../../../../diagnostics';
import {TemplateCheck, TemplateContext, TemplateDiagnostic} from '../../../api/api';
import {TemplateCheck, TemplateContext, TemplateDiagnostic} from '../../../api';
/**
* Ensures the two-way binding syntax is correct.

View file

@ -12,7 +12,7 @@ import {absoluteFrom, getSourceFileOrError} from '../../../../../file_system';
import {runInEachFileSystem} from '../../../../../file_system/testing';
import {getSourceCodeForDiagnostic} from '../../../../../testing';
import {getClass, setup} from '../../../../testing';
import {getExtendedTemplateDiagnosticsForComponent} from '../../../src/template_checker';
import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker';
import {InvalidBananaInBoxCheck} from '../../../src/template_checks/invalid_banana_in_box/index';
runInEachFileSystem(() => {
@ -22,19 +22,19 @@ runInEachFileSystem(() => {
const {program, templateTypeChecker} = setup([{
fileName,
templates: {
'TestCmp': '<div ([input])="var1"> </div>',
'TestCmp': '<div ([notARealThing])="var1"> </div>',
},
source: 'export class TestCmp { var1: string = "text"; }'
}]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const diags = getExtendedTemplateDiagnosticsForComponent(
component, templateTypeChecker, program.getTypeChecker(),
[new InvalidBananaInBoxCheck()]);
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [new InvalidBananaInBoxCheck()]);
const diags = extendedTemplateChecker.getExtendedTemplateDiagnosticsForComponent(component);
expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ErrorCode.INVALID_BANANA_IN_BOX);
expect(getSourceCodeForDiagnostic(diags[0])).toBe('([input])="var1"');
expect(getSourceCodeForDiagnostic(diags[0])).toBe('([notARealThing])="var1"');
});
it('should not produce invalid banana in a box warning if written correctly', () => {
@ -42,15 +42,15 @@ runInEachFileSystem(() => {
const {program, templateTypeChecker} = setup([{
fileName,
templates: {
'TestCmp': '<div [(input)]="var1"> </div>',
'TestCmp': '<div [(notARealThing)]="var1"> </div>',
},
source: 'export class TestCmp { var1: string = "text"; }'
}]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const diags = getExtendedTemplateDiagnosticsForComponent(
component, templateTypeChecker, program.getTypeChecker(),
[new InvalidBananaInBoxCheck()]);
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [new InvalidBananaInBoxCheck()]);
const diags = extendedTemplateChecker.getExtendedTemplateDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
@ -60,15 +60,16 @@ runInEachFileSystem(() => {
const {program, templateTypeChecker} = setup([{
fileName,
templates: {
'TestCmp': '<div (abc[123]def)="var1"> </div>',
'TestCmp': '<div (not[AReal]Thing)="var1"> </div>',
},
source: 'export class TestCmp { var1: string = "text"; }'
}]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const diags = getExtendedTemplateDiagnosticsForComponent(
component, templateTypeChecker, program.getTypeChecker(),
[new InvalidBananaInBoxCheck()]);
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [new InvalidBananaInBoxCheck()]);
const diags =
extendedTemplateChecker.getExtendedTemplateDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
@ -78,8 +79,8 @@ runInEachFileSystem(() => {
fileName,
templates: {
'TestCmp': `<div>
<div *ngIf="false" ([foo])="var1"> </div>
<ng-template #elseBlock ([bar])="var1">Content to render when condition is false.</ng-template>
<div *ngIf="false" ([notARealThing])="var1"> </div>
<ng-template #elseBlock ([notARealThing2])="var1">Content to render when condition is false.</ng-template>
</div>`,
},
source: `export class TestCmp {
@ -88,16 +89,16 @@ runInEachFileSystem(() => {
}]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const diags = getExtendedTemplateDiagnosticsForComponent(
component, templateTypeChecker, program.getTypeChecker(),
[new InvalidBananaInBoxCheck()]);
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [new InvalidBananaInBoxCheck()]);
const diags = extendedTemplateChecker.getExtendedTemplateDiagnosticsForComponent(component);
expect(diags.length).toBe(2);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ErrorCode.INVALID_BANANA_IN_BOX);
expect(getSourceCodeForDiagnostic(diags[0])).toBe('([foo])="var1"');
expect(getSourceCodeForDiagnostic(diags[0])).toBe('([notARealThing])="var1"');
expect(diags[1].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[1].code).toBe(ErrorCode.INVALID_BANANA_IN_BOX);
expect(getSourceCodeForDiagnostic(diags[1])).toBe('([bar])="var1"');
expect(getSourceCodeForDiagnostic(diags[1])).toBe('([notARealThing2])="var1"');
});
});
});