fix(compiler-cli): interpolatedSignalNotInvoked diagnostic (#53585)

cexbrayat · atscott · commit 33b5707ee9e4 · 2024-01-03T10:19:54.000-08:00
The diagnostic was catching the following case: ```ts name = signal('Angular'); ``` but not the following ones: ```ts name = signal('Angular').asReadonly(); name = computed(() => 'Angular'); name!: Signal<string> ``` This was not catched in the tests because the type of `Signal` is different than the one actually used in core. It turns out the real type forces the diagnostic to check both the `symbol.tsType.symbol` and the `symbol.tsType.aliasSymbol`. PR Close #53585
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
@@ -37,16 +37,19 @@ class InterpolatedSignalCheck extends
   }
 }
 
+function isSignal(symbol: ts.Symbol|undefined): boolean {
+  return (symbol?.escapedName === 'WritableSignal' || symbol?.escapedName === 'Signal') &&
+      (symbol as any).parent.escapedName.includes('@angular/core');
+}
+
 function buildDiagnosticForSignal(
     ctx: TemplateContext<ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED>, node: PropertyRead,
     component: ts.ClassDeclaration):
     Array<NgTemplateDiagnostic<ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED>> {
   const symbol = ctx.templateTypeChecker.getSymbolOfNode(node, component);
+
   if (symbol?.kind === SymbolKind.Expression &&
-      /* can this condition be improved ? */
-      (symbol.tsType.symbol?.escapedName === 'WritableSignal' ||
-       symbol.tsType.symbol?.escapedName === 'Signal') &&
-      (symbol.tsType.symbol as any).parent.escapedName.includes('@angular/core')) {
+      (isSignal(symbol.tsType.symbol) || isSignal(symbol.tsType.aliasSymbol))) {
     const templateMapping =
         ctx.templateTypeChecker.getTemplateMappingAtTcbLocation(symbol.tcbLocation)!;
 
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
@@ -20,11 +20,16 @@ function coreDtsWithSignals() {
   return {
     fileName: absoluteFrom('/node_modules/@angular/core/index.d.ts'),
     source: `
-    export class Signal<T> {};
+    export declare const SIGNAL: unique symbol;
+    export declare type Signal<T> = (() => T) & {
+        [SIGNAL]: unknown;
+    };
     export declare function signal<T>(initialValue: T): WritableSignal<T>;
     export declare function computed<T>(computation: () => T): Signal<T>;
 
-    export interface WritableSignal<T> extends Signal<T> {}
+    export interface WritableSignal<T> extends Signal<T> {
+      asReadonly(): Signal<T>;
+    }
   `,
     templates: {},
   };
@@ -97,6 +102,35 @@ runInEachFileSystem(() => {
     expect(getSourceCodeForDiagnostic(diags[1])).toBe(`mySignal2`);
   });
 
+  it('should produce a warning when a readonly signal isn\'t invoked', () => {
+    const fileName = absoluteFrom('/main.ts');
+    const {program, templateTypeChecker} = setup([
+      coreDtsWithSignals(),
+      {
+        fileName,
+        templates: {
+          'TestCmp': `<div>{{ count }}</div>`,
+        },
+        source: `
+          import {signal} from '@angular/core';
+
+          export class TestCmp {
+            count = signal(0).asReadonly();
+          }`,
+      },
+    ]);
+    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('count');
+  });
+
   it('should produce a warning when a computed signal isn\'t invoked', () => {
     const fileName = absoluteFrom('/main.ts');
     const {program, templateTypeChecker} = setup([
