From 2f8da46a0f2d28a3a7c3652a0b5254e085992b46 Mon Sep 17 00:00:00 2001
From: Andrew Scott <atscott@google.com>
Date: Tue, 28 Jun 2022 12:58:09 -0700
Subject: [PATCH] fixup! feat(compiler): Add extended diagnostic to warn when
 text attributes are intended to be bindings

---
 .../text_attribute_not_binding/index.ts       | 19 +++++++++----------
 .../text_attribute_not_binding_spec.ts        |  4 +++-
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding/index.ts
index fafd15e746528..5b3c2db077119 100644
--- a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding/index.ts
+++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding/index.ts
@@ -32,28 +32,27 @@ class TextAttributeNotBindingSpec extends
     if (!(node instanceof TmplAstTextAttribute)) return [];
 
     const name = node.name;
-    if ((!name.startsWith('attr.') && !name.startsWith('style.') && !name.startsWith('class.')) ||
-        !node.value) {
+    if ((!name.startsWith('attr.') && !name.startsWith('style.') && !name.startsWith('class.'))) {
       return [];
     }
 
-    const boundSyntax = node.sourceSpan.toString();
     let errorString: string;
     if (name.startsWith('attr.')) {
       const staticAttr = name.replace('attr.', '');
-      errorString = `Static attributes should be written without the 'attr.' prefix: ${
-          staticAttr}="${node.value}".`;
-
+      errorString = `Static attributes should be written without the 'attr.' prefix.`;
+      if (node.value) {
+        errorString += ` For example, ${staticAttr}="${node.value}".`;
+      }
     } else {
       const expectedKey = `[${name}]`;
       const expectedValue =
           // true/false are special cases because we don't want to convert them to strings but
           // rather maintain the logical true/false when bound.
           (node.value === 'true' || node.value === 'false') ? node.value : `'${node.value}'`;
-      errorString = boundSyntax.replace(
-          name,
-          `Attribute, style, and class bindings should be enclosed with square braces: '${
-              expectedKey}="${expectedValue}"'.`);
+      errorString = 'Attribute, style, and class bindings should be enclosed with square braces.';
+      if (node.value) {
+        errorString += ` For example, '${expectedKey}="${expectedValue}"'.`;
+      }
     }
     const diagnostic = ctx.makeTemplateDiagnostic(node.sourceSpan, errorString);
     return [diagnostic];
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/text_attribute_not_binding/text_attribute_not_binding_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/text_attribute_not_binding/text_attribute_not_binding_spec.ts
index b7f81ca27a61b..b940a36df5c00 100644
--- a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/text_attribute_not_binding/text_attribute_not_binding_spec.ts
+++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/text_attribute_not_binding/text_attribute_not_binding_spec.ts
@@ -103,7 +103,9 @@ runInEachFileSystem(() => {
           templateTypeChecker, program.getTypeChecker(), [textAttributeNotBindingFactory],
           {} /* options */);
       const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
-      expect(diags.length).toBe(0);
+      expect(diags.length).toBe(1);
+      expect(diags[0].code).toBe(ngErrorCode(ErrorCode.TEXT_ATTRIBUTE_NOT_BINDING));
+      expect(getSourceCodeForDiagnostic(diags[0])).toBe(`attr.readonly`);
     });
   });
 });