fix(compiler-cli): avoid duplicate diagnostics about unknown pipes (#39517)

TCB generation occasionally transforms binding expressions twice, which can
result in a `BindingPipe` operation being `resolve()`'d multiple times. When
the pipe does not exist, this caused multiple OOB diagnostics to be recorded
about the missing pipe.

This commit fixes the problem by making the OOB recorder track which pipe
expressions have had diagnostics produced already, and only producing them
once per expression.

PR Close #39517
This commit is contained in:
Alex Rickabaugh 2020-10-30 14:37:02 -07:00 committed by Misko Hevery
parent 0929099e41
commit cf88ea0bf3
2 changed files with 37 additions and 0 deletions

View File

@ -73,6 +73,12 @@ export interface OutOfBandDiagnosticRecorder {
export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder {
private _diagnostics: TemplateDiagnostic[] = [];
/**
* Tracks which `BindingPipe` nodes have already been recorded as invalid, so only one diagnostic
* is ever produced per node.
*/
private recordedPipes = new Set<BindingPipe>();
constructor(private resolver: TemplateSourceResolver) {}
get diagnostics(): ReadonlyArray<TemplateDiagnostic> {
@ -90,6 +96,10 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
}
missingPipe(templateId: TemplateId, ast: BindingPipe): void {
if (this.recordedPipes.has(ast)) {
return;
}
const mapping = this.resolver.getSourceMapping(templateId);
const errorMsg = `No pipe found with name '${ast.name}'.`;
@ -101,6 +111,7 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
this._diagnostics.push(makeTemplateDiagnostic(
templateId, mapping, sourceSpan, ts.DiagnosticCategory.Error,
ngErrorCode(ErrorCode.MISSING_PIPE), errorMsg));
this.recordedPipes.add(ast);
}
illegalAssignmentToTemplateVar(

View File

@ -210,6 +210,32 @@ runInEachFileSystem(() => {
]);
});
it('does not repeat diagnostics for missing pipes in directive inputs', () => {
// The directive here is structured so that a type constructor is used, which resuts in each
// input binding being processed twice. This results in the 'uppercase' pipe being resolved
// twice, and since it doesn't exist this operation will fail. The test is here to verify that
// failing to resolve the pipe twice only produces a single diagnostic (no duplicates).
const messages = diagnose(
'<div *dir="let x of name | uppercase"></div>', `
class Dir<T> {
dirOf: T;
}
class TestComponent {
name: string;
}`,
[{
type: 'directive',
name: 'Dir',
selector: '[dir]',
inputs: {'dirOf': 'dirOf'},
isGeneric: true,
}]);
expect(messages.length).toBe(1);
expect(messages[0]).toContain(`No pipe found with name 'uppercase'.`);
});
it('does not repeat diagnostics for errors within LHS of safe-navigation operator', () => {
const messages = diagnose(`{{ personn?.name }} {{ personn?.getName() }}`, `
class TestComponent {