diff --git a/crates/perry-hir/src/lower/expr_call/static_receiver.rs b/crates/perry-hir/src/lower/expr_call/static_receiver.rs index cbcd310d2..76abdffcf 100644 --- a/crates/perry-hir/src/lower/expr_call/static_receiver.rs +++ b/crates/perry-hir/src/lower/expr_call/static_receiver.rs @@ -50,20 +50,43 @@ pub(super) fn static_receiver_class( } } if let ast::Expr::New(new_expr) = obj { - let class_name = match new_expr.callee.as_ref() { - ast::Expr::Ident(ident) => Some(ident.sym.as_ref()), + // Issue #5912 (CodeRabbit follow-up): `new globalThis.URL(...)` + // always reaches the REAL global regardless of any local `URL` + // shadowing — that's the entire point of the explicit `globalThis.` + // qualifier. Track which callee shape matched instead of collapsing + // both into one `class_name` capture; the original version fed both + // forms into the same shadow check below, incorrectly downgrading + // `new globalThis.URL()` to "Object" whenever a local `URL` shadowed + // the bare name. + let (class_name, is_global_qualified) = match new_expr.callee.as_ref() { + ast::Expr::Ident(ident) => (Some(ident.sym.as_ref()), false), ast::Expr::Member(member) if matches!(member.obj.as_ref(), ast::Expr::Ident(obj) if obj.sym.as_ref() == "globalThis") && ctx.lookup_local("globalThis").is_none() => { match &member.prop { - ast::MemberProp::Ident(prop) => Some(prop.sym.as_ref()), - _ => None, + ast::MemberProp::Ident(prop) => (Some(prop.sym.as_ref()), true), + _ => (None, false), } } - _ => None, + _ => (None, false), }; if let Some(class_name) = class_name { + // Issue #5912: a local function/const/class/imported-binding + // (`shadows_unqualified_global` covers all four — see #5912 + // review follow-up) shadowing one of the well-known names below + // (e.g. a vendored `function URL(url) { ... }` polyfill) is the + // user's own value, never perry's native built-in — classify as + // a generic "Object" (skips the ambiguous Date/URL method arms, + // same treatment as an object-literal receiver below) instead of + // misrouting `.toString()`/`.toJSON()` through the native fast + // paths (`UrlInstanceToJSON` etc.) on a value that was never + // actually constructed via the native path. Doesn't apply to the + // `globalThis.X` form above — see the comment on + // `is_global_qualified`. + if !is_global_qualified && ctx.shadows_unqualified_global(class_name) { + return Some("Object"); + } let resolved_class = ctx .resolve_class_alias(class_name) .unwrap_or_else(|| class_name.to_string()); @@ -139,6 +162,14 @@ pub(super) fn static_receiver_class( _ => None, }; if let Some(n) = named { + // Issue #5912: the local's inferred type name can legitimately + // be "URL" because it holds an instance of a REAL user class + // named `URL` (shadowing the global) — not perry's native + // WHATWG URL. Route those through generic dispatch too, same + // as the `New`-expression branch above. + if ctx.lookup_class(n).is_some() { + return Some("Object"); + } return match n { "Date" => Some("Date"), "URL" => Some("URL"), diff --git a/crates/perry-hir/src/lower/expr_new.rs b/crates/perry-hir/src/lower/expr_new.rs index b30fb587b..0a98ea01c 100644 --- a/crates/perry-hir/src/lower/expr_new.rs +++ b/crates/perry-hir/src/lower/expr_new.rs @@ -416,8 +416,16 @@ pub(super) fn lower_new(ctx: &mut LoweringContext, new_expr: &ast::NewExpr) -> R } } + // Issue #5912 (CodeRabbit follow-up): an alias like + // `const MyURL = URL; new MyURL()` must not bind to the native + // constructor either when the ALIASED name is itself shadowed + // (`function URL(url) {...}` in scope) — `resolve_class_alias` + // is name-keyed and not scope-aware, so it happily maps + // `MyURL` -> `"URL"` without knowing `URL` was ever shadowed. if let Some(resolved) = ctx.resolve_class_alias(&class_name) { - if is_url_encoding_constructor_name(&resolved) { + if is_url_encoding_constructor_name(&resolved) + && !ctx.shadows_unqualified_global(&resolved) + { if let Some(expr) = lower_url_encoding_constructor(ctx, &resolved, new_expr.args.as_deref())? { @@ -941,15 +949,41 @@ pub(super) fn lower_new(ctx: &mut LoweringContext, new_expr: &ast::NewExpr) -> R } } - // Handle URL class - if class_name == "URL" { + // Handle URL class. #5912: gated on `callee_local_at_entry` / + // `lookup_func` / `lookup_imported_func` so a local function/ + // const/imported-binding shadowing the global name (e.g. a + // vendored `function URL(url?) {...}` polyfill, or `import { + // URL } from "./my-url-polyfill"`) routes through the generic + // local-dispatch fallback below instead of always binding to + // perry's native WHATWG URL constructor — matches the + // `lookup_local`/`lookup_func`/`lookup_class` shadowing guard + // used for `Function`/`Object` above (a named function + // declaration is tracked via `lookup_func`, not + // `lookup_local`/`callee_local_at_entry`). Deliberately doesn't + // use the `shadows_unqualified_global` one-liner here: that + // helper's `lookup_local` call is a FRESH scope lookup, but + // `callee_local_at_entry` must stay a pre-captured snapshot (see + // the comment above its definition) — the Error-type branch + // just above already lowers `new_expr.args`, which can disturb + // the locals scope stack before we get here. + if class_name == "URL" + && callee_local_at_entry.is_none() + && ctx.lookup_func(&class_name).is_none() + && ctx.lookup_imported_func(&class_name).is_none() + && ctx.lookup_class(&class_name).is_none() + { return Ok( lower_url_encoding_constructor(ctx, "URL", new_expr.args.as_deref())?.unwrap(), ); } // Handle URLSearchParams / URLPattern classes - if matches!(class_name.as_str(), "URLSearchParams" | "URLPattern") { + if matches!(class_name.as_str(), "URLSearchParams" | "URLPattern") + && callee_local_at_entry.is_none() + && ctx.lookup_func(&class_name).is_none() + && ctx.lookup_imported_func(&class_name).is_none() + && ctx.lookup_class(&class_name).is_none() + { return Ok(lower_url_encoding_constructor( ctx, &class_name, @@ -993,7 +1027,12 @@ pub(super) fn lower_new(ctx: &mut LoweringContext, new_expr: &ast::NewExpr) -> R return Ok(Expr::FinalizationRegistryNew(Box::new(cb))); } // Handle TextEncoder constructor - if class_name == "TextEncoder" { + if class_name == "TextEncoder" + && callee_local_at_entry.is_none() + && ctx.lookup_func(&class_name).is_none() + && ctx.lookup_imported_func(&class_name).is_none() + && ctx.lookup_class(&class_name).is_none() + { return Ok(lower_url_encoding_constructor( ctx, "TextEncoder", @@ -1002,7 +1041,12 @@ pub(super) fn lower_new(ctx: &mut LoweringContext, new_expr: &ast::NewExpr) -> R .unwrap()); } // Handle TextDecoder constructor: new TextDecoder(label?, opts?) - if class_name == "TextDecoder" { + if class_name == "TextDecoder" + && callee_local_at_entry.is_none() + && ctx.lookup_func(&class_name).is_none() + && ctx.lookup_imported_func(&class_name).is_none() + && ctx.lookup_class(&class_name).is_none() + { return Ok(lower_url_encoding_constructor( ctx, "TextDecoder", diff --git a/test-files/test_issue_5912_url_new_local_shadow.ts b/test-files/test_issue_5912_url_new_local_shadow.ts new file mode 100644 index 000000000..171774628 --- /dev/null +++ b/test-files/test_issue_5912_url_new_local_shadow.ts @@ -0,0 +1,37 @@ +// Issue #5912 — `new URL(...)` (and URLSearchParams/URLPattern/TextEncoder/ +// TextDecoder) were dispatched by bare identifier name with no check for +// whether the name is actually the global constructor or shadowed by a +// local function/class. Real packages ship their own tolerant `URL` +// polyfill (e.g. @mixmark-io/domino's lib/URL.js calls `new URL()` with +// zero args against ITS OWN constructor) and hit perry's native URL +// constructor instead, which requires at least one argument. +// +// This exercises the exact shadowing shape: a local function named `URL` +// that tolerates a missing argument, matching Node's output. + +function URL(url?: string) { + return { url: url ?? "default", kind: "local" }; +} + +console.log(JSON.stringify(new URL())); +console.log(JSON.stringify(new URL("explicit"))); + +function withTextEncoder() { + function TextEncoder(label?: string) { + return { label: label ?? "utf-8", kind: "local" }; + } + return new TextEncoder(); +} + +console.log(JSON.stringify(withTextEncoder())); + +// CodeRabbit follow-up on the #5913 PR — an explicit `globalThis.` qualifier +// is an escape hatch to the REAL global and must keep working even while the +// bare `URL` identifier is shadowed above. +console.log(new (globalThis as any).URL("https://example.com/path").hostname); + +// CodeRabbit follow-up — a local alias of the shadowed name must not resolve +// back to the native constructor either (`resolve_class_alias` is name-keyed, +// not scope-aware, so this needs its own explicit check). +const MyURL = URL; +console.log(JSON.stringify(new MyURL("via-alias")));