fix(hir): a?.b?.method(args) must short-circuit when a?.b is undefined (threw on undefined receiver)

[proggeramlug]

Problem

a?.b?.some(x => ...)   // when a?.b is undefined → TypeError: Cannot read properties of undefined (reading 'some')
a.b?.some(...)         // works

A double optional-member method call threw on an undefined intermediate receiver instead of short-circuiting to undefined per spec.

Root cause

crates/perry-hir/src/lower/lower_expr.rs, OptChain(Call) lowering. When the upstream a?.b is undefined, the receiver lowers to a Conditional; the inner-Conditional nesting branch reused the un-short-circuited receiver (a.b) as the call object without re-applying the per-receiver nullish guard the non-Conditional path emits. So (undefined).some(args) was evaluated. The optional-CALL variant a?.b?.method?.(args) had the same gap: the function-value guard read (a.b).method off the nullish receiver and threw during guard evaluation.

Fix

Re-apply a receiver-nullish short-circuit (a.b == null ? undefined : (a.b).method(args)) for the optional-member case in both the plain-call and optional-call branches, gated on a side-effect-free receiver (no double evaluation).

Tests

tests/test_optional_chain_double_member_call.sh (runnable: o?.missing?.some(...) → undefined, not a throw; chained and optional-call variants). cargo test -p perry-hir: 196 + new pass; cargo test -p perry-runtime --lib: 1074 passed. Verified end-to-end: a real bundle's _?.allowModels?.some(...) config check no longer throws.

Summary by CodeRabbit

• Bug Fixes

  • Resolved optional chaining behavior for method calls, ensuring undefined is returned instead of throwing when receivers in the chain are nullish.

• Tests

  • Added regression tests covering optional chaining edge cases with nested optional member access and optional call patterns.

[coderabbitai]

📝 Walkthrough

Walkthrough

In the HIR optional-call lowering (OptChainBase::Call), a new opt_member_chain flag tracks whether the callee member access used ?.. When set and the receiver is side-effect-free, a receiver_for_member_guard is captured and used to wrap the resulting else_expr with an additional receiver-nullish conditional, preventing throws on nullish receivers in both the callee_from_chain and non-callee_from_chain paths. A regression test script validates six optional-chaining scenarios.

Changes

Optional chain double member call lowering fix

Layer / File(s)	Summary
opt_member_chain flag and else_expr receiver guard crates/perry-hir/src/lower/lower_expr.rs	Declares opt_member_chain boolean (lines 1980–1992), wires it from inner.optional on OptChainBase::Member (lines 2066–2070), computes receiver_for_member_guard when the receiver is repeatable (lines 2096–2112), then reworks else_expr to add an outer receiver-nullish conditional in the callee_from_chain path and re-adds a direct receiver guard in the non-callee_from_chain path (lines 2147–2191).
Regression test for double optional member call tests/test_optional_chain_double_member_call.sh	Shell script that locates the perry binary, writes an inline main.ts with six ?. call/member scenarios (including nullish and non-nullish receivers), compiles and runs the binary, and asserts the output matches the expected undefined/boolean/number values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 Hop hop through the chain so fine,
A ?. here, a guard in line,
When receiver is null, we don't throw — we flee,
Return undefined, soft as can be!
The tests all PASS, the bunnies cheer,
No more chain-crash bugs this year! 🌸

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and specifically describes the main fix: double optional-member method calls now short-circuit correctly when intermediate values are undefined, addressing a critical spec compliance bug.
Description check	✅ Passed	The PR description provides comprehensive context with a clear problem statement, root cause analysis, fix explanation, and test coverage, though the description template checklist items are not explicitly addressed.
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.

