fix(transform): do not beta-reduce a local arrow whose param is captured by a nested closure

changelog.d/10653-arrow-param-capture.md

@@ -0,0 +1,37 @@
+Fixed: a closure created inside a local arrow function that captured the
+arrow's OWN parameter kept seeing the FIRST call's argument on every later
+call to the same arrow — a per-call closure that should differ between
+`iter(fields, false)` and `iter(optFields, true)` silently ran the first
+call's body for both. This blocked `@noble/curves` 2.2.0's
+`validateObject` helper, among any code shaped like a local arrow that
+constructs a callback for `forEach`/`map`/etc. and hands it a value derived
+from the arrow's own parameter.
+
+Root cause: `closure_local_inline` (`crates/perry-transform/src/closure_local_inline.rs`)
+beta-reduces a local `let f = (a, b) => <expr>` closure that is only ever
+called, cloning the return expression fresh per call site and substituting
+each parameter via `substitute_locals`. For a parameter read inside a
+NESTED closure, `substitute_locals` bakes a non-`LocalGet` argument straight
+into that nested closure's body and drops it from the closure's `captures`
+list — correct for one clone in isolation — but never mints a fresh
+`func_id`, and codegen compiles exactly one body per `func_id` (whichever
+`Expr::Closure` occurrence it encounters first). With more than one call
+site, every clone of the nested closure shared the same `func_id`, so only
+the first-seen clone's baked-in argument was ever compiled.
+
+Fix: `arrow_candidate` now bails out of the beta-reduction when any
+parameter is captured by a closure nested in the arrow's body (reusing the
+`collect_closure_captured_local_ids` helper the #858 fix already
+established for the sibling FuncRef-keyed inliner), leaving such an arrow
+as a real, per-call closure.
+
+Validation: new gap test
+(`test_gap_10567_arrow_param_closure_capture.ts`) covering the issue repro
+plus several-params, nested-arrows, arrow-in-a-method, and by-write-capture
+variants — proven to fail on the pre-fix tree (e.g. `A:1,B:1` → wrongly
+prints the first call's value on later calls) and pass on this one,
+byte-identical to Node 26.5.1. `cargo test --release -p perry-transform
+--tests`: 152 passed. Instructions regress ~7.3% for the exact bug shape
+(the correctness cost of no longer sharing one wrongly-baked closure body
+across call sites with different arguments); the safe single-call-site
+shape is unaffected (within noise).

crates/perry-transform/src/closure_local_inline.rs

@@ -183,6 +183,33 @@ fn arrow_candidate(id: LocalId, init: &Expr) -> Option<(LocalId, Vec<LocalId>, E
     let [Stmt::Return(Some(expr))] = body.as_slice() else {
         return None;
     };
+    // #10567: a param captured by a closure NESTED inside `expr` (e.g. `(f,
+    // isOpt) => arr.forEach(([k, v]) => check(k, v, isOpt))`) cannot be
+    // beta-reduced the way a plain read can. `rewrite_calls` clones
+    // `body_expr` fresh per call site and hands the clone to
+    // `substitute_locals`, which — for a nested `Expr::Closure` — bakes a
+    // non-`LocalGet` argument straight into that closure's body and drops
+    // it from its `captures` list (see `inline/substitute.rs`'s
+    // `Expr::Closure` arm), but never mints a fresh `func_id` for the
+    // rewritten closure literal. Codegen compiles exactly one body per
+    // `func_id` (whichever occurrence its module-wide closure scan sees
+    // first), so every call site's clone of the nested closure keeps
+    // sharing the SAME `func_id` — once there is more than one call site,
+    // only the first-seen clone's baked-in argument is ever compiled, and
+    // every other call silently runs it too. Bail out when any param is
+    // captured by a nested closure so such an arrow is left as a real,
+    // per-call closure — each invocation then creates its own closure
+    // instance whose nested callback correctly captures that call's
+    // argument by reference (the existing, non-beta-reduced path already
+    // gets this right).
+    let mut closure_captured_params = std::collections::HashSet::new();
+    crate::inline::collect_closure_captured_local_ids(body, &mut closure_captured_params);
+    if params
+        .iter()
+        .any(|p| closure_captured_params.contains(&p.id))
+    {
+        return None;
+    }
     Some((id, params.iter().map(|p| p.id).collect(), expr.clone()))
 }
 

test-files/test_gap_10567_arrow_param_closure_capture.ts

@@ -0,0 +1,96 @@
+// #10567: a closure created inside an arrow function captured the arrow's
+// OWN parameter by first-call value -- a later call to the same arrow still
+// saw the FIRST call's argument inside the nested closure.
+//
+// Root cause: `closure_local_inline` (crates/perry-transform/src/closure_local_inline.rs)
+// beta-reduces a `let f = (a, b) => <single return expr>` local that is only
+// ever called, cloning the return expression fresh per call site and
+// substituting each parameter with that call's own argument via
+// `substitute_locals`. When a parameter is read inside a NESTED closure
+// (e.g. `(f, isOpt) => arr.forEach(([k, v]) => check(k, v, isOpt))`),
+// `substitute_locals`'s `Expr::Closure` arm bakes a non-`LocalGet` argument
+// straight into that nested closure's body and drops it from the closure's
+// `captures` list -- but never mints a fresh `func_id` for the rewritten
+// closure literal. Codegen compiles exactly one body per `func_id`
+// (whichever `Expr::Closure` occurrence its module-wide scan sees first), so
+// every call site's clone of the nested closure kept sharing the SAME
+// `func_id`: with more than one call site, only the first-seen clone's
+// baked-in argument was ever compiled, and every other call silently ran it
+// too.
+
+function validate(fields: any = {}, optFields: any = {}) {
+  function check(name: string, t: string, isOpt: boolean) {
+    console.log(name, t, isOpt);
+  }
+  const iter = (f: any, isOpt: boolean) =>
+    Object.entries(f).forEach(([k, v]) => check(k, v as string, isOpt));
+  iter(fields, false);
+  iter(optFields, true); // the inner arrow must see isOpt === true here
+}
+validate({ x: "number" }, { a: "boolean" });
+
+// Several params: the CAPTURED one is not the first, and not the last.
+function severalParams() {
+  const combine = (prefix: string, mid: number, tag: boolean) =>
+    [1, 2].forEach((v) => console.log(prefix, mid, v, tag));
+  combine("A", 1, false);
+  combine("B", 2, true);
+}
+severalParams();
+
+// Nested arrows: outer -> middle (forwards) -> inner (captures outer's own
+// param transitively, two closure boundaries away).
+function nestedArrows() {
+  const outer = (tag: string) => {
+    const middle = () => [10, 20].forEach((v) => console.log(tag, v));
+    return middle();
+  };
+  outer("first");
+  outer("second");
+}
+nestedArrows();
+
+// Arrow inside a class METHOD: same shape as `iter` above, but declared
+// inside a method body rather than a plain function.
+class Validator {
+  run() {
+    const iter = (isOpt: boolean) =>
+      [1, 2].forEach((v) => console.log("method", v, isOpt));
+    iter(false);
+    iter(true);
+  }
+}
+new Validator().run();
+
+// Capturing the param by WRITE inside the nested closure (a multi-statement
+// arrow body, so it never becomes a `closure_local_inline` candidate at
+// all -- this is a control that should keep working, matching the
+// already-correct `outer`/`inner` shape from the original issue).
+function byWrite() {
+  const make = (isOpt: boolean) => {
+    let seen = isOpt;
+    [1, 2].forEach((v) => {
+      seen = seen || v > 1;
+    });
+    return seen;
+  };
+  console.log(make(false), make(true));
+}
+byWrite();
+
+// Plain-function control that already worked on Perry: a directly-invoked
+// inner closure, and a function declaration called twice.
+const calls: any[] = [];
+function outer(tag: string) {
+  const inner = (v: number) => calls.push(tag + ":" + v);
+  inner(1);
+}
+outer("A");
+outer("B");
+console.log(calls.join(","));
+
+function twice(p: boolean) {
+  const g = () => p;
+  return g();
+}
+console.log(twice(false), twice(true));