Skip to content

Commit

Permalink
fix(compiler-cli): properly catch fatal diagnostics in type checking (#…
Browse files Browse the repository at this point in the history
…54309)

An identical addition to: 760b1f3.

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
  • Loading branch information
devversion authored and thePunderWoman committed Feb 7, 2024
1 parent 4482cbd commit 40e1edc
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 26 deletions.
69 changes: 43 additions & 26 deletions packages/compiler-cli/src/ngtsc/core/src/compiler.ts
Expand Up @@ -420,11 +420,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);
}

Expand All @@ -437,21 +454,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);
Expand All @@ -464,6 +482,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));

Expand Down Expand Up @@ -876,23 +900,16 @@ export class NgCompiler {

private getTemplateDiagnostics(): ReadonlyArray<ts.Diagnostic> {
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();
Expand Down Expand Up @@ -1018,8 +1035,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.
Expand Down
28 changes: 28 additions & 0 deletions packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
Expand Up @@ -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<string>();
}
@Component({
template: \`<sub-cmp [someInput]="''" />\`,
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});
Expand Down

0 comments on commit 40e1edc

Please sign in to comment.