fix(hir): route node-core module spread calls through the variadic dispatch

[proggeramlug]

Fixes #7720.

The bug

import path from 'node:path';
const parts = ['/tmp/x', 'project.json'];
path.join('/tmp/x', 'project.json');  // "/tmp/x/project.json"
path.join(...parts);                  // TypeError [ERR_INVALID_ARG_TYPE]

Every native-module fast path in lower_call consumes its arguments
positionally, and a spread argument is lowered as a single expression
holding the whole array. path.join(...parts) therefore reached the
args.len() == 1 arm and became PathNormalize(<array>), which rejected the
array as a non-string.

It is not a path bug. The same positional fold is why, on main:

call	perry	node
path.join(...['a','b'])	TypeError [ERR_INVALID_ARG_TYPE]	a/b
util.format(...['x=%s','X'])	[ 'x=%s', 'X' ]	x=X
fs.existsSync(...['/tmp'])	false	true

The fix

When any argument is spread and the callee is a node-core module namespace
method (or a named export of one), decline the whole fast-path chain. The
fall-through tail then builds an Expr::CallSpread over the namespace member —
exactly the lowering the value-read form already takes:

const j = path.join; j(...parts);   // this already worked on main

Codegen materializes the argument array and dispatches through
js_closure_call_apply_with_spread → js_native_call_method →
dispatch_native_module_method, which is variadic by construction. It gets the
valid case right and reproduces Node's ERR_INVALID_ARG_TYPE (with the
matching code) for an invalid one.

This generalizes the per-module bail #6668 added for crypto; those two
crypto guards stay, because they also cover the bare crypto global
receiver, which is not an import and so is invisible to the new predicate.

What is deliberately out of scope

• ext/npm native modules (mysql2, redis, node-forge). Their
  NativeMethodCall rows are wired in codegen with no by-name runtime
  dispatcher behind them, so declining their fast path would trade a wrong
  answer for no answer.
• Native class statics (Buffer.concat(...list), URL.parse(...)). A
  different lowering family whose dynamic dispatch does not cover the same
  surface — Buffer.concat is already broken through the dynamic path for the
  plain B.concat(list) call, independent of spread, so routing spread onto it
  would swap a loud failure for a silent wrong answer.

Both exclusions are asserted by tests, not just described here.

Tests

crates/perry-hir/src/lower/expr_call/native_module_spread_tests.rs (6 tests,
cargo test -p perry-hir --lib, so per-PR-visible) asserts which lowering a
call got, in both directions:

• a spread call is diverted to CallSpread;
• a non-spread path.join('a','b') still lowers to PathJoin.

The second half matters: the generic dispatch is a correct fallback, so a
regression that disabled the fast path everywhere would still print the right
answer. Both halves were sabotage-checked — with the guard forced to false,
3 of the 6 fail; with the fast path removed, the other half fails.

This is also why the existing node-suite/path/join/type-errors-extra.ts
already contained path.join(...args) and stayed green through the whole life
of the bug: it only spreads segments Node rejects, so both the broken and the
correct lowering throw ERR_INVALID_ARG_TYPE. CLAUDE.md's fourth way a gate
can be unable to fail — the gate ran, its subject never did. The new
node-suite/path/join/spread.ts supplies the valid-segment cases the issue
asked for.

Byte-compared against node 26.5.1:

• test-parity/node-suite/path/join/spread.ts — namespace / namespace-import /
  require-alias / named-import / posix / win32 / sub-namespace alias /
  value-read / in-loop forms, plus mixed, trailing, single, empty and
  normalizing segment lists
• test-parity/node-suite/path/resolve/spread.ts — the reset-on-absolute sibling
• test-parity/node-suite/util/format/spread.ts

Verification

• run_parity_tests.sh --suite node-suite --module path — 94/94, 100%
  (including the two new fixtures)
