From 8e237a016134bfbfd4f8a312531ff1376f4f4a36 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 7 Feb 2024 12:39:00 +0000 Subject: [PATCH] fix(compiler-cli): properly catch fatal diagnostics in type checking (#54309) An identical addition to: 760b1f3d0b857288980f2d9929147f331d657f7d. This commit expands the `try/catch`-es: - to properly NOT throw and just convert the diagnostic. - to be in place for all top-level instances. Notably, this logic cannot reside in the template type checker directly as otherwise we would risk multiple duplicate diagnostics. PR Close #54309 --- .../src/ngtsc/core/src/compiler.ts | 69 ++++++++++++------- .../test/ngtsc/template_typecheck_spec.ts | 28 ++++++++ 2 files changed, 71 insertions(+), 26 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index af569bbc9d726..586c178bef0e2 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -427,11 +427,28 @@ export class NgCompiler { * Get all Angular-related diagnostics for this compilation. */ getDiagnostics(): ts.Diagnostic[] { - const diagnostics: ts.Diagnostic[] = []; - diagnostics.push(...this.getNonTemplateDiagnostics(), ...this.getTemplateDiagnostics()); - if (this.options.strictTemplates) { - diagnostics.push(...this.getExtendedTemplateDiagnostics()); + const diagnostics: ts.Diagnostic[] = [ + ...this.getNonTemplateDiagnostics(), + ]; + + // Type check code may throw fatal diagnostic errors if e.g. the type check + // block cannot be generated. Gracefully return the associated diagnostic. + // Note: If a fatal diagnostic is raised, do not repeat the same diagnostics + // by running the extended template checking code, which will attempt to + // generate the same TCB. + try { + diagnostics.push(...this.getTemplateDiagnostics()); + + if (this.options.strictTemplates) { + diagnostics.push(...this.getExtendedTemplateDiagnostics()); + } + } catch (err: unknown) { + if (!(err instanceof FatalDiagnosticError)) { + throw err; + } + diagnostics.push(err.toDiagnostic()); } + return this.addMessageTextDetails(diagnostics); } @@ -444,21 +461,22 @@ export class NgCompiler { const diagnostics: ts.Diagnostic[] = [...this.getNonTemplateDiagnostics().filter(diag => diag.file === file)]; + // Type check code may throw fatal diagnostic errors if e.g. the type check + // block cannot be generated. Gracefully return the associated diagnostic. + // Note: If a fatal diagnostic is raised, do not repeat the same diagnostics + // by running the extended template checking code, which will attempt to + // generate the same TCB. try { diagnostics.push(...this.getTemplateDiagnosticsForFile(file, optimizeFor)); + if (this.options.strictTemplates) { diagnostics.push(...this.getExtendedTemplateDiagnostics(file)); } - } catch (e) { - // Type check code may throw fatal diagnostic errors if e.g. the type check - // block cannot be generated. Gracefully return the associated diagnostic. - // Note: If a fatal diagnostic is raised, do not repeat the same diagnostics - // by running the extended template checking code, which will attempt to - // generate the same TCB. - if (e instanceof FatalDiagnosticError) { - diagnostics.push(e.toDiagnostic()); + } catch (err: unknown) { + if (!(err instanceof FatalDiagnosticError)) { + throw err; } - throw e; + diagnostics.push(err.toDiagnostic()); } return this.addMessageTextDetails(diagnostics); @@ -471,6 +489,12 @@ export class NgCompiler { const compilation = this.ensureAnalyzed(); const ttc = compilation.templateTypeChecker; const diagnostics: ts.Diagnostic[] = []; + + // Type check code may throw fatal diagnostic errors if e.g. the type check + // block cannot be generated. Gracefully return the associated diagnostic. + // Note: If a fatal diagnostic is raised, do not repeat the same diagnostics + // by running the extended template checking code, which will attempt to + // generate the same TCB. try { diagnostics.push(...ttc.getDiagnosticsForComponent(component)); @@ -884,23 +908,16 @@ export class NgCompiler { private getTemplateDiagnostics(): ReadonlyArray { const compilation = this.ensureAnalyzed(); - - // Get the diagnostics. const diagnostics: ts.Diagnostic[] = []; + + // Get diagnostics for all files. for (const sf of this.inputProgram.getSourceFiles()) { if (sf.isDeclarationFile || this.adapter.isShim(sf)) { continue; } - try { - diagnostics.push( - ...compilation.templateTypeChecker.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram)); - } catch (err) { - if (!(err instanceof FatalDiagnosticError)) { - throw err; - } - diagnostics.push(err.toDiagnostic()); - } + diagnostics.push( + ...compilation.templateTypeChecker.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram)); } const program = this.programDriver.getProgram(); @@ -1028,8 +1045,8 @@ export class NgCompiler { ]); // If an entrypoint is present, then all user imports should be directed through the - // entrypoint and private exports are not needed. The compiler will validate that all publicly - // visible directives/pipes are importable via this entrypoint. + // entrypoint and private exports are not needed. The compiler will validate that all + // publicly visible directives/pipes are importable via this entrypoint. if (this.entryPoint === null && this.options.generateDeepReexports === true) { // No entrypoint is present and deep re-exports were requested, so configure the aliasing // system to generate them. diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 65ba0a3dbbfc8..c5d425ad2ccc2 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -69,6 +69,34 @@ export declare class AnimationEvent { expect(getSourceCodeForDiagnostic(diags[0])).toBe('does_not_exist'); }); + it('should not fail with a runtime error when generating TCB', () => { + env.tsconfig({strictTemplates: true}); + env.write('test.ts', ` + import {Component, input} from '@angular/core'; + + @Component({ + selector: 'sub-cmp', + standalone: true, + template: '', + }) + class Sub { // intentionally not exported + someInput = input.required(); + } + + @Component({ + template: \`\`, + standalone: true, + imports: [Sub], + }) + export class MyComponent {} + `); + + const diagnostics = env.driveDiagnostics(); + expect(diagnostics).toEqual([jasmine.objectContaining( + {messageText: jasmine.objectContaining({messageText: 'Unable to import symbol Sub.'})}, + )]); + }); + it('should check regular attributes that are directive inputs', () => { env.tsconfig( {fullTemplateTypeCheck: true, strictInputTypes: true, strictAttributeTypes: true});