fix: process handler_expression after handler_ops to avoid duplicate tmp declarations

handler_expression was being processed inside the per-op loop, causing it
to be visited/transformed once per handler_op. When handler_ops had multiple
entries (e.g., restoreView + nextContext), this produced spurious declarations
(let tmp_0_0; let tmp_1_0;) and wrong variable names.

Now handler_expression is processed once after the loop with op_count set to
handler_ops.len(), matching Angular TS where the return expression is the last
entry in handlerOps. For a listener with 2 handler_ops, this correctly
generates tmp_2_0 instead of tmp_1_0.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Author: Brooooooklyn

crates/oxc_angular_compiler/src/pipeline/phases/temporary_variables.rs

@@ -235,21 +235,28 @@ fn generate_temporaries_for_handler_ops<'a>(
 /// temporary variable processing so that `let tmp_N_0;` declarations are generated inside
 /// the listener function scope rather than the parent create scope.
 ///
-/// The algorithm mirrors `generate_temporaries_for_handler_ops` but additionally visits/transforms
-/// the handler_expression alongside the ops.
+/// The handler_ops are processed first (same as `generate_temporaries_for_handler_ops`),
+/// then handler_expression is processed once AFTER the loop with op_count equal to
+/// handler_ops.len(). This matches Angular TS where the return expression is the last
+/// entry in handlerOps at index N.
 fn generate_temporaries_for_handler_ops_with_expression<'a>(
     ops: &mut [UpdateOp<'a>],
     handler_expression: &mut Option<oxc_allocator::Box<'a, IrExpression<'a>>>,
     allocator: &'a Allocator,
 ) -> Vec<UpdateOp<'a>> {
