fix(hir): #5195 — inline property-access .indexOf/.includes miscompiled as array search

[proggeramlug]

Fixes #5195.

Problem

Calling .indexOf / .includes inline on a property access —
arr[i].id.indexOf("PRO"), p.id.includes("PRO") — returned the wrong result
with no error. Extracting the property to a local (const pid = arr[i].id; pid.indexOf(...)) worked.

Reproduced on the native host (v0.5.1191, current main):

interface P { id: string }
const arr: P[] = [{ id: "GSC_PRO_MONTHLY" }, { id: "GSC_AGENCY_MONTHLY" }, { id: "GSC_PRO_ANNUAL" }]
let a = 0
for (let i = 0; i < arr.length; i++) if (arr[i].id.indexOf("PRO") >= 0) a++
console.log(a)  // printed 0 ❌ (expected 2)
console.log(arr.filter(p => p.id.indexOf("PRO") >= 0).length)  // printed 0 ❌ (expected 2)

Downstream this silently made a StoreKit product lookup
(products.filter(p => p.id.indexOf("PRO") >= 0)) return empty, so an
in-app-purchase button reported the plan as unavailable.

Root cause

crates/perry-hir/src/lower/expr_call/array_only_methods.rs folded a
PropertyGet receiver of .indexOf / .includes to Expr::ArrayIndexOf /
Expr::ArrayIncludes (an element-equality search), gated only by a hardcoded
property-name allowlist (stack|message|name|sourceSQL|expandedSQL). A string
property like id isn't on that list, so arr[i].id (typed string) was
searched as an array → -1. HIR confirmed it:

ArrayIndexOf { array: PropertyGet { object: IndexGet { LocalGet(arr), 0 }, property: "id" }, value: String("PRO") }

Fix

Only fold receivers that are provably array-producing (.map/.filter/
.sort/.slice/.split/Object.keys/Object.values/array literals/
Array.from). Every property access now falls through to the generic
js_native_call_method dispatch, which checks the runtime value type and
routes string- vs. array-indexOf/includes correctly — the same way the
sibling slice arm already behaves and the same path a known-string local
already took. This removes the unsustainable property-name allowlist band-aid
in both arms.

Genuine array-typed properties keep correct semantics via that dynamic
dispatch (verified below).

Verification

New regression test crates/perry/tests/issue_5195_property_method_chain.rs (2 cases, both green):

• string_property_indexof_includes_inline_match — the issue shape now returns 2 for the loop, .filter, and .includes forms, and arr[0].id.toLowerCase().indexOf("pro") === 4.
• array_typed_property_indexof_includes_still_search_array — box.tags.indexOf("green") === 1, .includes("blue") === true, miss === -1, numeric array property === 1.

Also ran a broader behavioral sweep (bare-local / literal / .map/.filter/.split/Object.keys chains, string locals, string & array properties) — all correct. perry-hir unit tests and the related issue_5271_string_method_nonstring_receiver / issue_5139_object_arraylike_method_dispatch integration tests pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Corrected the behavior of indexOf and includes methods when called on array properties accessed within method chains. Previously, these methods could incorrectly apply string semantics. Now the implementation properly distinguishes between array and string operations, ensuring these search methods return accurate results based on the actual data type being operated upon.

[coderabbitai]

📝 Walkthrough

Walkthrough

The indexOf and includes arms in try_array_only_methods replace a hardcoded string-property-name exclusion list with a conservative check that only folds into ArrayIndexOf/ArrayIncludes when the lowered receiver expression is provably array-producing. A new regression test file compiles and runs TypeScript programs to verify both string and array property dispatch.

Fix #5195: property-access receiver folding for indexOf/includes

