fix(runtime): recover string.length mis-lowered to a 0-arg method call

[proggeramlug]

Problem

A bare member read str.length can reach js_native_call_method as a 0-arg native method call: member reads on a native-instance-classified receiver are lowered to a 0-arg NativeMethodCall so FFI getters dispatch (see the existing per-module guards in expr_member.rs). When the HIR mis-classifies the receiver — a stale/aliased native-instance class tag, the same shape already documented in this file for the closure-captured array registered as FormData — and the receiver's runtime type is actually a string, "length" has no callable method, so the dispatcher threw:

TypeError: (string).length is not a function

This aborts code paths that read .length per element, e.g. an inlined string-width/wrap-ansi text-measurement loop (H += chunk.length over [...str]).

Fix

A string's length is a data property, never a method. In the primitive-receiver branch of the dispatcher, when a string method-call misses and the "method" is length with no args, return the string's UTF-16 length instead of throwing — matching what the mis-lowered property access intended.

Native classes with a real FFI length getter (e.g. cheerio selections, js_cheerio_selection_length) are objects, not primitives, and dispatch through their own arm above, so they are unaffected.

Tests

primitive_dataprop_recovery_tests::string_length_method_miss_returns_length — dispatching length (0 args) on a string receiver returns its UTF-16 length. Full perry-runtime + perry-codegen suites pass (1081 + 0 failures).

https://claude.ai/code/session_01EUJtfcsGNWpqrGiufHXHxt

Summary by CodeRabbit

• Bug Fixes

  • Fixed an edge case in dynamic method dispatch for primitive strings where accessing length could incorrectly throw an error. It now correctly returns the UTF-16 code unit length (e.g., "helloé" → 6).

• Tests

  • Added a regression test validating the nullsafe dynamic dispatch path for length, ensuring the expected numeric result is returned.

[coderabbitai]

📝 Walkthrough

Walkthrough

js_native_call_method_nullsafe now returns a string receiver’s UTF-16 length for a zero-argument length dispatch instead of following the generic failure path. A regression test covers the mis-dispatched bare property read case.

Changes

String length recovery

Layer / File(s)	Summary
Dispatch recovery and regression crates/perry-runtime/src/object/native_call_method.rs	The nullsafe native call path returns a primitive string’s UTF-16 length for length with zero args, the plain entrypoint comment clarifies that behavior, and a regression test checks helloé returns 6.0.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I hopped through dispatch with a tiny gleam,
A string said “length,” and got its true dream.
No throw, just a count from the header’s embrace,
Six little code units found their place.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the runtime fix for mis-lowered string.length calls.
Description check	✅ Passed	The description covers the problem, fix, and tests well enough, even though it doesn't follow the template headings exactly.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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]

Actionable comments posted: 1

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

Inline comments:
In `@crates/perry-runtime/src/object/native_call_method.rs`:
- Around line 1293-1298: The zero-arg string length special case in
js_native_call_method is treating real method calls as property reads, so
x.length() incorrectly succeeds instead of throwing TypeError. Update the
lowering/disptach flow around native_call_method and js_native_call_method to
carry a distinct signal for a lowered property read versus an actual call, and
only return the string header utf16_len fast path when that read signal is
present; otherwise let genuine string.length() calls follow the normal callable
path and error behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4ce718b5-e254-4f56-8b6e-19c629d48930

📥 Commits

Reviewing files that changed from the base of the PR and between 9fb1940 and 5b8ef28.

📒 Files selected for processing (1)

• crates/perry-runtime/src/object/native_call_method.rs

[proggeramlug]

Addressed the CodeRabbit finding (genuine ("abc" as any).length() must still throw): the recovery is now gated to js_native_call_method_nullsafe only, which codegen emits exclusively for the native-instance member-access fallback (lower_call/native/native_instance_branch.rs) — i.e. the bare property-read path where the mis-lowered .length lands. A genuine non-optional x.length() call lowers to the plain js_native_call_method entrypoint, which is unchanged and keeps throwing the spec-required TypeError. That's the read-vs-call signal: the entrypoint itself.

[coderabbitai]

🧹 Nitpick comments (1)

crates/perry-runtime/src/object/native_call_method.rs (1)

1494-1511: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test covers only the positive recovery path.

Consider adding a negative assertion that the plain js_native_call_method entrypoint still throws for a 0-arg length on a string receiver — that's the invariant the whole "scoped to nullsafe" design hinges on, and it's currently unverified by tests. If the FFI throw aborts rather than panics (making it untestable in-process), a nullsafe test with args_len > 0 asserting non-recovery would still guard the boundary.

🤖 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-runtime/src/object/native_call_method.rs` around lines 1494 -
1511, The new nullsafe length test only checks the recovery path in
js_native_call_method_nullsafe; add a negative test around js_native_call_method
to verify a string receiver with 0 args still throws, or if that FFI path can’t
be asserted in-process, add a nullsafe case with args_len > 0 that confirms the
length member-read is not recovered. Use the existing js_native_call_method and
js_native_call_method_nullsafe symbols so the scoped-to-nullsafe boundary is
explicitly covered.

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

Nitpick comments:
In `@crates/perry-runtime/src/object/native_call_method.rs`:
- Around line 1494-1511: The new nullsafe length test only checks the recovery
path in js_native_call_method_nullsafe; add a negative test around
js_native_call_method to verify a string receiver with 0 args still throws, or
if that FFI path can’t be asserted in-process, add a nullsafe case with args_len
> 0 that confirms the length member-read is not recovered. Use the existing
js_native_call_method and js_native_call_method_nullsafe symbols so the
scoped-to-nullsafe boundary is explicitly covered.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5bd5148a-6129-4e59-ac32-116779a4d905

📥 Commits

Reviewing files that changed from the base of the PR and between 5b8ef28 and 48c1fad.

📒 Files selected for processing (1)

• crates/perry-runtime/src/object/native_call_method.rs