fix(compiler-cli): interpolatedSignalNotInvoked diagnostic for class, style, attribute and animation bindings (#55969)

cexbrayat · pkozlowski-opensource · commit 4c7efc005a8d · 2024-05-23T15:05:02.000+02:00
This extends the diagnostic to catch issues like: ```html <div [class.green]="mySignal"></div> <div [style.width]="mySignal"></div> <div [attr.role]="mySignal"></div> <div [@TriggerName]="mySignal"></div> ``` PR Close #55969
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/interpolated_signal_not_invoked/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/interpolated_signal_not_invoked/index.ts
@@ -55,9 +55,18 @@ class InterpolatedSignalCheck extends TemplateCheckWithVisitor<ErrorCode.INTERPO
       if (symbol !== null && symbol.kind === SymbolKind.Input) {
         return [];
       }
-      // otherwise, we check if the node is a bound property like `[prop]="mySignal"`
+      // otherwise, we check if the node is
       if (
-        node.type === BindingType.Property &&
+        // a bound property like `[prop]="mySignal"`
+        (node.type === BindingType.Property ||
+          // or a class binding like `[class.myClass]="mySignal"`
+          node.type === BindingType.Class ||
+          // or a style binding like `[style.width]="mySignal"`
+          node.type === BindingType.Style ||
+          // or an attribute binding like `[attr.role]="mySignal"`
+          node.type === BindingType.Attribute ||
+          // or an animation binding like `[@myAnimation]="mySignal"`
+          node.type === BindingType.Animation) &&
         node.value instanceof ASTWithSource &&
         node.value.ast instanceof PropertyRead
       ) {
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/interpolated_signal_not_invoked/interpolated_signal_not_invoked_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/interpolated_signal_not_invoked/interpolated_signal_not_invoked_spec.ts
@@ -625,63 +625,71 @@ runInEachFileSystem(() => {
     expect(getSourceCodeForDiagnostic(diags[0])).toBe(`myNestedSignal`);
   });
 
-  it("should produce a warning when signal isn't invoked on dom property binding", () => {
-    const fileName = absoluteFrom('/main.ts');
-    const {program, templateTypeChecker} = setup([
-      {
-        fileName,
-        templates: {
-          'TestCmp': `<div [id]="mySignal"></div>`,
-        },
-        source: `
+  [
+    ['dom property', 'id'],
+    ['class', 'class.green'],
+    ['style', 'style.width'],
+    ['attribute', 'attr.role'],
+    ['animation', '@triggerName'],
+  ].forEach(([name, binding]) => {
+    it(`should produce a warning when signal isn't invoked on ${name} binding`, () => {
+      const fileName = absoluteFrom('/main.ts');
+      const {program, templateTypeChecker} = setup([
+        {
+          fileName,
+          templates: {
+            'TestCmp': `<div [${binding}]="mySignal"></div>`,
+          },
+          source: `
           import {signal} from '@angular/core';
 
           export class TestCmp {
             mySignal = signal<number>(0);
           }`,
-      },
-    ]);
-    const sf = getSourceFileOrError(program, fileName);
-    const component = getClass(sf, 'TestCmp');
-    const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
-      templateTypeChecker,
-      program.getTypeChecker(),
-      [interpolatedSignalFactory],
-      {} /* options */,
-    );
-    const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
-    expect(diags.length).toBe(1);
-    expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
-    expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED));
-    expect(getSourceCodeForDiagnostic(diags[0])).toBe(`mySignal`);
-  });
-
-  it('should not produce a warning when signal is invoked on dom property binding', () => {
-    const fileName = absoluteFrom('/main.ts');
-    const {program, templateTypeChecker} = setup([
-      {
-        fileName,
-        templates: {
-          'TestCmp': `<div [id]="mySignal()"></div>`,
         },
-        source: `
+      ]);
+      const sf = getSourceFileOrError(program, fileName);
+      const component = getClass(sf, 'TestCmp');
+      const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
+        templateTypeChecker,
+        program.getTypeChecker(),
+        [interpolatedSignalFactory],
+        {} /* options */,
+      );
+      const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
+      expect(diags.length).toBe(1);
+      expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
+      expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED));
+      expect(getSourceCodeForDiagnostic(diags[0])).toBe(`mySignal`);
+    });
+
+    it(`should not produce a warning when signal is invoked on ${name} binding`, () => {
+      const fileName = absoluteFrom('/main.ts');
+      const {program, templateTypeChecker} = setup([
+        {
+          fileName,
+          templates: {
+            'TestCmp': `<div [${binding}]="mySignal()"></div>`,
+          },
+          source: `
           import {signal} from '@angular/core';
 
           export class TestCmp {
             mySignal = signal<number>(0);
           }`,
-      },
-    ]);
-    const sf = getSourceFileOrError(program, fileName);
-    const component = getClass(sf, 'TestCmp');
-    const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
-      templateTypeChecker,
-      program.getTypeChecker(),
-      [interpolatedSignalFactory],
-      {} /* options */,
-    );
-    const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
-    expect(diags.length).toBe(0);
+        },
+      ]);
+      const sf = getSourceFileOrError(program, fileName);
+      const component = getClass(sf, 'TestCmp');
+      const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
+        templateTypeChecker,
+        program.getTypeChecker(),
+        [interpolatedSignalFactory],
+        {} /* options */,
+      );
+      const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
+      expect(diags.length).toBe(0);
+    });
   });
 
   it('should not produce a warning with other Signal type', () => {
