From cf88ea0bf31cefaa7673c48cecf0501585e0739f Mon Sep 17 00:00:00 2001
From: Alex Rickabaugh <alx+alxhub@alxandria.net>
Date: Fri, 30 Oct 2020 14:37:02 -0700
Subject: [PATCH] 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
---
 .../src/ngtsc/typecheck/src/oob.ts            | 11 ++++++++
 .../ngtsc/typecheck/test/diagnostics_spec.ts  | 26 +++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts
index 2b7cbd02e5..75170a918a 100644
--- a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts
+++ b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts
@@ -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(
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts
index 0104b024a2..6662032296 100644
--- a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts
+++ b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts
@@ -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 {