fix(hir): #5912 — new URL()/TextEncoder()/etc. ignore lexical shadowing (#5913)

* fix(hir): #5912 — new URL()/TextEncoder()/etc. ignore lexical shadowing

`new URL(...)` (and URLSearchParams/URLPattern/TextEncoder/TextDecoder)
were dispatched by bare identifier name with no check for whether the
name actually resolves to the global constructor or is shadowed by a
local function/class — unlike sibling well-known constructors
(Function, Object) in the same match, which already guard on
lookup_local/lookup_func/lookup_class.

Real packages ship their own tolerant polyfills under these names
(found via @mixmark-io/domino's lib/URL.js, which defines
`function URL(url) { ... }` and calls `new URL()` with zero args
against its own constructor) and hit perry's native WHATWG URL
constructor instead, which requires at least one argument.

The same unguarded name match also existed in a second, independent
spot: `static_receiver_class` (used to decide whether `.toString()` /
`.toJSON()` / `JSON.stringify()` on a receiver should route through
the native Date/URL fast paths) classified any `new URL(...)` — or a
local typed as `URL` — as the native type by name alone, so even after
fixing the construction site, printing/serializing a shadowed
instance still silently substituted native URL output.

Fixes both sites by checking `lookup_local`/`lookup_func`/
`lookup_class` before applying the native lowering, matching the
existing shadowing-guard pattern used elsewhere in expr_new.rs.

* fix(hir): #5912 CodeRabbit follow-up — globalThis escape hatch, alias, imported bindings

Three issues from CodeRabbit's review of the initial fix:

1. Real regression: the static_receiver_class shadowing guard collapsed
   `new globalThis.URL(...)` and bare `new URL(...)` into the same
   class_name capture, so a locally-shadowed URL made even the explicit
   globalThis-qualified form misclassify as "Object" and lose the
   native fast path. Track which callee shape matched and only apply
   the shadow check to the bare-identifier form.

2. Coverage gap: `const MyURL = URL; new MyURL()` bypassed the
   shadowing guard entirely via the resolve_class_alias branch, which
   runs before the later callee_local_at_entry/lookup_func/lookup_class
   checks. Added the same shadowing check there, against the
   ALIAS-RESOLVED name (aliases are name-keyed, not scope-aware).

3. Coverage gap: an imported binding shadowing one of these names
   (`import { URL } from "./polyfill"`) wasn't covered by the
   lookup_local/lookup_func/lookup_class triplet. Added
   lookup_imported_func alongside it in expr_new.rs (module-level
   registry, safe to query fresh — unlike lookup_local, this one isn't
   subject to the scope-stack-disturbance issue callee_local_at_entry
   exists to avoid, so no special snapshotting needed).

static_receiver.rs's own guard now calls the existing
`shadows_unqualified_global` helper (already covers all four cases)
instead of reimplementing three of them by hand.

Extended the regression test with both the globalThis-escape-hatch and
alias cases; both match node byte-for-byte.

---------

Co-authored-by: Ralph <ralph@skelpo.com>

Author: proggeramlug

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"),

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",

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")));