refactor(compiler-cli): account for let declarations in two-way binding check (#56199)

Updates the check that prevent writes to template variables in two-way bindings to account for let declarations.

Also fixes some old tests that weren't properly setting up two-way bindings.

PR Close #56199

Author: crisbeto

packages/compiler-cli/src/ngtsc/typecheck/src/symbol_util.ts

@@ -22,7 +22,9 @@ const SIGNAL_FNS = new Set([
 /** Returns whether a symbol is a reference to a signal. */
 export function isSignalReference(symbol: Symbol): boolean {
   return (
-    (symbol.kind === SymbolKind.Expression || symbol.kind === SymbolKind.Variable) &&
+    (symbol.kind === SymbolKind.Expression ||
+      symbol.kind === SymbolKind.Variable ||
+      symbol.kind === SymbolKind.LetDeclaration) &&
     // Note that `tsType.symbol` isn't optional in the typings,
     // but it appears that it can be undefined at runtime.
     ((symbol.tsType.symbol !== undefined && isSignalSymbol(symbol.tsType.symbol)) ||

packages/compiler-cli/src/ngtsc/typecheck/template_semantics/src/template_semantics_checker.ts

@@ -15,6 +15,7 @@ import {
   PropertyWrite,
   RecursiveAstVisitor,
   TmplAstBoundEvent,
+  TmplAstLetDeclaration,
   TmplAstNode,
   TmplAstRecursiveVisitor,
   TmplAstVariable,
@@ -109,23 +110,35 @@ class ExpressionsSemanticsVisitor extends RecursiveAstVisitor {
     }
 
     const target = this.templateTypeChecker.getExpressionTarget(ast, this.component);
-    if (!(target instanceof TmplAstVariable)) {
+    const isVariable = target instanceof TmplAstVariable;
+    const isLet = target instanceof TmplAstLetDeclaration;
+
+    if (!isVariable && !isLet) {
       return;
     }
 
     // Two-way bindings to template variables are only allowed if the variables are signals.
     const symbol = this.templateTypeChecker.getSymbolOfNode(target, this.component);
     if (symbol !== null && !isSignalReference(symbol)) {
-      const errorMessage = `Cannot use a non-signal variable '${target.name}' in a two-way binding expression. Template variables are read-only.`;
+      let errorMessage: string;
+
+      if (isVariable) {
+        errorMessage = `Cannot use a non-signal variable '${target.name}' in a two-way binding expression. Template variables are read-only.`;
+      } else {
+        errorMessage = `Cannot use non-signal @let declaration '${target.name}' in a two-way binding expression. @let declarations are read-only.`;
+      }
+
       this.diagnostics.push(this.makeIllegalTemplateVarDiagnostic(target, context, errorMessage));
     }
   }
 
   private makeIllegalTemplateVarDiagnostic(
-    target: TmplAstVariable,
+    target: TmplAstVariable | TmplAstLetDeclaration,
     expressionNode: TmplAstBoundEvent,
     errorMessage: string,
   ): TemplateDiagnostic {
+    const span =
+      target instanceof TmplAstVariable ? target.valueSpan || target.sourceSpan : target.sourceSpan;
     return this.templateTypeChecker.makeTemplateDiagnostic(
       this.component,
       expressionNode.handlerSpan,
@@ -134,9 +147,9 @@ class ExpressionsSemanticsVisitor extends RecursiveAstVisitor {
       errorMessage,
       [
         {
-          text: `The variable ${target.name} is declared here.`,
-          start: target.valueSpan?.start.offset || target.sourceSpan.start.offset,
-          end: target.valueSpan?.end.offset || target.sourceSpan.end.offset,
+          text: `'${target.name}' is declared here.`,
+          start: span.start.offset,
+          end: span.end.offset,
           sourceFile: this.component.getSourceFile(),
         },
       ],

packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts

@@ -5503,7 +5503,7 @@ suppress
           })
           export class TwoWayDir {
             @Input() value: number = 0;
-            @Output() valueChanges: EventEmitter<number> = new EventEmitter();
+            @Output() valueChange: EventEmitter<number> = new EventEmitter();
           }
 
           @Component({
@@ -5539,7 +5539,7 @@ suppress
           })
           export class TwoWayDir {
             @Input() value: number = 0;
-            @Output() valueChanges: EventEmitter<number> = new EventEmitter();
+            @Output() valueChange: EventEmitter<number> = new EventEmitter();
           }
 
           @Component({
@@ -7072,6 +7072,69 @@ suppress
         expect(diags.length).toBe(1);
         expect(diags[0].messageText).toBe(`Property 'value' does not exist on type 'Main'.`);
       });
+
+      it('should not be able to write to let declaration in a two-way binding', () => {
+        env.write(
+          'test.ts',
+          `
+          import {Component, Directive, Input, Output, EventEmitter} from '@angular/core';
+          @Directive({
+            selector: '[twoWayDir]',
+            standalone: true
+          })
+          export class TwoWayDir {
+            @Input() value: number = 0;
+            @Output() valueChange: EventEmitter<number> = new EventEmitter();
+          }
+          @Component({
+            template: \`
+              @let nonWritable = 1;
+              <button twoWayDir [(value)]="nonWritable"></button>
+            \`,
+            standalone: true,
+            imports: [TwoWayDir]
+          })
+          export class Main {
+          }
+        `,
+        );
+
+        const diags = env.driveDiagnostics();
+        expect(diags.map((d) => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
+          `Cannot use non-signal @let declaration 'nonWritable' in a two-way binding expression. @let declarations are read-only.`,
+        ]);
+      });
+
+      it('should allow two-way bindings to signal-based let declarations', () => {
+        env.write(
+          'test.ts',
+          `
+          import {Component, Directive, Input, Output, EventEmitter, signal} from '@angular/core';
+          @Directive({
+            selector: '[twoWayDir]',
+            standalone: true
+          })
+          export class TwoWayDir {
+            @Input() value: number = 0;
+            @Output() valueChange: EventEmitter<number> = new EventEmitter();
+          }
+          @Component({
+            template: \`
+              @let writable = signalValue;
+              <button twoWayDir [(value)]="writable"></button>
+            \`,
+            standalone: true,
+            imports: [TwoWayDir]
+          })
+          export class Main {
+            signalValue = signal(1);
+          }
+        `,
+        );
+
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(0);
+      });
     });
   });
 });