Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 36 additions & 5 deletions crates/perry-hir/src/lower/expr_call/static_receiver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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"),
Expand Down
56 changes: 50 additions & 6 deletions crates/perry-hir/src/lower/expr_new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?
{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down
37 changes: 37 additions & 0 deletions test-files/test_issue_5912_url_new_local_shadow.ts
Original file line number Diff line number Diff line change
@@ -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")));
Loading