fix(hir): honor scope-local class renames in instanceof

[proggeramlug]

Summary

When two classes share a name in one compilation — e.g. a bundler (webpack/turbopack) flattens two module factories that each declare class l — the second is registered under a suffixed name (l$0), and class_renames maps the raw name to it. extends, new, and static reads already resolve through that map, but the instanceof right-hand side used the raw identifier. So x instanceof l inside the second factory resolved to the other factory's same-named class, mismatched the class_id, and returned false — even though the runtime prototype chain was correct.

Resolve the instanceof class-name operand through resolve_class_name, exactly like the other class references.

Why it matters

Auth.js v5 bundled into one chunk: its AuthError collided with another same-named minified class and was renamed. error instanceof AuthError then returned false, so a CredentialsSignin was mis-wrapped in a CallbackRouteError and the login callback redirected to ?error=Configuration instead of ?error=CredentialsSignin.

Testing

test_gap_instanceof_flattened_class_rename.ts (byte-compared to Node): two "module factory" closures each declare class L; x instanceof L inside the second must resolve to its own L (a subclass instance is instanceof its own base, not the other factory's L).

https://claude.ai/code/session_01NjymZygYh31wM9h3wLnUqv

Summary by CodeRabbit

• Bug Fixes

  • Fixed instanceof checks for same-named classes defined in different module scopes.
  • Ensured class checks resolve to the correct renamed class after bundling and flattening.

• Tests

  • Added regression coverage for instanceof behavior across colliding class names.

[coderabbitai]

📝 Walkthrough

Walkthrough

instanceof lowering now resolves identifier-based class names through scope-aware class renaming. A regression test covers same-named classes from flattened module factories and verifies that checks bind to the correct renamed class.

Changes

Instanceof class resolution

Layer / File(s)	Summary
Resolve renamed instanceof classes crates/perry-hir/src/lower/lower_expr/arm_bin.rs, test-files/test_gap_instanceof_flattened_class_rename.ts	Identifier RHS values use ctx.resolve_class_name(...), with regression coverage for colliding class names across module factories.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• PerryTS/perry#5408: Addresses class identity and renaming collisions during lowering.
• PerryTS/perry#6270: Also modifies instanceof lowering in arm_bin.rs.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: resolving scope-local class renames in instanceof lowering.
Description check	✅ Passed	The description covers summary and testing well, but it omits the template's Changes and Related issue sections.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/perry-hir/src/lower/lower_expr/arm_bin.rs (1)

55-63: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Enforce consistent shadowing guard for built-in constructors.

The current check only verifies that the built-in is not shadowed by a non-class binding (!ctx.shadows_unqualified_global). If a user defines class WeakRef {}, it will bypass this check and incorrectly take the built-in fast path, causing instanceof to return a boolean instead of evaluating normally.

Based on learnings, you must check shadowing at both class binding sites (ctx.classes_index) and non-class binding sites, and use builtin_constructor_inference_name to safely determine if the name refers to a built-in.

🐛 Proposed fix for the built-in shadowing guard

-            if (class_name == "WeakRef" || class_name == "FinalizationRegistry")
-                && !ctx.shadows_unqualified_global(class_name)
+            let effective_name = crate::lower_types::builtin_constructor_inference_name(class_name);
+            if (effective_name == "WeakRef" || effective_name == "FinalizationRegistry")
+                && !ctx.shadows_unqualified_global(class_name)
+                && !ctx.classes_index.contains_key(class_name)
             {
                 if let ast::Expr::Ident(left_ident) = bin.left.as_ref() {
                     let local_name = left_ident.sym.to_string();
-                    let is_match = (class_name == "WeakRef"
+                    let is_match = (effective_name == "WeakRef"
                         && ctx.weakref_locals.contains(&local_name))
-                        || (class_name == "FinalizationRegistry"
+                        || (effective_name == "FinalizationRegistry"
                             && ctx.finreg_locals.contains(&local_name));

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-hir/src/lower/lower_expr/arm_bin.rs` around lines 55 - 63,
Update the built-in constructor guard in the arm_bin lowering logic to reject
names shadowed by either class bindings in ctx.classes_index or non-class
bindings via ctx.shadows_unqualified_global. Use
builtin_constructor_inference_name to determine the canonical built-in name
before applying the WeakRef and FinalizationRegistry local checks, preserving
normal instanceof evaluation for user-defined classes.

Source: Learnings

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/perry-hir/src/lower/lower_expr/arm_bin.rs`:
- Around line 55-63: Update the built-in constructor guard in the arm_bin
lowering logic to reject names shadowed by either class bindings in
ctx.classes_index or non-class bindings via ctx.shadows_unqualified_global. Use
builtin_constructor_inference_name to determine the canonical built-in name
before applying the WeakRef and FinalizationRegistry local checks, preserving
normal instanceof evaluation for user-defined classes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a777b70e-15e1-4117-8a4b-f239f8b06e30

📥 Commits

Reviewing files that changed from the base of the PR and between cb03eb9 and 2d4e3b0.

📒 Files selected for processing (2)

• crates/perry-hir/src/lower/lower_expr/arm_bin.rs
• test-files/test_gap_instanceof_flattened_class_rename.ts