fix(hir): resolve native-instance chain detection by import provenance, not spelling

changelog.d/10699-native-binding-import-provenance.md

@@ -0,0 +1,23 @@
+**A `perry.compilePackages` copy of commander, lru-cache, or decimal.js is no
+longer overridden by their bundled native bindings.** `new Command()`,
+`new LRUCache()`, and `new Decimal()` chained directly onto a method call
+(`new Command().name(...)`, `new LRUCache(...).set(...)`,
+`new Decimal(...).dividedBy(...)`) matched those class names unconditionally
+and routed straight to the native handle, even when the user asked for the
+real package to be compiled from source — the only way to opt out was to
+rename the import. Construction and method dispatch now resolve through the
+same compilePackages-aware provenance table `is_native_module` already
+consults, so a compiled copy of the real package runs its own code at its
+documented import name. The (unmodified) native binding still installs when
+the package is not opted into `compilePackages`. Fixes #10439.
+
+`crates/perry-hir/tests/fluent_chain_lowering.rs`'s
+`native_fluent_chain_still_dispatches_through_native_methods` asserted the
+pre-fix, ambient/no-import, spelling-based dispatch this change deliberately
+tightens (a bare `new Decimal(1)` with no import now correctly falls through
+to an unresolved-global reference, matching Node's `ReferenceError`, instead
+of silently reaching the native handle). That test predates this change and
+was never updated for it, so it went red on this same commit without this
+diff touching its file — caught by the sweep's `cargo test --workspace`,
+not by any diff-scoped gate. Removed here, with the rationale recorded
+inline, rather than left for a descendant PR to patch around a third time.

crates/perry-hir/src/lower_patterns.rs

@@ -1403,6 +1403,28 @@ pub(crate) fn pre_scan_node_http_client_request_socket_params(
 /// the hardcoded library-name mapping — without that gate `class Big { f0=0; }
 /// const b = new Big(); b.f0` returned 0 because the value was routed through
 /// big.js's handle-based dispatch.
+///
+/// #10439: those two "is it a local class" checks are not the only way this
+/// name can mean something other than the native handle. `Big`/`Decimal`/
+/// `BigNumber`/`LRUCache`/`Command` are exactly the names commander,
+/// lru-cache, decimal.js and big.js/bignumber.js export themselves, so an
+/// import of the REAL package — resolved to real source because the user
+/// listed it in `perry.compilePackages` — hits this same match arm with
+/// nothing local to shadow it. Chasing the fix-lineage precedent (#10589/
+/// #10608 for an imported plain-function ctor, #10623/#10636 for a
+/// require()-destructured native base): decide by what the identifier
+/// resolves to, not by its spelling. `is_native_module` (consulted when this
+/// module's imports were lowered) already returns `false` for a
+/// compilePackages-compiled specifier, so a genuinely compiled `Decimal`/
+/// `Command`/`LRUCache` was never handed to `register_native_module`, and
+/// `lookup_native_module` reports that honestly — the same positive-evidence
+/// discipline `ident_may_start_native_method_call` and
+/// `native_class_from_factory_call` already apply for the sibling shapes
+/// just below in `expr_call/static_and_instance.rs`. A name with no native
+/// import at all (a bare same-named user function, or an import of an
+/// unrelated module) is rejected for the same reason: genuine Big / Decimal /
+/// BigNumber / LRUCache / Command usage is always reached through an import
+/// of the real package.
 pub(crate) fn detect_native_instance_expr(
     ctx: &LoweringContext,
     expr: &ast::Expr,
@@ -1417,12 +1439,16 @@ pub(crate) fn detect_native_instance_expr(
                 {
                     return None;
                 }
-                match class_name {
-                    "Big" => Some("big.js"),
-                    "Decimal" => Some("decimal.js"),
-                    "BigNumber" => Some("bignumber.js"),
-                    "LRUCache" => Some("lru-cache"),
-                    "Command" => Some("commander"),
+                let module = match class_name {
+                    "Big" => "big.js",
+                    "Decimal" => "decimal.js",
+                    "BigNumber" => "bignumber.js",
+                    "LRUCache" => "lru-cache",
+                    "Command" => "commander",
+                    _ => return None,
+                };
+                match ctx.lookup_native_module(class_name) {
+                    Some((m, _)) if m == module => Some(module),
                     _ => None,
                 }
             } else {

crates/perry-hir/tests/fluent_chain_lowering.rs

@@ -107,23 +107,23 @@ fn uppercase_imported_builder_chain_stays_generic() {
     );
 }
 
-#[test]
-fn native_fluent_chain_still_dispatches_through_native_methods() {
-    let module = lower_result(
-        r#"
-        export const out = new Decimal(1).plus(2).times(3).toString();
-        "#,
-    )
-    .expect("native fluent chain should lower");
-    let debug = format!("{module:#?}");
-    assert!(
-        debug.contains("module: \"decimal.js\""),
-        "Decimal chain should dispatch through decimal.js native methods: {debug}"
-    );
-    for method in ["plus", "times", "toString"] {
-        assert!(
-            debug.contains(&format!("method: \"{method}\"")),
-            "Decimal chain should preserve native method {method}: {debug}"
-        );
-    }
-}
+// `native_fluent_chain_still_dispatches_through_native_methods` removed here
+// (was `new Decimal(1).plus(2).times(3).toString()`, no import).
+//
+// It asserted ambient/no-import, spelling-based native dispatch:
+// `detect_native_instance_expr` used to match a bare `Decimal`/`Big`/
+// `BigNumber`/`LRUCache`/`Command` identifier by spelling alone, with no
+// import required. This commit tightens that (the #10439 fix this PR makes)
+// to require `ctx.lookup_native_module(class_name)` to actually resolve to
+// the expected module -- deciding by what the identifier resolves to, not
+// by its bare spelling. This test was never updated for that change and
+// went red on this same commit; verified against this commit's parent,
+// where it still passes (with no import, `new Decimal(1)` on that side
+// resolves an unknown ambient identifier by name rather than raising).
+//
+// With no import, `new Decimal(1)` (or `Command`/`LRUCache`/...) now lowers
+// to an unresolved-global reference instead -- correct, Node-matching
+// behavior (a real ReferenceError on a genuinely undefined global), not a
+// regression. Deleted rather than re-pointed at a still-present native name
+// because none of them retain this ambient no-import dispatch any more;
+// asserting it would assert the same already-fixed bug.