fix(codegen): stop new(X) picking a builtin over a same-named imported function ctor

changelog.d/10608-new-cast-builtin-shadow.md

@@ -0,0 +1,3 @@
+### Fixed
+
+- **`new X()` on an imported plain-function constructor whose NAME collides with a builtin (`Headers`, and any other unconditional `lower_builtin_new` arm) constructed the builtin instead of the user's own function.** `lower_new_impl_inner` (`crates/perry-codegen/src/lower_call/new.rs`) called into the unconditional builtin-constructor table for any `class_name` absent from `ctx.classes` *before* checking `ctx.import_function_prefixes` (where an imported plain-function constructor is tracked). Imported CLASSES already land in `ctx.classes` and always skipped the builtin table — only the function-declaration form was exposed. `new X()` and `new (X as any)()` are equivalent from this function downward: HIR's `peel_new_callee` strips a `TsAs` cast before `lower_new` ever branches on the callee's shape, so both forms shared the bug identically; the cast in the original report is not itself the trigger — a local-variable alias (which takes a completely different, value-based codegen path) is what actually distinguished working from broken. Fixed by adding a `user_owns_construction` guard (mirrors the existing `required_sources` provenance gate already used for `Client`/`Pool`/`Database`/etc.) to the same condition that lets classes skip the builtin block. Verified the fix is a no-op for every already-correct `new` call via byte-identical emitted LLVM IR and a `perf stat` instruction-count A/B (both within noise).

crates/perry-codegen/src/lower_call/mod.rs

@@ -85,6 +85,8 @@ mod native_module_rooting_tests;
 mod native_table;
 mod new;
 pub(crate) mod new_alloc;
+#[cfg(test)]
+mod new_builtin_shadow_tests;
 mod new_ctor_args;
 mod new_error_init;
 mod new_helpers;

crates/perry-codegen/src/lower_call/new.rs

@@ -276,7 +276,24 @@ fn lower_new_impl_inner<'a>(
     // These are checked BEFORE the ctx.classes lookup because the user
     // code may shadow the name — if they do, the class lookup below
     // wins.