• cargo test -p perry-hir --lib — green
• the issue's repro, plus util.format / fs.existsSync / os.homedir /
  fs.writeFileSync / fs/promises.readFile / crypto.createHash /
  named-import and node:console / node:process namespace forms, all now
  byte-identical to node
• non-module spread paths unchanged: Math.min(...xs), arr.push(...xs),
  console.log(...xs)

One unrelated pre-existing gap surfaced while testing and is not touched
here: Object.assign(target, ...sources) passes the sources array as one
argument ({"0":{...},"1":{...}} instead of a merge). Different family
(Object statics, not a node module) — worth its own issue.

[coderabbitai]

Caution

Review failed

An error occurred during the review process. Please try again later.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/7720-native-module-spread

---

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.}

[proggeramlug]

Merging as v0.5.1421

The best thing in this PR is the observation that the existing coverage could not fail, and I verified it before trusting it. test-parity/node-suite/path/join/type-errors-extra.ts is four lines:

for (const args of [["a", 1], ["a", null], ["a", {}], ["a", []]] as any[]) {
  try { console.log("join:", path.join(...args)); } catch (err: any) { ... }
}

Every case contains a non-string Node rejects, so the broken lowering (which rejects the whole array) and the correct one (which rejects the offending element) throw the identical ERR_INVALID_ARG_TYPE. The only pre-existing spread coverage for path.join was structurally incapable of telling working from broken — CLAUDE.md's hazard 4, in a fixture rather than a gate.

The root cause is general, and the table proves it isn't a path bug: every native-module fast path consumes arguments positionally, so a spread — lowered as one expression holding the array — folds into the args.len() == 1 arm. util.format(...['x=%s','X']) returning [ 'x=%s', 'X' ] and fs.existsSync(...['/tmp']) returning false are the same defect wearing different clothes.

Declining the fast path rather than teaching each one about spread is the right shape. The fall-through already builds Expr::CallSpread over the namespace member, which is exactly what the value-read form (const j = path.join; j(...parts)) has always taken and which already worked. It dispatches through js_native_call_method → dispatch_native_module_method, variadic by construction — so this reuses a working path instead of adding a parallel one, and it reproduces Node's error (with matching code) on the invalid case rather than only fixing the happy path.

The out-of-scope section is the part I'd want kept. Both exclusions are argued from what the alternative would cost, not from effort:

• ext/npm modules — their NativeMethodCall rows have no by-name runtime dispatcher behind them, so declining would trade a wrong answer for no answer.
• Native class statics — Buffer.concat(list) is already broken through the dynamic path independent of spread, so routing spread onto it would swap a loud failure for a silent wrong one.

The two #6668 crypto guards are correctly retained (verified at native_module.rs:340): they also cover the bare crypto global receiver, which is not an import and so is invisible to the new predicate.

Verification

cargo test -p perry-hir --lib: 287 passed, 0 failed. run_parity_tests.sh --suite node-suite --module path: 94/94.

Full cargo test -p perry-runtime --lib shows 3 failures, byte-identical to the set already red on main (generator_attach_prototype::*) — not from this PR, and being fixed separately. Lint 19/19.

The Object.assign(target, ...sources) finding is correctly left alone and deserves its own issue — different family, and folding it in would have made this diff unreviewable.

[proggeramlug]

Marking draft: a same-host A/B (this tree vs. the same tree with the guard forced to false) turned up a regression I introduced — fs.promises.readFile(...args) returns ENOENT on main (i.e. the call really happens) but throws TypeError: value is not a function on this branch.

Cause: the predicate recurses through any sub-namespace receiver, but only some sub-namespaces have a runtime dispatch bucket (nm_module_index has path.posix, path.win32, util.types, crypto.subtle, punycode.ucs2 — there is no fs.promises bucket). Diverting a bucket-less sub-namespace to the dynamic path replaces a working call with a hard throw.

Running a 32-case A/B matrix over the node-core spread surface (both arms + node oracle) to set that list from measurement rather than guesswork; will push the narrowed predicate and re-request review.