Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
proggeramlug merged 1 commit into from
Jun 19, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 46 additions & 47 deletions crates/perry-hir/src/lower/expr_call/array_only_methods.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -909,29 +909,32 @@ pub(super) fn try_array_only_methods(
// Fall through to string/generic dispatch.
} else {
let array_expr = lower_expr(ctx, &member.obj)?;
let is_known_string_prop = matches!(&array_expr,
Expr::PropertyGet { property, .. }
if matches!(
property.as_str(),
"stack" | "message" | "name" | "sourceSQL" | "expandedSQL"
)
);
if !is_known_string_prop
&& matches!(
&array_expr,
Expr::ArrayMap { .. }
| Expr::ArrayFilter { .. }
| Expr::ArraySort { .. }
| Expr::ArraySlice { .. }
| Expr::Array(_)
| Expr::ArrayFrom(_)
| Expr::ArrayFromArrayLikeHoley(_)
| Expr::StringSplit(_, _)
| Expr::ObjectKeys(_)
| Expr::ObjectValues(_)
| Expr::PropertyGet { .. }
)
{
// #5195: a `PropertyGet` receiver
// (`arr[i].id.indexOf(...)`, `p.id.indexOf(...)`) is
// NOT necessarily an array — the property is very often
// a string. Folding it to `ArrayIndexOf` (an
// element-equality search) silently returned the wrong
// result for string-typed properties; the old hardcoded
// `stack|message|name|…` allowlist only patched a
// handful of property names. Only fold receivers that
// are *provably* array-producing; every other receiver
// (all property accesses included) falls through to the
// generic method dispatch, which checks the runtime
// value type and routes string- vs. array-`indexOf`
// correctly — exactly as the `slice` arm above does.
if matches!(
&array_expr,
Expr::ArrayMap { .. }
| Expr::ArrayFilter { .. }
| Expr::ArraySort { .. }
| Expr::ArraySlice { .. }
| Expr::Array(_)
| Expr::ArrayFrom(_)
| Expr::ArrayFromArrayLikeHoley(_)
| Expr::StringSplit(_, _)
| Expr::ObjectKeys(_)
| Expr::ObjectValues(_)
) {
let mut it = args.into_iter();
let value_expr = it.next().unwrap();
let from_index = it.next().map(Box::new);
Expand All @@ -948,30 +951,26 @@ pub(super) fn try_array_only_methods(
// Fall through to string/generic dispatch.
} else {
let array_expr = lower_expr(ctx, &member.obj)?;
// Don't treat known string-valued properties as arrays.
let is_known_string_prop = matches!(&array_expr,
Expr::PropertyGet { property, .. }
if matches!(
property.as_str(),
"stack" | "message" | "name" | "sourceSQL" | "expandedSQL"
)
);
if !is_known_string_prop
&& matches!(
&array_expr,
Expr::ArrayMap { .. }
| Expr::ArrayFilter { .. }
| Expr::ArraySort { .. }
| Expr::ArraySlice { .. }
| Expr::Array(_)
| Expr::ArrayFrom(_)
| Expr::ArrayFromArrayLikeHoley(_)
| Expr::StringSplit(_, _)
| Expr::ObjectKeys(_)
| Expr::ObjectValues(_)
| Expr::PropertyGet { .. }
)
{
// #5195: see the `indexOf` arm above — a `PropertyGet`
// receiver is not necessarily an array (the property is
// often a string), so folding it to `ArrayIncludes`
// silently returned `false` on string properties. Only
// fold provably array-producing receivers; property
// accesses fall through to the runtime-type-checked
// generic dispatch.
if matches!(
&array_expr,
Expr::ArrayMap { .. }
| Expr::ArrayFilter { .. }
| Expr::ArraySort { .. }
| Expr::ArraySlice { .. }
| Expr::Array(_)
| Expr::ArrayFrom(_)
| Expr::ArrayFromArrayLikeHoley(_)
| Expr::StringSplit(_, _)
| Expr::ObjectKeys(_)
| Expr::ObjectValues(_)
) {
let mut it = args.into_iter();
let value_expr = it.next().unwrap();
let from_index = it.next().map(Box::new);
Expand Down
138 changes: 138 additions & 0 deletions crates/perry/tests/issue_5195_property_method_chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
//! Regression tests for #5195 — an inline property-access chained into an
//! array/string-overlapping method call (`arr[i].id.indexOf(x)`,
//! `p.id.includes(x)`) miscompiled and returned the wrong result.
//!
//! Root cause: `lower/expr_call/array_only_methods.rs` folded a `PropertyGet`
//! receiver of `.indexOf` / `.includes` to `ArrayIndexOf` / `ArrayIncludes`
//! (an element-equality search) unless the property name happened to be in a
//! hardcoded `stack|message|name|sourceSQL|expandedSQL` allowlist. A genuine
//! string property (`id: string`) fell through that allowlist and was searched
//! as an array, so `arr[i].id.indexOf("PRO")` returned -1 (and the equivalent
//! `.filter(p => p.id.indexOf("PRO") >= 0)` predicate returned 0 matches) with
//! no error — silently breaking a StoreKit product lookup downstream.
//!
//! The fix only folds receivers that are *provably* array-producing
//! (`.map`/`.filter`/`.split`/`Object.keys`/array literals/…). Every property
//! access now falls through to `js_native_call_method`, which checks the
//! runtime value type and routes string- vs. array-`indexOf` correctly — the
//! same way the `slice` arm already behaved.

use std::path::PathBuf;
use std::process::Command;

fn perry_bin() -> PathBuf {
PathBuf::from(env!("CARGO_BIN_EXE_perry"))
}

fn compile_and_run(dir: &std::path::Path, entry: &std::path::Path) -> String {
let output = dir.join("main_bin");
let compile = Command::new(perry_bin())
.current_dir(dir)
.arg("compile")
.arg(entry)
.arg("-o")
.arg(&output)
.output()
.expect("run perry compile");
assert!(
compile.status.success(),
"perry compile failed\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&compile.stdout),
String::from_utf8_lossy(&compile.stderr)
);
let run = Command::new(&output).output().expect("run compiled binary");
assert!(
run.status.success(),
"compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}",
run.status,
String::from_utf8_lossy(&run.stdout),
String::from_utf8_lossy(&run.stderr)
);
String::from_utf8_lossy(&run.stdout).to_string()
}

/// The exact issue shape: a string property read inline and immediately
/// `.indexOf`/`.includes`'d, both in a loop and inside a `.filter` predicate.
/// Pre-fix every count was 0; they must match the extract-to-local form.
#[test]
fn string_property_indexof_includes_inline_match() {
let dir = tempfile::tempdir().expect("tempdir");
let entry = dir.path().join("main.ts");
std::fs::write(
&entry,
r#"
interface P { id: string }
const arr: P[] = [{ id: "GSC_PRO_MONTHLY" }, { id: "GSC_AGENCY_MONTHLY" }, { id: "GSC_PRO_ANNUAL" }]

let inlineIdx = 0
for (let i = 0; i < arr.length; i++) { if (arr[i].id.indexOf("PRO") >= 0) inlineIdx++ }
console.log("inlineIdx=" + inlineIdx)

let inlineInc = 0
for (let i = 0; i < arr.length; i++) { if (arr[i].id.includes("PRO")) inlineInc++ }
console.log("inlineInc=" + inlineInc)

console.log("filter=" + arr.filter(function (p: P) { return p.id.indexOf("PRO") >= 0 }).length)
console.log("chain=" + arr[0].id.toLowerCase().indexOf("pro"))
"#,
)
.expect("write entry");

let stdout = compile_and_run(dir.path(), &entry);
assert!(
stdout.contains("inlineIdx=2"),
"arr[i].id.indexOf(\"PRO\") must find 2 (got: {stdout})"
);
assert!(
stdout.contains("inlineInc=2"),
"arr[i].id.includes(\"PRO\") must find 2 (got: {stdout})"
);
assert!(
stdout.contains("filter=2"),
"filter(p => p.id.indexOf(\"PRO\") >= 0) must keep 2 (got: {stdout})"
);
assert!(
stdout.contains("chain=4"),
"arr[0].id.toLowerCase().indexOf(\"pro\") must be 4 (got: {stdout})"
);
}

/// The fix must NOT regress a genuinely array-typed property: `.indexOf` /
/// `.includes` on an array field still search the array (via the runtime-typed
/// generic dispatch) and return the element index / membership.
#[test]
fn array_typed_property_indexof_includes_still_search_array() {
let dir = tempfile::tempdir().expect("tempdir");
let entry = dir.path().join("main.ts");
std::fs::write(
&entry,
r#"
class Box { tags: string[] = ["red", "green", "blue"] }
const b = new Box()
console.log("idx=" + b.tags.indexOf("green"))
console.log("inc=" + b.tags.includes("blue"))
console.log("miss=" + b.tags.indexOf("pink"))
const cfg = { nums: [10, 20, 30] }
console.log("nidx=" + cfg.nums.indexOf(20))
"#,
)
.expect("write entry");

let stdout = compile_and_run(dir.path(), &entry);
assert!(
stdout.contains("idx=1"),
"array-property indexOf must return the element index (got: {stdout})"
);
assert!(
stdout.contains("inc=true"),
"array-property includes must return true for a member (got: {stdout})"
);
assert!(
stdout.contains("miss=-1"),
"array-property indexOf must return -1 for a non-member (got: {stdout})"
);
assert!(
stdout.contains("nidx=1"),
"numeric array-property indexOf must return the index (got: {stdout})"
);
}
Loading