Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

refactor(compiler-cli): call factories directly from extended template checker (#44391)

Browse source 
Refs #42966.

This moves extended template check factory invocations into the checker itself, where it can provide a more consistent API contract. Factories are called with compiler options and may return a `TemplateCheck` or `undefined` if the current options do not support that check. This allows `nullishCoalescingNotNullable` to disable itself when `strictNullChecks` is disabled without throwing errors. This gives extended template diagnostics a stronger abstraction model to define their behavior with.

PR Close #44391
This commit is contained in:
Doug Parker 2021-12-06 18:23:21 -08:00 committed by Andrew Scott
parent ed74e3ad61
commit 1446017e4b
10 changed files with 58 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
packages/compiler-cli/src/ngtsc/core/src/compiler.ts
View file

@ -29,7 +29,7 @@ import {aliasTransformFactory, CompilationMode, declarationTransformFactory, Dec
import {TemplateTypeCheckerImpl} from '../../typecheck';
import {OptimizeFor, TemplateTypeChecker, TypeCheckingConfig} from '../../typecheck/api';
import {ExtendedTemplateCheckerImpl} from '../../typecheck/extended';
import {ExtendedTemplateChecker, TemplateCheck} from '../../typecheck/extended/api';
import {ExtendedTemplateChecker} from '../../typecheck/extended/api';
import {factory as invalidBananaInBoxFactory} from '../../typecheck/extended/checks/invalid_banana_in_box';
import {factory as NullishCoalescingNotNullableFactory} from '../../typecheck/extended/checks/nullish_coalescing_not_nullable';
import {getSourceFileOrNull, isDtsPath, toUnredirectedSourceFile} from '../../util/src/typescript';
@ -1067,12 +1067,9 @@ export class NgCompiler {
reflector, this.adapter, this.incrementalCompilation, scopeRegistry, typeCheckScopeRegistry,
this.delegatingPerfRecorder);
const templateChecks: TemplateCheck<ErrorCode>[] = [invalidBananaInBoxFactory.create()];
if (this.options.strictNullChecks) {
templateChecks.push(NullishCoalescingNotNullableFactory.create());
}
const extendedTemplateChecker =
new ExtendedTemplateCheckerImpl(templateTypeChecker, checker, templateChecks);
const diagnosticFactories = [invalidBananaInBoxFactory, NullishCoalescingNotNullableFactory];
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, checker, diagnosticFactories, this.options);
return {
isCore,

1
packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel
View file

@ -7,6 +7,7 @@ ts_library(
),
visibility = ["//packages/compiler-cli/src/ngtsc:__subpackages__"],
deps = [
"//packages/compiler-cli/src/ngtsc/core:api",
"//packages/compiler-cli/src/ngtsc/diagnostics",
"//packages/compiler-cli/src/ngtsc/typecheck/api",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/api",

1
packages/compiler-cli/src/ngtsc/typecheck/extended/api/BUILD.bazel
View file

@ -8,6 +8,7 @@ ts_library(
visibility = ["//packages/compiler-cli/src/ngtsc:__subpackages__"],
deps = [
"//packages/compiler",
"//packages/compiler-cli/src/ngtsc/core:api",
"//packages/compiler-cli/src/ngtsc/diagnostics",
"//packages/compiler-cli/src/ngtsc/typecheck/api",
"@npm//typescript",

3
packages/compiler-cli/src/ngtsc/typecheck/extended/api/api.ts
View file

@ -9,6 +9,7 @@
import {AST, ASTWithSource, RecursiveAstVisitor, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstContent, TmplAstElement, TmplAstIcu, TmplAstNode, TmplAstRecursiveVisitor, TmplAstReference, TmplAstTemplate, TmplAstText, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
import ts from 'typescript';
import {NgCompilerOptions} from '../../../core/api';
import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../diagnostics';
import {NgTemplateDiagnostic, TemplateTypeChecker} from '../../api';
@ -49,7 +50,7 @@ export interface TemplateCheckFactory<
> {
code: Code;
name: Name;
create(): TemplateCheck<Code>;
create(options: NgCompilerOptions): TemplateCheck<Code>|null;
}
/**

1
packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable/BUILD.bazel
View file

@ -6,6 +6,7 @@ ts_library(
visibility = ["//packages/compiler-cli/src/ngtsc:__subpackages__"],
deps = [
"//packages/compiler",
"//packages/compiler-cli/src/ngtsc/core:api",
"//packages/compiler-cli/src/ngtsc/diagnostics",
"//packages/compiler-cli/src/ngtsc/typecheck/api",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/api",

10
packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable/index.ts
View file

@ -9,6 +9,7 @@
import {AST, Binary, TmplAstNode} from '@angular/compiler';
import ts from 'typescript';
import {NgCompilerOptions} from '../../../../core/api';
import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics';
import {NgTemplateDiagnostic, SymbolKind} from '../../../api';
import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api';
@ -55,5 +56,12 @@ export const factory: TemplateCheckFactory<
ExtendedTemplateDiagnosticName.NULLISH_COALESCING_NOT_NULLABLE> = {
code: ErrorCode.NULLISH_COALESCING_NOT_NULLABLE,
name: ExtendedTemplateDiagnosticName.NULLISH_COALESCING_NOT_NULLABLE,
create: () => new NullishCoalescingNotNullableCheck(),
create: (options: NgCompilerOptions) => {
// Require `strictNullChecks` to be enabled.
if (options.strictNullChecks === false) {
return null;
}
return new NullishCoalescingNotNullableCheck();
},
};

14
packages/compiler-cli/src/ngtsc/typecheck/extended/src/extended_template_checker.ts
View file

@ -8,18 +8,24 @@
import ts from 'typescript';
import {ErrorCode} from '../../../diagnostics';
import {NgCompilerOptions} from '../../../core/api';
import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../diagnostics';
import {TemplateDiagnostic, TemplateTypeChecker} from '../../api';
import {ExtendedTemplateChecker, TemplateCheck, TemplateContext} from '../api';
import {ExtendedTemplateChecker, TemplateCheck, TemplateCheckFactory, TemplateContext} from '../api';
export class ExtendedTemplateCheckerImpl implements ExtendedTemplateChecker {
private ctx: TemplateContext;
private readonly ctx: TemplateContext;
private readonly templateChecks: TemplateCheck<ErrorCode>[];
constructor(
templateTypeChecker: TemplateTypeChecker, typeChecker: ts.TypeChecker,
private readonly templateChecks: TemplateCheck<ErrorCode>[]) {
templateCheckFactories:
readonly TemplateCheckFactory<ErrorCode, ExtendedTemplateDiagnosticName>[],
options: NgCompilerOptions) {
this.ctx = {templateTypeChecker: templateTypeChecker, typeChecker: typeChecker} as
TemplateContext;
this.templateChecks = templateCheckFactories.map((factory) => factory.create(options))
.filter((check): check is TemplateCheck<ErrorCode> => check !== null);
}
getDiagnosticsForComponent(component: ts.ClassDeclaration): TemplateDiagnostic[] {

12
packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/invalid_banana_in_box/invalid_banana_in_box_spec.ts
View file

@ -36,7 +36,8 @@ runInEachFileSystem(() => {
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [invalidBananaInBoxFactory.create()]);
templateTypeChecker, program.getTypeChecker(), [invalidBananaInBoxFactory],
{} /* options */);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
@ -56,7 +57,8 @@ runInEachFileSystem(() => {
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [invalidBananaInBoxFactory.create()]);
templateTypeChecker, program.getTypeChecker(), [invalidBananaInBoxFactory],
{} /* options */);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
@ -74,7 +76,8 @@ runInEachFileSystem(() => {
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [invalidBananaInBoxFactory.create()]);
templateTypeChecker, program.getTypeChecker(), [invalidBananaInBoxFactory],
{} /* options */);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
@ -96,7 +99,8 @@ runInEachFileSystem(() => {
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [invalidBananaInBoxFactory.create()]);
templateTypeChecker, program.getTypeChecker(), [invalidBananaInBoxFactory],
{} /* options */);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(2);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);

33
packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/nullish_coalescing_not_nullable/nullish_coalescing_not_nullable_spec.ts
View file

@ -25,6 +25,15 @@ runInEachFileSystem(() => {
.toBe(ExtendedTemplateDiagnosticName.NULLISH_COALESCING_NOT_NULLABLE);
});
it('should return a check if `strictNullChecks` is enabled', () => {
expect(nullishCoalescingNotNullableFactory.create({strictNullChecks: true})).toBeDefined();
expect(nullishCoalescingNotNullableFactory.create({})).not.toBeNull(); // Defaults enabled.
});
it('should not return a check if `strictNullChecks` is disabled', () => {
expect(nullishCoalescingNotNullableFactory.create({strictNullChecks: false})).toBeNull();
});
it('should produce nullish coalescing warning', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([{
@ -37,8 +46,8 @@ runInEachFileSystem(() => {
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(),
[nullishCoalescingNotNullableFactory.create()]);
templateTypeChecker, program.getTypeChecker(), [nullishCoalescingNotNullableFactory],
{} /* options */);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
@ -58,8 +67,8 @@ runInEachFileSystem(() => {
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(),
[nullishCoalescingNotNullableFactory.create()]);
templateTypeChecker, program.getTypeChecker(), [nullishCoalescingNotNullableFactory],
{} /* options */);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
@ -76,8 +85,8 @@ runInEachFileSystem(() => {
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(),
[nullishCoalescingNotNullableFactory.create()]);
templateTypeChecker, program.getTypeChecker(), [nullishCoalescingNotNullableFactory],
{} /* options */);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
@ -105,8 +114,8 @@ runInEachFileSystem(() => {
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(),
[nullishCoalescingNotNullableFactory.create()]);
templateTypeChecker, program.getTypeChecker(), [nullishCoalescingNotNullableFactory],
{} /* options */);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
@ -138,8 +147,8 @@ runInEachFileSystem(() => {
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(),
[nullishCoalescingNotNullableFactory.create()]);
templateTypeChecker, program.getTypeChecker(), [nullishCoalescingNotNullableFactory],
{} /* options */);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
@ -163,8 +172,8 @@ runInEachFileSystem(() => {
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(),
[nullishCoalescingNotNullableFactory.create()]);
templateTypeChecker, program.getTypeChecker(), [nullishCoalescingNotNullableFactory],
{} /* options */);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});

20
packages/compiler-cli/test/ngtsc/extended_template_diagnostics_spec.ts
View file

@ -7,6 +7,7 @@
*/
import ts from 'typescript';
import {ErrorCode, ngErrorCode} from '../../src/ngtsc/diagnostics';
import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
import {getSourceCodeForDiagnostic, loadStandardTestFiles} from '../../src/ngtsc/testing';
@ -122,24 +123,5 @@ runInEachFileSystem(() => {
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.NULLISH_COALESCING_NOT_NULLABLE));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('bar ?? "foo"');
});
it(`should not produce nullish coalescing not nullable warning with strictNullChecks disabled`,
() => {
env.tsconfig(
{_extendedTemplateDiagnostics: true, strictTemplates: true, strictNullChecks: false});
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'test',
template: '{{ bar ?? "foo" }}',
})
export class TestCmp {
bar: string = undefined;
}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
});
});