fix(compiler): narrow the type of expressions in event listeners inside if blocks (#52069)

Since expressions in event listener are added inside of a callback, type narrowing won't apply to them anymore. These changes add the logic to create a guard expression that will re-narrow the expression in the callback.

Fixes #52052.

PR Close #52069

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

@@ -1208,6 +1208,8 @@ class TcbBlockImplicitVariableOp extends TcbOp {
  * Executing this operation returns nothing.
  */
 class TcbIfOp extends TcbOp {
+  private expressionScopes = new Map<TmplAstIfBlockBranch, Scope>();
+
   constructor(private tcb: Context, private scope: Scope, private block: TmplAstIfBlock) {
     super();
   }
@@ -1231,29 +1233,81 @@ class TcbIfOp extends TcbOp {
 
     // If the expression is null, it means that it's an `else` statement.
     if (branch.expression === null) {
-      const branchScope = Scope.forNodes(this.tcb, this.scope, null, branch.children, null);
+      const branchScope = Scope.forNodes(
+          this.tcb, this.scope, null, branch.children, this.generateBranchGuard(index));
       return ts.factory.createBlock(branchScope.render());
     }
 
-    let branchParentScope: Scope;
+    // We need to process the expression first so it gets its own scope that the body of the
+    // conditional will inherit from. We do this, because we need to declare a separate variable
+    // for the case where the expression has an alias _and_ because we need the processed
+    // expression when generating the guard for the body.
+    const expressionScope = Scope.forNodes(this.tcb, this.scope, branch, [], null);
+    expressionScope.render().forEach(stmt => this.scope.addStatement(stmt));
+    this.expressionScopes.set(branch, expressionScope);
 
-    if (branch.expressionAlias === null) {
-      branchParentScope = this.scope;
-    } else {
-      // If the expression is aliased, we create a scope with a variable containing the expression.
-      // Further down we'll use the variable instead of the expression itself in the `if` statement.
-      // This allows for the type of the alias to be narrowed.
-      branchParentScope = Scope.forNodes(this.tcb, this.scope, branch, [], null);
-      branchParentScope.render().forEach(stmt => this.scope.addStatement(stmt));
-    }
-
-    const branchScope = Scope.forNodes(this.tcb, branchParentScope, null, branch.children, null);
     const expression = branch.expressionAlias === null ?
-        tcbExpression(branch.expression, this.tcb, branchScope) :
-        branchScope.resolve(branch.expressionAlias);
+        tcbExpression(branch.expression, this.tcb, expressionScope) :
+        expressionScope.resolve(branch.expressionAlias);
+
+    const bodyScope = Scope.forNodes(
+        this.tcb, expressionScope, null, branch.children, this.generateBranchGuard(index));
 
     return ts.factory.createIfStatement(
-        expression, ts.factory.createBlock(branchScope.render()), this.generateBranch(index + 1));
+        expression, ts.factory.createBlock(bodyScope.render()), this.generateBranch(index + 1));
+  }
+
+  private generateBranchGuard(index: number): ts.Expression|null {
+    let guard: ts.Expression|null = null;
+
+    // Since event listeners are inside callbacks, type narrowing doesn't apply to them anymore.
+    // To recreate the behavior, we generate an expression that negates all the values of the
+    // branches _before_ the current one, and then we add the current branch's expression on top.
+    // For example `@if (expr === 1) {} @else if (expr === 2) {} @else if (expr === 3)`, the guard
+    // for the last expression will be `!(expr === 1) && !(expr === 2) && expr === 3`.
+    for (let i = 0; i <= index; i++) {
+      const branch = this.block.branches[i];
+
+      // Skip over branches without an expression.
+      if (branch.expression === null) {
+        continue;
+      }
+
+      // This shouldn't happen since all the state is handled
+      // internally, but we have the check just in case.
+      if (!this.expressionScopes.has(branch)) {
+        throw new Error(`Could not determine expression scope of branch at index ${i}`);
+      }
+
+      const expressionScope = this.expressionScopes.get(branch)!;
+      let expression: ts.Expression;
+
+      if (branch.expressionAlias === null) {
+        // We need to recreate the expression and mark it to be ignored for diagnostics,
+        // because it was already checked as a part of the block's condition and we don't
+        // want it to produce a duplicate diagnostic.
+        expression = tcbExpression(branch.expression, this.tcb, expressionScope);
+        markIgnoreDiagnostics(expression);
+      } else {
+        expression = expressionScope.resolve(branch.expressionAlias);
+      }
+
+      // The expressions of the preceeding branches have to be negated
+      // (e.g. `expr` becomes `!(expr)`) when comparing in the guard, except
+      // for the branch's own expression which is preserved as is.
+      const comparisonExpression = i === index ?
+          expression :
+          ts.factory.createPrefixUnaryExpression(
+              ts.SyntaxKind.ExclamationToken, ts.factory.createParenthesizedExpression(expression));
+
+      // Finally add the expression to the guard with an && operator.
+      guard = guard === null ?
+          comparisonExpression :
+          ts.factory.createBinaryExpression(
+              guard, ts.SyntaxKind.AmpersandAmpersandToken, comparisonExpression);
+    }
+
+    return guard;
   }
 }
 

packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts

@@ -1418,6 +1418,30 @@ describe('type check blocks', () => {
               'else { "" + ((this).other()); }');
     });
 