-    let mut op_count = 0;
-    let mut generated_statements = Vec::new();
+    // First, process handler_ops exactly like generate_temporaries_for_handler_ops
+    let mut generated_statements = generate_temporaries_for_handler_ops(ops, allocator);
+
+    // Then process handler_expression once, with op_count = ops.len().
+    // In Angular TS, the return expression is the last entry in handlerOps,
+    // so its op index equals the number of preceding ops.
+    if let Some(handler_expr) = handler_expression.as_mut() {
+        let op_count = ops.len();
 
-    for op in ops.iter_mut() {
         // Pass 1: Count reads per xref to determine final reads
         let read_counts = RefCell::new(FxHashMap::<XrefId, usize>::default());
-        visit_expressions_in_update_op(
-            op,
+        visit_expressions_in_expression(
+            handler_expr,
             &|expr, flags| {
                 if flags.contains(VisitorContextFlag::IN_CHILD_OPERATION) {
                     return;
@@ -260,28 +267,12 @@ fn generate_temporaries_for_handler_ops_with_expression<'a>(
             },
             VisitorContextFlag::NONE,
         );
-        // Also count reads in handler_expression
-        if let Some(handler_expr) = handler_expression.as_ref() {
-            visit_expressions_in_expression(
-                handler_expr,
-                &|expr, flags| {
-                    if flags.contains(VisitorContextFlag::IN_CHILD_OPERATION) {
-                        return;
-                    }
-                    if let IrExpression::ReadTemporary(read) = expr {
-                        *read_counts.borrow_mut().entry(read.xref).or_insert(0) += 1;
-                    }
-                },
-                VisitorContextFlag::NONE,
-            );
-        }
         let total_reads = read_counts.into_inner();
 
         // Pass 2: Assign names with reuse when final read is encountered
         let tracker = RefCell::new(TempVarTracker::new(op_count, total_reads));
-
-        transform_expressions_in_update_op(
-            op,
+        transform_expressions_in_expression(
+            handler_expr,
             &|expr, flags| {
                 if flags.contains(VisitorContextFlag::IN_CHILD_OPERATION) {
                     return;
@@ -290,19 +281,6 @@ fn generate_temporaries_for_handler_ops_with_expression<'a>(
             },
             VisitorContextFlag::NONE,
         );
-        // Also assign names in handler_expression
-        if let Some(handler_expr) = handler_expression.as_mut() {
-            transform_expressions_in_expression(
-                handler_expr,
-                &|expr, flags| {
-                    if flags.contains(VisitorContextFlag::IN_CHILD_OPERATION) {
-                        return;
-                    }
-                    assign_temp_names(expr, &tracker, allocator);
-                },
-                VisitorContextFlag::NONE,
-            );
-        }
 
         // Collect unique names and create declarations
         let defs = tracker.into_inner().defs;
@@ -313,49 +291,6 @@ fn generate_temporaries_for_handler_ops_with_expression<'a>(
                 statement: stmt,
             }));
         }
-
-        op_count += 1;
-    }
-
-    // If there are no ops but handler_expression has temporaries, process it standalone
-    if ops.is_empty() {
-        if let Some(handler_expr) = handler_expression.as_mut() {
-            let read_counts = RefCell::new(FxHashMap::<XrefId, usize>::default());
-            visit_expressions_in_expression(
-                handler_expr,
-                &|expr, flags| {
-                    if flags.contains(VisitorContextFlag::IN_CHILD_OPERATION) {
-                        return;
-                    }
-                    if let IrExpression::ReadTemporary(read) = expr {
-                        *read_counts.borrow_mut().entry(read.xref).or_insert(0) += 1;
-                    }
-                },
-                VisitorContextFlag::NONE,
-            );
-            let total_reads = read_counts.into_inner();
-
-            let tracker = RefCell::new(TempVarTracker::new(op_count, total_reads));
-            transform_expressions_in_expression(
-                handler_expr,
-                &|expr, flags| {
-                    if flags.contains(VisitorContextFlag::IN_CHILD_OPERATION) {
-                        return;
-                    }
-                    assign_temp_names(expr, &tracker, allocator);
-                },
-                VisitorContextFlag::NONE,
-            );
-
-            let defs = tracker.into_inner().defs;
-            for name in collect_unique_names(&defs) {
-                let stmt = create_declare_var_statement(allocator, &name);
-                generated_statements.push(UpdateOp::Statement(StatementOp {
-                    base: UpdateOpBase::default(),
-                    statement: stmt,
-                }));
-            }
-        }
     }
 
     generated_statements

crates/oxc_angular_compiler/tests/integration_test.rs

@@ -1062,6 +1062,19 @@ fn test_safe_property_read_with_call_receiver_in_listener() {
     insta::assert_snapshot!("safe_property_read_with_call_receiver_in_listener", js);
 }
 
+#[test]
+fn test_safe_call_in_listener_inside_conditional() {
+    // When a listener is inside an embedded view (e.g., @if), handler_ops contains
+    // restoreView and nextContext statements. The handler_expression (return value)
+    // must be processed AFTER those ops, not once per op, to get the correct
+    // tmp variable name (tmp_2_0, matching the op index after restoreView and nextContext).
+    let js = compile_template_to_js(
+        r#"@if (show) { <button (click)="getPopover()?.close()">Close</button> }"#,
+        "TestComponent",
+    );
+    insta::assert_snapshot!("safe_call_in_listener_inside_conditional", js);
+}
+
 // ============================================================================
 // Event Modifier Tests
 // ============================================================================

crates/oxc_angular_compiler/tests/snapshots/integration_test__safe_call_in_listener_inside_conditional.snap

@@ -0,0 +1,24 @@
+---
+source: crates/oxc_angular_compiler/tests/integration_test.rs
+expression: js
+---
+function TestComponent_Conditional_0_Template(rf,ctx) {
+  if ((rf & 1)) {
+    const _r1 = i0.ɵɵgetCurrentView();
+    i0.ɵɵtext(0," ");
+    i0.ɵɵelementStart(1,"button",0);
+    i0.ɵɵlistener("click",function TestComponent_Conditional_0_Template_button_click_1_listener() {
+      let tmp_2_0;
+      i0.ɵɵrestoreView(_r1);
+      const ctx_r1 = i0.ɵɵnextContext();
+      return i0.ɵɵresetView((((tmp_2_0 = ctx_r1.getPopover()) == null)? null: tmp_2_0.close()));
+    });
+    i0.ɵɵtext(2,"Close");
+    i0.ɵɵelementEnd();
+    i0.ɵɵtext(3," ");
+  }
+}
+function TestComponent_Template(rf,ctx) {
+  if ((rf & 1)) { i0.ɵɵconditionalCreate(0,TestComponent_Conditional_0_Template,4,0); }
+  if ((rf & 2)) { i0.ɵɵconditional((ctx.show? 0: -1)); }
+}