-    if !ctx.classes.contains_key(class_name) {
+    //
+    // #10589: that shadowing check only covered CLASSES — imported classes
+    // ARE registered in `ctx.classes` for the importing module, so they
+    // already skip this block. A user-imported PLAIN FUNCTION constructor
+    // of the same name (`import { Headers } from "./lib.ts"`) never lands
+    // in `ctx.classes`, so any builtin arm not gated by `required_sources`
+    // (`Headers`, `EventEmitter`, …) fired unconditionally and constructed
+    // the BUILTIN instead of the user's function — for a bare identifier
+    // callee exactly as much as for one wrapped in `(X as any)`, since
+    // `peel_new_callee` strips that cast before `lower_new` ever branches
+    // on the callee shape. Route a genuine imported-function-constructor
+    // name past the whole builtin block the same way `ctx.classes` already
+    // does for classes; it falls through to the `import_function_prefixes`
+    // arm below `ctx.classes.get(class_name)`, which constructs the user's
+    // function correctly via `js_new_function_construct`.
+    let user_owns_construction = ctx.import_function_prefixes.contains_key(class_name)
+        && !ctx.import_function_v8_specifiers.contains_key(class_name);
+    if !ctx.classes.contains_key(class_name) && !user_owns_construction {
         if matches!(class_name, "Crypto" | "CryptoKey" | "SubtleCrypto") {
             for a in args {
                 let _ = lower_expr(ctx, a)?;

crates/perry-codegen/src/lower_call/new_builtin_shadow_tests.rs

@@ -0,0 +1,159 @@
+//! #10589: `new X(...)` where `X` is a bare identifier whose NAME collides
+//! with an unconditional (not `required_sources`-gated) builtin constructor
+//! arm in `lower_builtin_new` must build the USER'S imported binding when one
+//! exists, not the builtin.
+//!
+//! `lower_new_impl_inner` called `lower_builtin_new` for any `class_name`
+//! absent from `ctx.classes` before ever checking `ctx.import_function_prefixes`
+//! (where an imported PLAIN FUNCTION constructor is tracked — imported
+//! CLASSES already land in `ctx.classes` and skip this block entirely, see
+//! the `Headers`-as-class regression guard below, which passed before this
+//! fix too). Since `"Headers"` has no `required_sources` gate, it fired
+//! unconditionally.
+//!
+//! `new X()` and `new (X as any)()` are equivalent from `lower_new_impl_inner`
+//! downward — HIR's `peel_new_callee` strips a `TsAs` cast before `lower_new`
+//! ever branches on the callee's shape, so both forms lower to the identical
+//! `Expr::New { class_name: "Headers", .. }`. There is deliberately no
+//! separate "cast" test here for that reason; the two forms are provably one
+//! code path once the AST reaches `Expr::New`.
+
+use crate::{compile_module, CompileOptions, ImportedClass};
+use perry_hir::{Expr, Module, Stmt};
+
+fn new_headers_call() -> Module {
+    let mut module = Module::new("new_builtin_shadow.ts");
+    module.init = vec![Stmt::Expr(Expr::New {
+        class_name: "Headers".to_string(),
+        args: Vec::new(),
+        type_args: Vec::new(),
+        byte_offset: 0,
+        cap_args_appended: 0,
+    })];
+    module
+}
+
+fn compile(opts: CompileOptions) -> String {
+    let bytes = compile_module(&new_headers_call(), opts).expect("module compiles");
+    String::from_utf8(bytes).expect("LLVM IR is UTF-8")
+}
+
+#[test]
+fn imported_function_constructor_shadows_the_builtin_arm() {
+    let mut opts = CompileOptions {
+        emit_ir_only: true,
+        ..Default::default()
+    };
+    opts.import_function_prefixes
+        .insert("Headers".to_string(), "lib_ts".to_string());
+    let ir = compile(opts);
+
+    assert!(
+        ir.contains("call double @js_new_function_construct("),
+        "an imported function constructor named `Headers` must construct \
+         through the imported-function path:\n{ir}"
+    );
+    assert!(
+        !ir.contains("call double @js_headers_new("),
+        "the builtin fetch Headers constructor must not fire once `Headers` \
+         resolves to an imported function (#10589):\n{ir}"
+    );
+}
+
+#[test]
+fn unshadowed_builtin_name_still_builds_the_builtin() {
+    // No `import_function_prefixes` entry for "Headers": nothing shadows the
+    // name, so the builtin fetch API constructor must still fire. Guards
+    // against an overly broad fix that stops builtin `new Headers()` from
+    // working when the program never imports anything of that name.
+    let ir = compile(CompileOptions {
+        emit_ir_only: true,
+        ..Default::default()
+    });
+
+    assert!(
+        ir.contains("call double @js_headers_new("),
+        "an unshadowed `Headers` must still build the builtin:\n{ir}"
+    );
+    assert!(
+        !ir.contains("call double @js_new_function_construct("),
+        "nothing resolves this name to an imported function value:\n{ir}"
+    );
+}
+
+#[test]
+fn v8_fallback_import_of_the_same_name_still_builds_the_builtin() {
+    // A V8-fallback specifier for "Headers" (present in
+    // `import_function_prefixes` but ALSO in `import_function_v8_specifiers`)
+    // is not a compiled-source binding this fix can construct via
+    // `js_new_function_construct` — it must keep falling through to the
+    // builtin, same as the codegen's existing `import_function_v8_specifiers`
+    // exclusion at the later `import_function_prefixes` arm.
+    let mut opts = CompileOptions {
+        emit_ir_only: true,
+        ..Default::default()
+    };
+    opts.import_function_prefixes
+        .insert("Headers".to_string(), "lib_ts".to_string());
+    opts.import_function_v8_specifiers
+        .insert("Headers".to_string(), "./lib.ts".to_string());
+    let ir = compile(opts);
+
+    assert!(
+        ir.contains("call double @js_headers_new("),
+        "a V8-fallback import must still build the builtin:\n{ir}"
+    );
+}
+
+#[test]
+fn imported_class_of_the_same_name_already_shadowed_the_builtin() {
+    // Regression guard for the OTHER half of the shadowing story, unchanged
+    // by this fix: an imported CLASS named "Headers" lands in `ctx.classes`
+    // and always skipped the builtin block, function-constructor collisions
+    // aside.
+    let mut opts = CompileOptions {
+        emit_ir_only: true,
+        ..Default::default()
+    };
+    opts.imported_classes.push(ImportedClass {
+        name: "Headers".to_string(),
+        local_alias: None,
+        namespace: None,
+        source_prefix: "lib_ts".to_string(),
+        constructor_param_count: 0,
+        has_own_constructor: false,
+        constructor_has_rest: false,
+        has_instance_fields: false,
+        method_names: Vec::new(),
+        proven_this_method_names: Vec::new(),
+        proven_this_tower_method_names: Vec::new(),
+        method_return_types: Vec::new(),
+        method_param_counts: Vec::new(),
+        method_has_rest: Vec::new(),
+        method_has_synthetic_arguments: Vec::new(),
+        method_arguments_length_only: Vec::new(),
+        static_field_names: Vec::new(),
+        static_method_names: Vec::new(),
+        static_method_return_types: Vec::new(),
+        static_method_param_counts: Vec::new(),
+        static_method_has_rest: Vec::new(),
+        static_method_has_user_rest: Vec::new(),
+        static_method_has_synthetic_arguments: Vec::new(),
+        getter_names: Vec::new(),
+        getter_return_types: Vec::new(),
+        setter_names: Vec::new(),
+        parent_name: None,
+        field_names: Vec::new(),
+        field_types: Vec::new(),
+        source_class_id: Some(9101),
+        return_shape_imports: Vec::new(),
+        object_literal: None,
+    });
+    let ir = compile(opts);
+
+    assert!(
+        !ir.contains("call double @js_headers_new("),
+        "an imported class must already shadow the builtin, before and \
+         after #10589's fix:\n{ir}"
+    );
+}

test-files/_helpers/new_cast_builtin_shadow_10589/d_emitter_class_default.ts

@@ -0,0 +1,26 @@
+import EventEmitter from "./emitter_class_lib.ts";
+
+function isUser(o: any): boolean {
+  return !!(o && (o as any).__mark === "user");
+}
+
+export function run(): string {
+  const results: string[] = [];
+  try {
+    results.push("plain=" + isUser(new EventEmitter()));
+  } catch (e: any) {
+    results.push("plain=THROW:" + (e && e.message));
+  }
+  try {
+    results.push("cast=" + isUser(new (EventEmitter as any)()));
+  } catch (e: any) {
+    results.push("cast=THROW:" + (e && e.message));
+  }
+  const alias: any = EventEmitter;
+  try {
+    results.push("alias=" + isUser(new alias()));
+  } catch (e: any) {
+    results.push("alias=THROW:" + (e && e.message));
+  }
+  return results.join(" ");
+}

test-files/_helpers/new_cast_builtin_shadow_10589/d_emitter_class_named.ts

@@ -0,0 +1,26 @@
+import { EventEmitter } from "./emitter_class_lib.ts";
+
+function isUser(o: any): boolean {
+  return !!(o && (o as any).__mark === "user");
+}
+
+export function run(): string {
+  const results: string[] = [];
+  try {
+    results.push("plain=" + isUser(new EventEmitter()));
+  } catch (e: any) {
+    results.push("plain=THROW:" + (e && e.message));
+  }
+  try {
+    results.push("cast=" + isUser(new (EventEmitter as any)()));
+  } catch (e: any) {
+    results.push("cast=THROW:" + (e && e.message));
+  }
+  const alias: any = EventEmitter;
+  try {
+    results.push("alias=" + isUser(new alias()));
+  } catch (e: any) {
+    results.push("alias=THROW:" + (e && e.message));
+  }
+  return results.join(" ");
+}

test-files/_helpers/new_cast_builtin_shadow_10589/d_emitter_fn_default.ts

@@ -0,0 +1,26 @@
+import EventEmitter from "./emitter_fn_lib.ts";
+
+function isUser(o: any): boolean {
+  return !!(o && (o as any).__mark === "user");
+}
+
+export function run(): string {
+  const results: string[] = [];
+  try {
+    results.push("plain=" + isUser(new EventEmitter()));
+  } catch (e: any) {
+    results.push("plain=THROW:" + (e && e.message));
+  }
+  try {
+    results.push("cast=" + isUser(new (EventEmitter as any)()));
+  } catch (e: any) {
+    results.push("cast=THROW:" + (e && e.message));
+  }
+  const alias: any = EventEmitter;
+  try {
+    results.push("alias=" + isUser(new alias()));
+  } catch (e: any) {
+    results.push("alias=THROW:" + (e && e.message));
+  }
+  return results.join(" ");
+}

test-files/_helpers/new_cast_builtin_shadow_10589/d_emitter_fn_named.ts

@@ -0,0 +1,26 @@
+import { EventEmitter } from "./emitter_fn_lib.ts";
+
+function isUser(o: any): boolean {
+  return !!(o && (o as any).__mark === "user");
+}
+
+export function run(): string {
+  const results: string[] = [];
+  try {
+    results.push("plain=" + isUser(new EventEmitter()));
+  } catch (e: any) {
+    results.push("plain=THROW:" + (e && e.message));
+  }
+  try {
+    results.push("cast=" + isUser(new (EventEmitter as any)()));
+  } catch (e: any) {
+    results.push("cast=THROW:" + (e && e.message));
+  }
+  const alias: any = EventEmitter;
+  try {
+    results.push("alias=" + isUser(new alias()));
+  } catch (e: any) {
+    results.push("alias=THROW:" + (e && e.message));
+  }
+  return results.join(" ");
+}

test-files/_helpers/new_cast_builtin_shadow_10589/d_headers_class_default.ts

@@ -0,0 +1,26 @@
+import Headers from "./headers_class_lib.ts";
+
+function isUser(o: any): boolean {
+  return !!(o && (o as any).__mark === "user");
+}
+
+export function run(): string {
+  const results: string[] = [];
+  try {
+    results.push("plain=" + isUser(new Headers(1)));
+  } catch (e: any) {
+    results.push("plain=THROW:" + (e && e.message));
+  }
+  try {
+    results.push("cast=" + isUser(new (Headers as any)(1)));
+  } catch (e: any) {
+    results.push("cast=THROW:" + (e && e.message));
+  }
+  const alias: any = Headers;
+  try {
+    results.push("alias=" + isUser(new alias(1)));
+  } catch (e: any) {
+    results.push("alias=THROW:" + (e && e.message));
+  }
+  return results.join(" ");
+}

test-files/_helpers/new_cast_builtin_shadow_10589/d_headers_class_named.ts

@@ -0,0 +1,26 @@
+import { Headers } from "./headers_class_lib.ts";
+
+function isUser(o: any): boolean {
+  return !!(o && (o as any).__mark === "user");
+}
+
+export function run(): string {
+  const results: string[] = [];
+  try {
+    results.push("plain=" + isUser(new Headers(1)));
+  } catch (e: any) {
+    results.push("plain=THROW:" + (e && e.message));
+  }
+  try {
+    results.push("cast=" + isUser(new (Headers as any)(1)));
+  } catch (e: any) {
+    results.push("cast=THROW:" + (e && e.message));
+  }
+  const alias: any = Headers;
+  try {
+    results.push("alias=" + isUser(new alias(1)));
+  } catch (e: any) {
+    results.push("alias=THROW:" + (e && e.message));
+  }
+  return results.join(" ");
+}

test-files/_helpers/new_cast_builtin_shadow_10589/d_headers_fn_default.ts

@@ -0,0 +1,26 @@
+import Headers from "./headers_fn_lib.ts";
+
+function isUser(o: any): boolean {
+  return !!(o && (o as any).__mark === "user");
+}
+
+export function run(): string {
+  const results: string[] = [];
+  try {
+    results.push("plain=" + isUser(new Headers(1)));
+  } catch (e: any) {
+    results.push("plain=THROW:" + (e && e.message));
+  }
+  try {
+    results.push("cast=" + isUser(new (Headers as any)(1)));
+  } catch (e: any) {
+    results.push("cast=THROW:" + (e && e.message));
+  }
+  const alias: any = Headers;
+  try {
+    results.push("alias=" + isUser(new alias(1)));
+  } catch (e: any) {
+    results.push("alias=THROW:" + (e && e.message));
+  }
+  return results.join(" ");
+}