Layer / File(s)	Summary
indexOf/includes receiver guard: allowlist → provably-array check crates/perry-hir/src/lower/expr_call/array_only_methods.rs	Replaces the old string-property-name exclusion gate in both the indexOf (lines 912–937) and includes (lines 954–973) arms with a match on the shape of the lowered receiver Expr. Only receivers matching a whitelist of array-producing Expr variants (Array, array-map/filter/sort/slice, ObjectKeys/Values, StringSplit) fold into ArrayIndexOf/ArrayIncludes; all others fall through to generic runtime dispatch.
Regression tests: string and array property dispatch crates/perry/tests/issue_5195_property_method_chain.rs	Adds module docs, perry_bin() and compile_and_run() helpers, and two #[test] cases. The first asserts inline arr[i].id.indexOf(...) / includes(...) and .filter(p => p.id.indexOf(...)) produce correct string-search counts. The second asserts string[] and numeric array typed properties continue to use array indexOf/includes semantics (element index and boolean membership).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• PerryTS/perry#5194: Also modifies try_array_only_methods in the same file to tighten receiver-based folding guards, specifically for mutating array methods on unknown/any receivers.
• PerryTS/perry#5386: Changes try_array_only_methods folding by tightening receiver classification, including sort and Ident receiver gating in the same code path.

Poem

🐇 A rabbit hops through property chains,
No more wrong indexOf pains!
The allowlist shed, a shape-check instead,
Array or string — the right path explained.
Regression tests run, the bug now slain! 🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main bug fix: inline property-access method calls like .indexOf/.includes were miscompiled as array searches.
Description check	✅ Passed	The description is comprehensive, covering Problem, Root Cause, Fix, and Verification sections. It follows the template structure with Summary and Changes, though test plan checklist is not explicitly marked.
Linked Issues check	✅ Passed	The PR directly addresses issue #5195 by fixing the exact bug described: inline property-access method chaining now correctly dispatches based on runtime value type instead of incorrectly folding to array search.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing issue #5195: the HIR lowering logic and new regression test. No unrelated modifications detected.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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/5195-property-method-chain-miscompile

---

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

[coderabbitai]

🧹 Nitpick comments (1)

crates/perry-hir/src/lower/expr_call/array_only_methods.rs (1)

925-937: 💤 Low value

Consider unifying the provably-array whitelist with slice/join arms for consistency.

The whitelist here is narrower than the slice arm (lines 815-841) and join arm (lines 858-881), which also include: ArraySpread, ArrayFromMapped, ArrayFlat, ArrayToReversed, ArrayToSorted, ArrayToSpliced, ArrayWith, ArrayEntries, ArrayKeys, ArrayValues, ObjectEntries, ProcessArgv.

Missing shapes correctly fall through to generic dispatch (no correctness issue), but unified whitelists would reduce maintenance burden when adding new array-producing variants.

Also applies to: 961-973

🤖 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/expr_call/array_only_methods.rs` around lines 925
- 937, The matches! check for provably-array expressions at lines 925-937 is
narrower than similar checks in the slice arm (lines 815-841) and join arm
(lines 858-881), missing variants like ArraySpread, ArrayFromMapped, ArrayFlat,
ArrayToReversed, ArrayToSorted, ArrayToSpliced, ArrayWith, ArrayEntries,
ArrayKeys, ArrayValues, ObjectEntries, and ProcessArgv. To fix this, expand the
matches! pattern at lines 925-937 to include all the array-producing variants
present in the slice and join arms, and apply the same unification to the other
location mentioned (lines 961-973). This will ensure consistency across all
three whitelists and reduce future maintenance burden when new array-producing
expression types are added.

🤖 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-hir/src/lower/expr_call/array_only_methods.rs`:
- Around line 925-937: The matches! check for provably-array expressions at
lines 925-937 is narrower than similar checks in the slice arm (lines 815-841)
and join arm (lines 858-881), missing variants like ArraySpread,
ArrayFromMapped, ArrayFlat, ArrayToReversed, ArrayToSorted, ArrayToSpliced,
ArrayWith, ArrayEntries, ArrayKeys, ArrayValues, ObjectEntries, and ProcessArgv.
To fix this, expand the matches! pattern at lines 925-937 to include all the
array-producing variants present in the slice and join arms, and apply the same
unification to the other location mentioned (lines 961-973). This will ensure
consistency across all three whitelists and reduce future maintenance burden
when new array-producing expression types are added.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ba54605b-4959-4c6f-87f9-d27a42234127

📥 Commits

Reviewing files that changed from the base of the PR and between 964b540 and 539842d.

📒 Files selected for processing (2)

• crates/perry-hir/src/lower/expr_call/array_only_methods.rs
• crates/perry/tests/issue_5195_property_method_chain.rs