diff --git a/changelog.d/11199-call-result-array-named-methods.md b/changelog.d/11199-call-result-array-named-methods.md new file mode 100644 index 0000000000..5deca8b6a4 --- /dev/null +++ b/changelog.d/11199-call-result-array-named-methods.md @@ -0,0 +1 @@ +Fix an Array-named method on a receiver that is not proven to be an Array being folded to the dense `Array.prototype` op (#11187). In `crates/perry-hir/src/lower/expr_call/array_only_methods.rs`, a call-result receiver was classified by method name: `sort`, `flat` and `toSpliced` folded unconditionally, and the callback iterators folded for `await`/conditional receivers. So mongodb's `collection.find({}).sort({ a: 1 })` became `Array.prototype.sort` and threw "The comparison function must be either a function or undefined". Those arms now require a proven Array receiver (inferred `T[]`/tuple/`Array`/`ReadonlyArray` type, or an Array-producer root via `chain_roots_at_array`). Bare typed identifiers keep their existing classification. Unproven receivers fall through to shape-aware dynamic dispatch, which still reaches the Array helper for a real array. Proven-array LLVM IR is unchanged. Gap test: `test-files/test_gap_11187_call_result_array_named_methods.ts`. diff --git a/crates/perry-hir/src/lower/expr_call/array_only_methods.rs b/crates/perry-hir/src/lower/expr_call/array_only_methods.rs index e783acfafa..351d20977f 100644 --- a/crates/perry-hir/src/lower/expr_call/array_only_methods.rs +++ b/crates/perry-hir/src/lower/expr_call/array_only_methods.rs @@ -325,6 +325,40 @@ fn chain_roots_at_array(ctx: &LoweringContext, expr: &ast::Expr) -> bool { } } +/// #11187: may the dense `Expr::Array` fold below claim this +/// receiver? +/// +/// A bare identifier keeps its existing, type-driven classification (the +/// `recv_is_class` Ident arm and the `local_array_methods.rs` pass own it): +/// only an *untyped* (`None` / `any` / `unknown`) local is unproven. Every +/// other receiver shape — a call result, `await`, a conditional, a property, +/// `this`, `new` — must be positively proven to be an Array, either by its +/// inferred static type or by rooting at an Array producer. +/// +/// Before this, a call-result receiver was classified by *method name* alone: +/// the overlapping callback names (`map`, `find`, …) required a proven root, +/// but `sort`, `flat` and `toSpliced` folded unconditionally, so mongodb's +/// `collection.find({}).sort({ a: 1 })` (`FindCursor.prototype.sort`) became +/// `Array.prototype.sort` and threw "The comparison function must be either a +/// function or undefined". Declining is always sound: the generic tail's +/// dynamic dispatch selects by the receiver's runtime shape, so a real Array +/// still reaches the dense helper. +fn receiver_is_proven_array(ctx: &LoweringContext, obj: &ast::Expr) -> bool { + if let ast::Expr::Ident(ident) = unwrap_transparent_expr(obj) { + return !matches!( + ctx.lookup_local_type(ident.sym.as_ref()), + None | Some(Type::Any) | Some(Type::Unknown) + ); + } + let ty = crate::lower_types::infer_type_from_expr(obj, ctx); + matches!(ty, Type::Array(_) | Type::Tuple(_)) + || matches!( + &ty, + Type::Generic { base, .. } if base == "Array" || base == "ReadonlyArray" + ) + || chain_roots_at_array(ctx, obj) +} + /// `CallExpr` form of [`chain_roots_at_array`] (the inner-call arm hands us a /// `&CallExpr`, not an `&Expr`). fn call_roots_at_array(ctx: &LoweringContext, call: &ast::CallExpr) -> bool { @@ -725,6 +759,33 @@ pub(super) fn try_array_only_methods( if nested_callback_receiver_is_ambiguous { return Ok(Err(args)); } + // #11187: the arms below that fold with no receiver evidence of + // their own — the callback iterators, `sort`, `flat`, + // `toSpliced` — require a proven Array receiver. (`slice`, + // `join`, `indexOf`, `includes`, `push`, `with`, `entries`/ + // `keys`/`values`, `reduceRight`, `toReversed` and `toSorted` + // already check their own proof.) See + // [`receiver_is_proven_array`]. + if matches!( + method_name, + "map" + | "filter" + | "forEach" + | "find" + | "findIndex" + | "findLast" + | "findLastIndex" + | "some" + | "every" + | "reduce" + | "sort" + | "flat" + | "toSpliced" + ) && !recv_is_class + && !receiver_is_proven_array(ctx, &member.obj) + { + return Ok(Err(args)); + } // `entries` / `keys` / `values` are not Array-only names. // Maps, Sets, iterator-like facades, and ordinary user objects // can all provide them. In particular, OpenCode's diff --git a/crates/perry-hir/src/lower/expr_call/array_only_methods_tests.rs b/crates/perry-hir/src/lower/expr_call/array_only_methods_tests.rs index 8c0d194f71..25e30d25fd 100644 --- a/crates/perry-hir/src/lower/expr_call/array_only_methods_tests.rs +++ b/crates/perry-hir/src/lower/expr_call/array_only_methods_tests.rs @@ -79,3 +79,55 @@ fn typed_nested_array_field_keeps_array_map_specialization() { "a statically typed Array field should retain the dense fast path: {hir}" ); } + +/// #11187: a call result whose static type is `any` is not an Array, so +/// `.sort(obj)` / `.flat()` / `.toSpliced()` / `.map(f)` on it must reach +/// the receiver's own method through dynamic dispatch (mongodb's +/// `collection.find({}).sort({ a: 1 })`). +#[test] +fn any_call_result_receiver_does_not_fold_to_array_methods() { + let hir = format!( + "{:?}", + lower( + r#" + function run(holder: any, flag: boolean, other: any) { + holder.find().sort({ a: 1 }); + holder.find().flat(); + holder.find().toSpliced(0, 1); + (flag ? holder : other).map((x: any) => x); + } + "#, + ) + ); + + for folded in ["ArraySort", "ArrayFlat", "ArrayToSpliced", "ArrayMap"] { + assert!( + !hir.contains(folded), + "an unproven receiver was folded to {folded}: {hir}" + ); + } +} + +/// #11187 control: receivers proven to be Arrays keep the dense folds. +#[test] +fn proven_array_call_results_keep_array_folds() { + let hir = format!( + "{:?}", + lower( + r#" + function nums(): number[] { return [3, 1, 2]; } + function run(s: string) { + nums().sort((a, b) => a - b); + nums().map((n) => [n]).flat(); + nums().toSpliced(0, 1); + s.split(",").sort((a, b) => (a < b ? -1 : 1)); + [1, 2].map((n) => n).sort((a, b) => a - b); + } + "#, + ) + ); + + assert_eq!(hir.matches("ArraySort").count(), 3, "{hir}"); + assert!(hir.contains("ArrayFlat"), "{hir}"); + assert!(hir.contains("ArrayToSpliced"), "{hir}"); +} diff --git a/test-files/test_gap_11187_call_result_array_named_methods.ts b/test-files/test_gap_11187_call_result_array_named_methods.ts new file mode 100644 index 0000000000..d31faf0304 --- /dev/null +++ b/test-files/test_gap_11187_call_result_array_named_methods.ts @@ -0,0 +1,163 @@ +// #11187: an Array-named method called on a receiver that is NOT proven to be +// an Array — a call result from an `any`-typed object, a builder method that +// returns `this`, an `await`ed value, a conditional — must run the receiver's +// own method. mongodb's `collection.find({}).sort({ a: 1 })` is the shape that +// failed: the chained call result was folded to `Array.prototype.sort` and +// threw "The comparison function must be either a function or undefined". +// Proven arrays (literals, Array-typed returns, split/Object.keys/Array.from +// chains) are controls: they must keep working exactly as before. + +class Cursor { + log: string[] = []; + note(name: string, args: any[]): this { + this.log.push(name + "(" + JSON.stringify(args) + ")"); + return this; + } + sort(sort: any, direction?: any) { return this.note("sort", [sort, direction]); } + map(f: any) { return this.note("map", [typeof f]); } + filter(f: any) { return this.note("filter", [typeof f]); } + forEach(f: any) { return this.note("forEach", [typeof f]); } + find(q: any) { return this.note("find", [q]); } + findIndex(q: any) { return this.note("findIndex", [q]); } + findLast(q: any) { return this.note("findLast", [q]); } + findLastIndex(q: any) { return this.note("findLastIndex", [q]); } + some(q: any) { return this.note("some", [q]); } + every(q: any) { return this.note("every", [q]); } + reduce(q: any, init?: any) { return this.note("reduce", [q, init]); } + reduceRight(q: any, init?: any) { return this.note("reduceRight", [q, init]); } + flat(depth?: any) { return this.note("flat", [depth]); } + flatMap(f: any) { return this.note("flatMap", [typeof f]); } + toSpliced(a?: any, b?: any) { return this.note("toSpliced", [a, b]); } + toSorted(a?: any) { return this.note("toSorted", [a]); } + toReversed() { return this.note("toReversed", []); } + with(a: any, b: any) { return this.note("with", [a, b]); } + join(sep?: any) { return this.note("join", [sep]); } + slice(a?: any, b?: any) { return this.note("slice", [a, b]); } + indexOf(v: any) { return this.note("indexOf", [v]); } + includes(v: any) { return this.note("includes", [v]); } + push(v: any) { return this.note("push", [v]); } + pop() { return this.note("pop", []); } + shift() { return this.note("shift", []); } + unshift(v: any) { return this.note("unshift", [v]); } + splice(a: any, b?: any) { return this.note("splice", [a, b]); } + reverse() { return this.note("reverse", []); } + fill(v: any) { return this.note("fill", [v]); } + copyWithin(a: any, b: any) { return this.note("copyWithin", [a, b]); } + concat(v: any) { return this.note("concat", [v]); } + at(i: any) { return this.note("at", [i]); } + entries() { return this.note("entries", []); } + keys() { return this.note("keys", []); } + values() { return this.note("values", []); } + limit(n: number): this { return this.note("limit", [n]); } +} + +function show(label: string, fn: () => any) { + try { + const r = fn(); + if (r instanceof Cursor) console.log(label, r.log.join(" ")); + else console.log(label, JSON.stringify(r)); + } catch (e: any) { + console.log(label, "THREW", e && e.message); + } +} + +// 1. The mongodb shape: a method on an `any`-typed holder returns a cursor. +const holder: any = { find() { return new Cursor(); } }; +show("find().sort", () => holder.find().sort({ a: 1 })); +show("find().sort(dir)", () => holder.find().sort("a", -1)); +show("find().sort.limit", () => holder.find().sort({ a: 1 }).limit(2)); +show("find().map", () => holder.find().map((d: any) => d)); +show("find().filter", () => holder.find().filter((d: any) => d)); +show("find().forEach", () => holder.find().forEach((d: any) => d)); +show("find().find", () => holder.find().find({ q: 1 })); +show("find().findIndex", () => holder.find().findIndex({ q: 1 })); +show("find().findLast", () => holder.find().findLast({ q: 1 })); +show("find().findLastIndex", () => holder.find().findLastIndex({ q: 1 })); +show("find().some", () => holder.find().some({ q: 1 })); +show("find().every", () => holder.find().every({ q: 1 })); +show("find().reduce", () => holder.find().reduce({ q: 1 }, 0)); +show("find().reduceRight", () => holder.find().reduceRight({ q: 1 }, 0)); +show("find().flat", () => holder.find().flat()); +show("find().flat(2)", () => holder.find().flat(2)); +show("find().flatMap", () => holder.find().flatMap((d: any) => d)); +show("find().toSpliced", () => holder.find().toSpliced(1, 2)); +show("find().toSpliced()", () => holder.find().toSpliced()); +show("find().toSorted", () => holder.find().toSorted({ a: 1 })); +show("find().toReversed", () => holder.find().toReversed()); +show("find().with", () => holder.find().with(0, 1)); +show("find().join", () => holder.find().join(",")); +show("find().slice", () => holder.find().slice(1, 2)); +show("find().indexOf", () => holder.find().indexOf(3)); +show("find().includes", () => holder.find().includes(3)); +show("find().push", () => holder.find().push(3)); +show("find().pop", () => holder.find().pop()); +show("find().shift", () => holder.find().shift()); +show("find().unshift", () => holder.find().unshift(3)); +show("find().splice", () => holder.find().splice(1, 1)); +show("find().reverse", () => holder.find().reverse()); +show("find().fill", () => holder.find().fill(0)); +show("find().copyWithin", () => holder.find().copyWithin(0, 1)); +show("find().concat", () => holder.find().concat([1])); +show("find().at", () => holder.find().at(0)); +show("find().entries", () => holder.find().entries()); +show("find().keys", () => holder.find().keys()); +show("find().values", () => holder.find().values()); + +// 2. A builder chain that returns `this` — the receiver of `.sort` is the +// result of a typed class method, not an array. +const cur = new Cursor(); +show("limit().sort", () => cur.limit(1).sort({ b: -1 })); +show("limit().flat", () => cur.limit(1).flat()); +show("limit().toSpliced", () => cur.limit(1).toSpliced(0, 1)); +show("limit().map", () => cur.limit(1).map((x: any) => x)); + +// 3. A free function returning `any`. +function openCursor(): any { return new Cursor(); } +show("fn().sort", () => openCursor().sort({ c: 1 })); +show("fn().flat", () => openCursor().flat()); +show("fn().reduce", () => openCursor().reduce({ q: 1 })); + +// 4. Conditional / parenthesised / member receivers. +const flag = Math.random() >= 0; +show("cond.sort", () => (flag ? new Cursor() : holder.find()).sort({ d: 1 })); +show("cond.map", () => (flag ? holder.find() : new Cursor()).map((x: any) => x)); +const wrap: any = { cur: new Cursor() }; +show("member.sort", () => wrap.cur.sort({ e: 1 })); +show("member.flat", () => wrap.cur.flat()); +show("member.toSpliced", () => wrap.cur.toSpliced(0)); + +// 5. Controls: a call result from `any` that IS an array at runtime still +// behaves as an Array (dynamic dispatch reaches the Array method). +const src: any = { list() { return [3, 1, 2]; }, nested() { return [[1], [2, [3]]]; } }; +show("anyArr.sort", () => src.list().sort((a: number, b: number) => a - b)); +show("anyArr.map", () => src.list().map((x: number) => x * 2)); +show("anyArr.flat", () => src.nested().flat()); +show("anyArr.toSpliced", () => src.list().toSpliced(0, 1)); +show("anyArr.reduce", () => src.list().reduce((a: number, b: number) => a + b, 0)); +show("anyArr.find", () => src.list().find((x: number) => x < 3)); + +// 6. Controls: proven arrays (literal, typed return, builtin producers). +function nums(): number[] { return [5, 3, 9, 1]; } +show("lit.sort", () => [3, 1, 2].sort((a, b) => a - b)); +show("typed().sort", () => nums().sort((a, b) => a - b)); +show("typed().flat", () => nums().map((n) => [n, n]).flat()); +show("typed().toSpliced", () => nums().toSpliced(1, 2)); +show("typed().reduce", () => nums().reduce((a, b) => a + b, 0)); +show("split.sort", () => "b,c,a".split(",").sort((x, y) => (x < y ? -1 : 1))); +show("keys.sort", () => Object.keys({ z: 1, y: 2 }).sort((x, y) => (x < y ? -1 : 1))); +show("from.flat", () => Array.from([[1, 2], [3]]).flat()); +show("slice.sort", () => nums().slice().sort((a, b) => b - a)); +show("filter.find", () => nums().filter((n) => n > 2).find((n) => n > 4)); +show("map.toSpliced", () => [1, 2, 3].map((n) => n * 10).toSpliced(0, 1)); + +// 7. Async: the awaited result of an `any` promise. +async function main() { + const coll: any = { find: async () => new Cursor(), rows: async () => [2, 1] }; + const c = await coll.find(); + show("awaited-local.sort", () => c.sort({ f: 1 })); + const viaAwait = (await coll.find()).sort({ g: 1 }); + console.log("await().sort", viaAwait.log.join(" ")); + const rowsSorted = (await coll.rows()).sort((a: number, b: number) => a - b); + console.log("await().rows.sort", JSON.stringify(rowsSorted)); +} +main();