_{✏️ 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/optchain-double-member-call

---

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

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

2139-2141: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

String-builtin optional calls can still short-circuit incorrectly in nested chains.

On Line 2139, guard_cond uses opt_call_member_receiver, which is often a Conditional in a?.b?.method?.(...) shapes. That bypasses the string-safe branch in opt_call_func_nullish_guard, so nested cases like obj?.s?.split?.("/") can incorrectly return undefined instead of calling split.

💡 Proposed fix

-                            let guard_cond = match &opt_call_member_receiver {
+                            let guard_receiver = receiver_for_member_guard
+                                .as_ref()
+                                .or(opt_call_member_receiver.as_ref());
+                            let guard_cond = match guard_receiver {
                                 Some(recv) => opt_call_func_nullish_guard(recv, fixed_callee),
                                 None => Expr::Compare {
                                     op: CompareOp::LooseEq,
                                     left: Box::new(fixed_callee),
                                     right: Box::new(Expr::Null),
                                 },
                             };

Also applies to: 2152-2170

🤖 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/lower_expr.rs` around lines 2139 - 2141, The guard
condition logic around line 2139 using `opt_call_member_receiver` bypasses the
string-safe branch in `opt_call_func_nullish_guard` when the receiver is a
Conditional expression from nested optional member accesses like
`obj?.s?.split?.`, causing it to incorrectly return undefined instead of calling
the builtin function. To fix this, extract or unwrap the innermost actual
receiver from `opt_call_member_receiver` even when it is a Conditional, then
pass that unwrapped receiver to `opt_call_func_nullish_guard` so the string-safe
logic is properly applied. Apply the same fix to the corresponding guard
condition construction code in the second location around lines 2152-2170 that
handles similar optional call patterns.

🧹 Nitpick comments (1)

tests/test_optional_chain_double_member_call.sh (1)

54-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a regression for nested optional-call on string receivers.

This script should also assert a nested-string case (e.g., obj?.s?.split?.("/")) so the string-safe guard path is protected in the same double-member chain shape.

🧪 Suggested test addition

 const o2: any = { fn: () => 99 };
 const r6 = o2?.fn?.();
 console.log("r6=" + r6);                                      // 99
+
+// Nested optional-chain + optional-call on string builtin: should call split.
+const sObj: any = { s: "a/b" };
+const r7 = sObj?.s?.split?.("/").join("|");
+console.log("r7=" + r7);                                      // "a|b"
 EOF
@@
 EXPECTED='r1=undefined
 r2=undefined
 r3=true
 r4=2,4,6
 r5=undefined
-r6=99'
+r6=99
+r7=a|b'

🤖 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 `@tests/test_optional_chain_double_member_call.sh` around lines 54 - 75, Add a
regression test case for nested optional-call on string receivers to protect the
string-safe guard path. Before the EOF marker in the TypeScript test code block,
add a test case that declares an object with a string property and calls an
optional method on that string (e.g., const r7 = obj?.stringProp?.split?.("/");
with appropriate expected output). Then append the corresponding expected result
to the EXPECTED variable to match the actual output from running this new test
case through the PERRY compiler.

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

Outside diff comments:
In `@crates/perry-hir/src/lower/lower_expr.rs`:
- Around line 2139-2141: The guard condition logic around line 2139 using
`opt_call_member_receiver` bypasses the string-safe branch in
`opt_call_func_nullish_guard` when the receiver is a Conditional expression from
nested optional member accesses like `obj?.s?.split?.`, causing it to
incorrectly return undefined instead of calling the builtin function. To fix
this, extract or unwrap the innermost actual receiver from
`opt_call_member_receiver` even when it is a Conditional, then pass that
unwrapped receiver to `opt_call_func_nullish_guard` so the string-safe logic is
properly applied. Apply the same fix to the corresponding guard condition
construction code in the second location around lines 2152-2170 that handles
similar optional call patterns.

---

Nitpick comments:
In `@tests/test_optional_chain_double_member_call.sh`:
- Around line 54-75: Add a regression test case for nested optional-call on
string receivers to protect the string-safe guard path. Before the EOF marker in
the TypeScript test code block, add a test case that declares an object with a
string property and calls an optional method on that string (e.g., const r7 =
obj?.stringProp?.split?.("/"); with appropriate expected output). Then append
the corresponding expected result to the EXPECTED variable to match the actual
output from running this new test case through the PERRY compiler.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fb1a2658-b6c7-4de9-aacd-28d2953dfa15

📥 Commits

Reviewing files that changed from the base of the PR and between bf06f90 and 3f7bfd3.

📒 Files selected for processing (2)

• crates/perry-hir/src/lower/lower_expr.rs
• tests/test_optional_chain_double_member_call.sh