+    it('should generate a guard expression for listener inside conditional', () => {
+      const TEMPLATE = `
+        @if (expr === 0) {
+          <button (click)="zero()"></button>
+        } @else if (expr === 1) {
+          <button (click)="one()"></button>
+        } @else if (expr === 2) {
+          <button (click)="two()"></button>
+        } @else {
+          <button (click)="otherwise()"></button>
+        }
+      `;
+
+      const result = tcb(TEMPLATE);
+
+      expect(result).toContain(`if ((((this).expr)) === (0)) (this).zero();`);
+      expect(result).toContain(
+          `if (!((((this).expr)) === (0)) && (((this).expr)) === (1)) (this).one();`);
+      expect(result).toContain(
+          `if (!((((this).expr)) === (0)) && !((((this).expr)) === (1)) && (((this).expr)) === (2)) (this).two();`);
+      expect(result).toContain(
+          `if (!((((this).expr)) === (0)) && !((((this).expr)) === (1)) && !((((this).expr)) === (2))) (this).otherwise();`);
+    });
+
     it('should generate an if block with an `as` expression', () => {
       const TEMPLATE = `@if (expr === 1; as alias) {
         {{alias}}

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

@@ -3831,7 +3831,7 @@ suppress
         ]);
       });
 
-      it('should check narrow the type in the alias', () => {
+      it('should narrow the type of the if alias', () => {
         env.write('test.ts', `
           import {Component} from '@angular/core';
 
@@ -3856,6 +3856,31 @@ suppress
         ]);
       });
 
+      it('should narrow the type of the if alias used in a listener', () => {
+        env.write('test.ts', `
+          import {Component} from '@angular/core';
+
+          @Component({
+            template: \`@if (value; as alias) {
+              <button (click)="acceptsNumber(alias)"></button>
+            }\`,
+            standalone: true,
+          })
+          export class Main {
+            value: 'one' | 0 = 0;
+
+            acceptsNumber(value: number) {
+              return value;
+            }
+          }
+        `);
+
+        const diags = env.driveDiagnostics();
+        expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
+          `Argument of type 'string' is not assignable to parameter of type 'number'.`,
+        ]);
+      });
+
       it('should not expose the aliased expression outside of the main block', () => {
         env.write('test.ts', `
           import {Component} from '@angular/core';
@@ -3939,6 +3964,93 @@ suppress
         ]);
       });
 
+      it('should narrow the type in listeners inside if blocks', () => {
+        env.write('test.ts', `
+          import {Component} from '@angular/core';
+
+          @Component({
+            template: \`
+              @if (expr === 'hello') {
+                <button (click)="acceptsNumber(expr)"></button>
+              }
+            \`,
+            standalone: true,
+          })
+          export class Main {
+            expr: 'hello' | 1 = 'hello';
+
+            acceptsNumber(value: number) {
+              return value;
+            }
+          }
+        `);
+
+        const diags = env.driveDiagnostics();
+        expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
+          `Argument of type 'string' is not assignable to parameter of type 'number'.`,
+        ]);
+      });
+
+      it('should narrow the type in listeners inside else if blocks', () => {
+        env.write('test.ts', `
+          import {Component} from '@angular/core';
+
+          @Component({
+            template: \`
+              @if (expr === 1) {
+                One
+              } @else if (expr === 'hello') {
+                <button (click)="acceptsNumber(expr)"></button>
+              }
+            \`,
+            standalone: true,
+          })
+          export class Main {
+            expr: 'hello' | 1 | 2 = 'hello';
+
+            acceptsNumber(value: number) {
+              return value;
+            }
+          }
+        `);
+
+        const diags = env.driveDiagnostics();
+        expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
+          `Argument of type 'string' is not assignable to parameter of type 'number'.`,
+        ]);
+      });
+
+      it('should narrow the type in listeners inside else blocks', () => {
+        env.write('test.ts', `
+          import {Component} from '@angular/core';
+
+          @Component({
+            template: \`
+              @if (expr === 1) {
+                One
+              } @else if (expr === 2) {
+                Two
+              } @else {
+                <button (click)="acceptsNumber(expr)"></button>
+              }
+            \`,
+            standalone: true,
+          })
+          export class Main {
+            expr: 'hello' | 1 | 2 = 'hello';
+
+            acceptsNumber(value: number) {
+              return value;
+            }
+          }
+        `);
+
+        const diags = env.driveDiagnostics();
+        expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
+          `Argument of type 'string' is not assignable to parameter of type 'number'.`,
+        ]);
+      });
+
       it('should check bindings inside switch blocks', () => {
         env.write('test.ts', `
           import {Component} from '@angular/core';
@@ -4126,6 +4238,30 @@ suppress
           `Argument of type 'string' is not assignable to parameter of type 'number'.`
         ]);
       });
+
+      it('should produce a single diagnostic for an invalid expression of a block containing a event listener',
+         () => {
+           env.write('test.ts', `
+              import {Component} from '@angular/core';
+
+              @Component({
+                template: \`
+                  @if (does_not_exist) {
+                    <button (click)="test()"></button>
+                  }
+                \`,
+                standalone: true,
+              })
+              export class Main {
+                test() {}
+              }
+            `);
+
+           const diags = env.driveDiagnostics();
+           expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
+             `Property 'does_not_exist' does not exist on type 'Main'.`,
+           ]);
+         });
     });
 
     describe('for loop blocks', () => {