fix(transform): give an exported dynamic-heritage class factory real per-evaluation identity

changelog.d/10622-exported-class-factory-identity.md

@@ -0,0 +1,26 @@
+### Fixed
+
+- **A class expression returned from a factory function had no
+  per-evaluation identity when the function lived in a non-entry module**
+  (#10455). `function withCommands(Base) { return class extends Base {}; }`
+  called twice with different `Base`s returned the SAME class object,
+  re-parented to the most recently passed `Base` — declared and called in
+  the entry module, the identical shape already worked because
+  `specialize_captured_class_factories` (`crates/perry-transform/src/inline/factory_specialize.rs`)
+  clones a distinct class per call site, but that pass only ever sees call
+  sites in the SAME module the factory Call expression appears in; a caller
+  in another module reaches the factory through an ordinary cross-module
+  call the pass never visits. redis's `commander.js` `attachConfig` (`Class
+  = class extends BaseClass {}`) hits this every time it's called with a
+  second `BaseClass`, from `RedisClient.factory` and
+  `Client.prototype.Multi`, and `new Client(options)` ran the wrong parent
+  constructor. New pass `fresh_export_dynamic_heritage_factories`: for an
+  **exported** factory whose body is exactly `return class extends <expr>
+  {};` (no other members), upgrades the class's shared-template `ClassRef`
+  to a fresh-per-evaluation `ClassExprFresh` object directly — exactly what
+  the same class expression would already lower to had it needed
+  per-evaluation statics/captures/a private brand — so every caller, local
+  or cross-module, gets a genuinely distinct class per call. Scoped to
+  exported functions and this single-statement shape only; a `Let`-bound
+  intermediate variable or Effect's object-literal wrapper factory shape
+  still rely on same-module specialization alone.

crates/perry-transform/src/inline/factory_specialize.rs

@@ -1078,3 +1078,79 @@ pub fn specialize_captured_class_factories(module: &mut Module) {
     // Flush new specialized classes.
     module.classes.extend(new_classes);
 }
+
+/// #10455: `specialize_captured_class_factories` above only rewrites a
+/// factory's CALL SITES, and only visits the module the Call expression
+/// appears in — a caller in ANOTHER module reaches the factory through an
+/// ordinary cross-module call this pass never sees. An exported factory
+/// (`export function withCommands(Base) { return class extends Base {}; }`,
+/// redis's `commander.js` `attachConfig` mixin shape) then keeps returning
+/// the ONE shared-template `ClassRef` every local call above would
+/// otherwise have cloned, and each call's `RegisterClassParentDynamic`
+/// silently re-parents that single shared class in place: `withCommands(A)
+/// === withCommands(B)` where the spec requires two distinct classes, and
+/// an earlier caller's result is corrupted by a later call.
+///
+/// Give an exported factory real per-EVALUATION identity directly, instead
+/// of relying on caller-side cloning that cannot reach outside the module:
+/// when its body is nothing but `return class extends <expr> {…}` — lowered
+/// to `Sequence([RegisterClassParentDynamic { class_name, parent_expr },
+/// ClassRef(class_name)])` by `crates/perry-hir/src/lower/lower_expr/
+/// arm_class.rs` — upgrade the trailing `ClassRef` to `ClassExprFresh`,
+/// exactly what the same class expression would have lowered to had it
+/// needed per-evaluation statics/captures/a private brand.
+/// `named_statics`/`computed_keys`/`captured_args`/static blocks/private
+/// elements/self-binding are all empty here by construction: any of those
+/// would already have forced `arm_class.rs` onto the `ClassExprFresh` route
+/// at lowering time, so a class that still presents as a bare `ClassRef`
+/// never had them.
+///
+/// Scoped to this exact single-statement direct-return shape — the filed
+/// repro's own pattern. A Let-bound intermediate variable, computed-name
+/// evaluations, or Effect's object-literal wrapper shape are left to the
+/// same-module handling above; those still work correctly for a caller in
+/// the SAME module as the factory (the specialization this file already
+/// performs), just not yet for a caller reached only across modules.
+pub fn fresh_export_dynamic_heritage_factories(module: &mut Module) {
+    if module.exported_functions.is_empty() {
+        return;
+    }
+    let exported: HashSet<FuncId> = module
+        .exported_functions
+        .iter()
+        .map(|(_, id)| *id)
+        .collect();
+    for f in &mut module.functions {
+        if !exported.contains(&f.id) {
+            continue;
+        }
+        let [Stmt::Return(Some(Expr::Sequence(parts)))] = f.body.as_mut_slice() else {
+            continue;
+        };
+        if parts.len() != 2 {
+            continue;
+        }
+        let is_dynamic_heritage_classref = matches!(
+            (&parts[0], &parts[1]),
+            (
+                Expr::RegisterClassParentDynamic { class_name: reg, .. },
+                Expr::ClassRef(rf),
+            ) if reg == rf
+        );
+        if !is_dynamic_heritage_classref {
+            continue;
+        }
+        let Expr::ClassRef(template) = parts.remove(1) else {
+            unreachable!("matched Expr::ClassRef(_) above");
+        };
+        parts.push(Expr::ClassExprFresh {
+            template,
+            evaluation_owner: None,
+            named_statics: Vec::new(),
+            computed_keys: Vec::new(),
+            computed_statics: Vec::new(),
+            static_init_order: Vec::new(),
+            captured_args: Vec::new(),
+        });
+    }
+}

crates/perry-transform/src/inline/mod.rs

@@ -44,7 +44,9 @@ pub(crate) use exact_receivers::{
     collect_module_prototype_facts, intersect_exact_receiver_facts,
     invalidate_exact_receivers_for_expr, kill_referenced_exact_receivers,
 };
-pub(crate) use factory_specialize::specialize_captured_class_factories;
+pub(crate) use factory_specialize::{
+    fresh_export_dynamic_heritage_factories, specialize_captured_class_factories,
+};
 pub(crate) use imul::{detect_math_imul_polyfill, rewrite_imul_calls_in_stmts};
 pub(crate) use substitute::{
     collect_body_local_ids, substitute_locals, substitute_locals_in_stmts, substitute_this,
@@ -497,6 +499,14 @@ fn inline_functions_inner(
     // no-op for the rewritten sites.
     specialize_captured_class_factories(module);
 
+    // #10455: same-module call sites above are cloned per call site;
+    // an EXPORTED factory also needs real per-evaluation identity for
+    // callers outside this module, which no per-module call-site pass
+    // can see. See `fresh_export_dynamic_heritage_factories`'s own doc
+    // comment for the exact shape and why it's safe to run after the
+    // pass above.
+    fresh_export_dynamic_heritage_factories(module);
+
     // Phases 0 + 1 fused (Tier 4.1, v0.5.335): single iteration over
     // module.functions collects both Math.imul polyfill ids AND
     // inlinable-function candidates. Pre-Tier-4 these were two separate

test-files/gap_10455_class_expr_factory_identity_helper.ts

@@ -0,0 +1,11 @@
+// Non-entry module half of test_gap_10455_class_expr_factory_identity.ts —
+// the exact shape redis's `commander.js` `attachConfig` uses
+// (`Class = class extends BaseClass {}`), reached through an ordinary
+// cross-module call.
+export function withCommands(Base: any) {
+  return class extends Base {};
+}
+
+export function withCommandsB(Base: any) {
+  return class extends Base {};
+}

test-files/test_gap_10455_class_expr_factory_identity.ts

@@ -0,0 +1,82 @@
+// #10455: a class expression returned from a function had no per-evaluation
+// identity when the function lived in a NON-ENTRY module — every call
+// returned the SAME class object, re-parented to the most recently passed
+// `Base`. A mixin/factory declared and called in the entry module (or
+// specialized per call site by `specialize_captured_class_factories`) always
+// worked; the same shape reached through an ordinary cross-module call did
+// not, because the per-module specialization pass never sees callers outside
+// its own module. redis's `commander.js` `attachConfig` (`Class = class
+// extends BaseClass {}`) hits exactly this from `RedisClient.factory` and
+// `Client.prototype.Multi = MultiCommand.extend(config)`.
+import { withCommands, withCommandsB } from "./gap_10455_class_expr_factory_identity_helper.ts";
+
+class A {
+  constructor() {
+    console.log("  A constructor");
+  }
+  hello() {
+    return "A.hello";
+  }
+}
+class B {
+  constructor() {
+    console.log("  B constructor");
+  }
+  world() {
+    return "B.world";
+  }
+}
+
+function localWithCommands(Base: any) {
+  return class extends Base {};
+}
+
+// ── entry-module factory: two calls, two evaluations ──
+const LocalA = localWithCommands(A);
+const LocalB = localWithCommands(B);
+const la = new LocalA();
+console.log(
+  "entry module:   LocalA !== LocalB:",
+  LocalA !== LocalB,
+  "| instanceof A:",
+  la instanceof A,
+  "| typeof hello:",
+  typeof (la as any).hello,
+);
+
+// ── non-entry-module factory: two calls, two evaluations ──
+const ClientA = withCommands(A);
+const ClientB = withCommands(B);
+const ca = new ClientA();
+const cb = new ClientB();
+console.log(
+  "non-entry module: ClientA !== ClientB:",
+  ClientA !== ClientB,
+  "| instanceof A:",
+  ca instanceof A,
+  "| typeof hello:",
+  typeof (ca as any).hello,
+);
+console.log(
+  "non-entry module: cb instanceof B:",
+  cb instanceof B,
+  "| cb instanceof A:",
+  cb instanceof A,
+  "| typeof world:",
+  typeof (cb as any).world,
+);
+// ── three sequential calls at the SAME call site (loop) also stay distinct ──
+const made: any[] = [];
+for (let i = 0; i < 3; i++) {
+  made.push(withCommandsB(i % 2 === 0 ? A : B));
+}
+console.log(
+  "loop calls pairwise distinct:",
+  made[0] !== made[1] && made[1] !== made[2] && made[0] !== made[2],
+);
+console.log(
+  "loop instances match their own parent:",
+  new made[0]() instanceof A,
+  new made[1]() instanceof B,
+  new made[2]() instanceof A,
+);