From 58a19a30217dc500bab47436710f726025603e27 Mon Sep 17 00:00:00 2001 From: Daniel Trevino <23410540+danieltre23@users.noreply.github.com> Date: Mon, 2 Aug 2021 19:06:42 +0000 Subject: [PATCH] refactor(compiler-cli): add `extendedTemplateCheck` phase to compiler (#43107) This commit integrates extended template checks with the compiler, by adding another phase of diagnostics generation. This integration is under the `_extendedTemplateDiagnostics` flag. Refs #42966 PR Close #43107 --- .../src/ngtsc/annotations/BUILD.bazel | 1 + .../src/ngtsc/annotations/src/component.ts | 7 +++ .../compiler-cli/src/ngtsc/core/BUILD.bazel | 2 + .../src/ngtsc/core/api/src/options.ts | 12 ++++- .../src/ngtsc/core/src/compiler.ts | 52 ++++++++++++++++--- .../src/ngtsc/transform/BUILD.bazel | 1 + .../src/ngtsc/transform/src/api.ts | 5 ++ .../src/ngtsc/transform/src/compilation.ts | 26 +++++++++- .../invalid_banana_in_box/BUILD.bazel | 2 +- 9 files changed, 99 insertions(+), 9 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/BUILD.bazel b/packages/compiler-cli/src/ngtsc/annotations/BUILD.bazel index fed9544064239..4361072e96420 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/annotations/BUILD.bazel @@ -26,6 +26,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/transform", "//packages/compiler-cli/src/ngtsc/typecheck/api", "//packages/compiler-cli/src/ngtsc/typecheck/diagnostics", + "//packages/compiler-cli/src/ngtsc/typecheck/extended/api", "//packages/compiler-cli/src/ngtsc/util", "//packages/compiler-cli/src/ngtsc/xi18n", "@npm//@types/node", diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 281c99b1178bb..835177d72641a 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -24,6 +24,7 @@ import {ClassDeclaration, DeclarationNode, Decorator, ReflectionHost, reflectObj import {ComponentScopeReader, LocalModuleScopeRegistry, TypeCheckScopeRegistry} from '../../scope'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerFlags, HandlerPrecedence, ResolveResult} from '../../transform'; import {TemplateSourceMapping, TypeCheckContext} from '../../typecheck/api'; +import {ExtendedTemplateChecker} from '../../typecheck/extended/api'; import {SubsetOfKeys} from '../../util/src/typescript'; import {Xi18nContext} from '../../xi18n'; @@ -612,6 +613,12 @@ export class ComponentDecoratorHandler implements meta.template.sourceMapping, meta.template.file, meta.template.errors); } + extendedTemplateCheck( + component: ts.ClassDeclaration, + extendedTemplateChecker: ExtendedTemplateChecker): ts.Diagnostic[] { + return extendedTemplateChecker.getExtendedTemplateDiagnosticsForComponent(component); + } + resolve( node: ClassDeclaration, analysis: Readonly, symbol: ComponentSymbol): ResolveResult { diff --git a/packages/compiler-cli/src/ngtsc/core/BUILD.bazel b/packages/compiler-cli/src/ngtsc/core/BUILD.bazel index 04122d3613997..371248d1d6a51 100644 --- a/packages/compiler-cli/src/ngtsc/core/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/core/BUILD.bazel @@ -37,6 +37,8 @@ ts_library( "//packages/compiler-cli/src/ngtsc/typecheck", "//packages/compiler-cli/src/ngtsc/typecheck/api", "//packages/compiler-cli/src/ngtsc/typecheck/diagnostics", + "//packages/compiler-cli/src/ngtsc/typecheck/extended", + "//packages/compiler-cli/src/ngtsc/typecheck/extended/src/template_checks/invalid_banana_in_box", "//packages/compiler-cli/src/ngtsc/util", "//packages/compiler-cli/src/ngtsc/xi18n", "@npm//typescript", diff --git a/packages/compiler-cli/src/ngtsc/core/api/src/options.ts b/packages/compiler-cli/src/ngtsc/core/api/src/options.ts index b9117d54599df..037de2de6ea5c 100644 --- a/packages/compiler-cli/src/ngtsc/core/api/src/options.ts +++ b/packages/compiler-cli/src/ngtsc/core/api/src/options.ts @@ -36,6 +36,16 @@ export interface TestOnlyOptions { tracePerformance?: string; } +/** + * Internal only options for compiler. + */ +export interface InternalOptions { + /** + * Whether to run all template checks and generate extended template diagnostics. + */ + _extendedTemplateDiagnostics?: boolean; +} + /** * A merged interface of all of the various Angular compiler options, as well as the standard * `ts.CompilerOptions`. @@ -45,4 +55,4 @@ export interface TestOnlyOptions { export interface NgCompilerOptions extends ts.CompilerOptions, LegacyNgcOptions, BazelAndG3Options, NgcCompatibilityOptions, StrictTemplateOptions, TestOnlyOptions, I18nOptions, TargetOptions, - MiscOptions {} + InternalOptions, MiscOptions {} diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index b3c0ac939c4a9..35b9fd7f38bab 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -30,6 +30,8 @@ import {ivySwitchTransform} from '../../switch'; import {aliasTransformFactory, CompilationMode, declarationTransformFactory, DecoratorHandler, DtsTransformRegistry, ivyTransformFactory, TraitCompiler} from '../../transform'; import {TemplateTypeCheckerImpl} from '../../typecheck'; import {OptimizeFor, TemplateTypeChecker, TypeCheckingConfig} from '../../typecheck/api'; +import {ExtendedTemplateCheckerImpl} from '../../typecheck/extended'; +import {InvalidBananaInBoxCheck} from '../../typecheck/extended/src/template_checks/invalid_banana_in_box'; import {getSourceFileOrNull, isDtsPath, resolveModuleName, toUnredirectedSourceFile} from '../../util/src/typescript'; import {Xi18nContext} from '../../xi18n'; import {LazyRoute, NgCompilerAdapter, NgCompilerOptions} from '../api'; @@ -316,6 +318,12 @@ export class NgCompiler { readonly usePoisonedData: boolean, private livePerfRecorder: ActivePerfRecorder, ) { + if (this.options._extendedTemplateDiagnostics === true && + this.options.strictTemplates === false) { + throw new Error( + 'The \'_extendedTemplateDiagnostics\' option requires \'strictTemplates\' to also be enabled.'); + } + this.constructionDiagnostics.push(...this.adapter.constructionDiagnostics); const incompatibleTypeCheckOptionsDiagnostic = verifyCompatibleTypeCheckOptions(this.options); if (incompatibleTypeCheckOptionsDiagnostic !== null) { @@ -425,8 +433,12 @@ export class NgCompiler { * Get all Angular-related diagnostics for this compilation. */ getDiagnostics(): ts.Diagnostic[] { - return this.addMessageTextDetails( - [...this.getNonTemplateDiagnostics(), ...this.getTemplateDiagnostics()]); + const diagnostics: ts.Diagnostic[] = []; + diagnostics.push(...this.getNonTemplateDiagnostics(), ...this.getTemplateDiagnostics()); + if (this.options._extendedTemplateDiagnostics) { + diagnostics.push(...this.getExtendedTemplateDiagnostics()); + } + return this.addMessageTextDetails(diagnostics); } /** @@ -435,10 +447,14 @@ export class NgCompiler { * If a `ts.SourceFile` is passed, only diagnostics related to that file are returned. */ getDiagnosticsForFile(file: ts.SourceFile, optimizeFor: OptimizeFor): ts.Diagnostic[] { - return this.addMessageTextDetails([ - ...this.getNonTemplateDiagnostics().filter(diag => diag.file === file), - ...this.getTemplateDiagnosticsForFile(file, optimizeFor) - ]); + const diagnostics: ts.Diagnostic[] = []; + diagnostics.push( + ...this.getNonTemplateDiagnostics().filter(diag => diag.file === file), + ...this.getTemplateDiagnosticsForFile(file, optimizeFor)); + if (this.options._extendedTemplateDiagnostics) { + diagnostics.push(...this.getExtendedTemplateDiagnostics(file)); + } + return this.addMessageTextDetails(diagnostics); } /** @@ -892,6 +908,30 @@ export class NgCompiler { return this.nonTemplateDiagnostics; } + /** + * Calls the `extendedTemplateCheck` phase of the trait compiler + * @param sf optional parameter to get diagnostics for a certain file + * or all files in the program if `sf` is undefined + * @returns generated extended template diagnostics + */ + private getExtendedTemplateDiagnostics(sf?: ts.SourceFile): ts.Diagnostic[] { + const diagnostics: ts.Diagnostic[] = []; + const compilation = this.ensureAnalyzed(); + const typeChecker = this.inputProgram.getTypeChecker(); + const templateChecks = [new InvalidBananaInBoxCheck()]; + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + compilation.templateTypeChecker, typeChecker, templateChecks); + if (sf !== undefined) { + return compilation.traitCompiler.extendedTemplateCheck(sf, extendedTemplateChecker); + } + for (const sf of this.inputProgram.getSourceFiles()) { + diagnostics.push( + ...compilation.traitCompiler.extendedTemplateCheck(sf, extendedTemplateChecker)); + } + + return diagnostics; + } + private makeCompilation(): LazyCompilationState { const checker = this.inputProgram.getTypeChecker(); diff --git a/packages/compiler-cli/src/ngtsc/transform/BUILD.bazel b/packages/compiler-cli/src/ngtsc/transform/BUILD.bazel index 86d831269dc31..62ace8b72ce40 100644 --- a/packages/compiler-cli/src/ngtsc/transform/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/transform/BUILD.bazel @@ -19,6 +19,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/reflection", "//packages/compiler-cli/src/ngtsc/translator", "//packages/compiler-cli/src/ngtsc/typecheck/api", + "//packages/compiler-cli/src/ngtsc/typecheck/extended/api", "//packages/compiler-cli/src/ngtsc/util", "//packages/compiler-cli/src/ngtsc/xi18n", "@npm//typescript", diff --git a/packages/compiler-cli/src/ngtsc/transform/src/api.ts b/packages/compiler-cli/src/ngtsc/transform/src/api.ts index 4387a651c54d1..5bffd37a086c7 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/api.ts @@ -15,6 +15,7 @@ import {IndexingContext} from '../../indexer'; import {ClassDeclaration, Decorator} from '../../reflection'; import {ImportManager} from '../../translator'; import {TypeCheckContext} from '../../typecheck/api'; +import {ExtendedTemplateChecker} from '../../typecheck/extended/api'; import {Xi18nContext} from '../../xi18n'; /** @@ -187,6 +188,10 @@ export interface DecoratorHandler { (ctx: TypeCheckContext, node: ClassDeclaration, analysis: Readonly, resolution: Readonly): void; + extendedTemplateCheck? + (component: ts.ClassDeclaration, extendedTemplateChecker: ExtendedTemplateChecker): + ts.Diagnostic[]; + /** * Generate a description of the field which should be added to the class, including any * initialization code to be generated. diff --git a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts index 87186fbb2bb25..4f1d708275b76 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts @@ -14,8 +14,9 @@ import {IncrementalBuild} from '../../incremental/api'; import {SemanticDepGraphUpdater, SemanticSymbol} from '../../incremental/semantic_graph'; import {IndexingContext} from '../../indexer'; import {PerfEvent, PerfRecorder} from '../../perf'; -import {ClassDeclaration, DeclarationNode, Decorator, ReflectionHost} from '../../reflection'; +import {ClassDeclaration, DeclarationNode, Decorator, isNamedClassDeclaration, ReflectionHost} from '../../reflection'; import {ProgramTypeCheckAdapter, TypeCheckContext} from '../../typecheck/api'; +import {ExtendedTemplateChecker} from '../../typecheck/extended/api'; import {getSourceFile, isExported} from '../../util/src/typescript'; import {Xi18nContext} from '../../xi18n'; @@ -490,6 +491,29 @@ export class TraitCompiler implements ProgramTypeCheckAdapter { } } + extendedTemplateCheck(sf: ts.SourceFile, extendedTemplateChecker: ExtendedTemplateChecker): + ts.Diagnostic[] { + const classes = this.fileToClasses.get(sf); + if (classes === undefined) { + return []; + } + + const diagnostics: ts.Diagnostic[] = []; + for (const clazz of classes) { + if (!isNamedClassDeclaration(clazz)) { + continue; + } + const record = this.classes.get(clazz)!; + for (const trait of record.traits) { + if (trait.handler.extendedTemplateCheck === undefined) { + continue; + } + diagnostics.push(...trait.handler.extendedTemplateCheck(clazz, extendedTemplateChecker)); + } + } + return diagnostics; + } + index(ctx: IndexingContext): void { for (const clazz of this.classes.keys()) { const record = this.classes.get(clazz)!; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/src/template_checks/invalid_banana_in_box/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/extended/src/template_checks/invalid_banana_in_box/BUILD.bazel index f2a1359185090..ff4c64af4448f 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/src/template_checks/invalid_banana_in_box/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/src/template_checks/invalid_banana_in_box/BUILD.bazel @@ -3,7 +3,7 @@ load("//tools:defaults.bzl", "ts_library") ts_library( name = "invalid_banana_in_box", srcs = ["index.ts"], - visibility = ["//packages/compiler-cli/src/ngtsc/typecheck/extended:__subpackages__"], + visibility = ["//packages/compiler-cli/src/ngtsc:__subpackages__"], deps = [ "//packages/compiler", "//packages/compiler-cli/src/ngtsc/diagnostics",