fix(symbol): resolve well-known symbols via optional chaining Symbol?.iterator (#6719)

[proggeramlug]

Fixes #6719. Follow-up to #6676 (computed Symbol["iterator"]).

Problem

Optional-member access of a well-known symbol on the Symbol constructor returned undefined:

typeof Symbol?.iterator          // node: "symbol"   perry: "undefined"
Symbol?.iterator === Symbol.iterator  // node: true   perry: false

#6676 fixed the bracket form Symbol["iterator"], and (0, Symbol.iterator) also worked, but the optional-chaining form Symbol?.iterator still returned undefined. Downstream, a class/object keyed with a well-known symbol reached via ?. was keyed under undefined and therefore not iterable:

class R { *[Symbol?.iterator]() { yield 1; yield 2; } }
[...new R()]   // node: [1,2]   perry: TypeError: value is not iterable

Root cause

The dot (Symbol.iterator) and string-bracket (Symbol["iterator"], #6676) forms fold to the @@__perry_wk_<name> well-known-symbol sentinel in lower_member_inner (expr_member.rs). The optional-chaining forms lower through a separate arm (arm_optchain.rs, OptChainBase::Member) that never routed through that resolver — so a ?. read fell through to a generic property get on the Symbol constructor and returned undefined.

Fix

• Extract the Symbol well-known resolver from lower_member_inner into a shared try_fold_symbol_well_known_member(ctx, obj, prop) — one resolver for the dot, string-bracket, and runtime-key (js_symbol_computed_member) forms — and wire it into the optional-chain Member arm. Symbol is a non-nullish global, so Symbol?.iterator === Symbol.iterator and the ?. short-circuit is dead; the resolver returns the fold directly.
• The extracted resolver also applies symbols: computed Symbol["iterator"] returns undefined — breaks es2015 yield*/for-of/spread lowering #6676's real-global gate (lookup_local("Symbol").is_none()) to the dot form, which previously lacked it: a locally-shadowed Symbol now resolves normally instead of folding (strict correctness improvement, covered by the new test's shadow case).

No runtime changes — js_symbol_computed_member already exists (#6676).

Validation

• New gap test test_gap_6719_optional_chain_symbol.ts is byte-identical to Node 26.5 — optional dot/bracket/runtime-key forms, class & object-literal computed method keys, non-well-known keys → undefined, local Symbol shadowing, and nullish-receiver short-circuit.
• Rigorous in-worktree A/B (base = branch point, node held constant): every other Symbol/iterator gap test and every gap test using optional chaining is byte-neutral (fixed output == base output); the target test flips base-FAIL → fixed-PASS.
• rustfmt --check clean; file-size gate green (expr_member.rs allowlisted).

Version bump + changelog intentionally omitted — maintainer folds those in at merge (external-contributor flow).

Summary by CodeRabbit

• Bug Fixes

  • Fixed optional chaining when accessing well-known Symbol properties (including dot and computed forms) so they resolve correctly instead of falling back to generic property access.
  • Improved handling for both literal and runtime computed keys: well-known names fold appropriately, while non-well-known keys keep normal behavior.
  • Ensured Symbol shadowing is respected and optional chaining still short-circuits for null/undefined receivers.

• Tests

  • Added a regression test covering optional-chain well-known Symbol access, literal vs runtime keys, shadowed Symbol, and null-receiver short-circuiting.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4e46a221-42f4-463f-b4b5-db33c129149f

📥 Commits

Reviewing files that changed from the base of the PR and between 6328a79 and 46709b9.

📒 Files selected for processing (2)

• crates/perry-hir/src/lower/expr_member.rs
• test-files/test_gap_6719_optional_chain_symbol.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/perry-hir/src/lower/expr_member.rs

---

📝 Walkthrough

Walkthrough

This change centralizes well-known Symbol member lowering, applies it to optional chaining, preserves shadowed bindings, and adds regression coverage for computed access, iterable keys, nullish receivers, and scoping.

Changes

Optional well-known Symbol access

Layer / File(s)	Summary
Shared Symbol member folding crates/perry-hir/src/lower/expr_member.rs	A shared helper folds well-known members on the unshadowed global Symbol, handles dynamic computed keys, and replaces duplicated lowering branches.
Optional-chain integration crates/perry-hir/src/lower/lower_expr/arm_optchain.rs	Optional member lowering uses the shared helper before falling through to generic property access.
Regression coverage test-files/test_gap_6719_optional_chain_symbol.ts	Tests cover optional dot and computed access, iterable class and object keys, local Symbol shadowing, and nullish short-circuiting.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• PerryTS/perry#6685: Introduced the computed well-known Symbol handling that this change shares and extends to optional chaining.

Sequence Diagram(s)

sequenceDiagram
  participant Source
  participant OptionalChainLowering
  participant SymbolMemberHelper
  participant FoldedExpression
  Source->>OptionalChainLowering: lower Symbol?.member
  OptionalChainLowering->>SymbolMemberHelper: resolve well-known Symbol member
  SymbolMemberHelper->>FoldedExpression: create folded symbol expression
  FoldedExpression-->>OptionalChainLowering: return expression

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the core fix for optional-chaining Symbol well-known member resolution.
Description check	✅ Passed	The description covers the issue, root cause, fix, and validation, though it doesn't follow the exact template headings.
Linked Issues check	✅ Passed	The changes match #6719 by routing optional-chaining Symbol member access through the shared well-known-symbol resolver and covering shadowing/tests.
Out of Scope Changes check	✅ Passed	The modified code and test are all directly related to the Symbol optional-chaining fix, with no obvious unrelated additions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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

🧹 Nitpick comments (1)

test-files/test_gap_6719_optional_chain_symbol.ts (1)

78-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add class Symbol and function Symbol shadowing cases. const Symbol only covers one binding form; these should also keep Symbol?.iterator on the local binding.

🤖 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 `@test-files/test_gap_6719_optional_chain_symbol.ts` around lines 78 - 83,
Extend the optional-chain shadowing coverage in shadowed() with separate class
Symbol and function Symbol declarations, verifying that Symbol?.iterator
continues resolving to each local binding rather than folding the global symbol.
Preserve the existing const Symbol case and expected runtime-value assertions.

Source: Path instructions

🤖 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-hir/src/lower/expr_member.rs`:
- Line 160: Update the Symbol shadowing condition in the member-expression
lowering logic to use ctx.shadows_unqualified_global("Symbol") instead of
ctx.lookup_local("Symbol"). Preserve the existing obj_ident.sym check while
ensuring all binding types that shadow the global Symbol are detected.

---

Nitpick comments:
In `@test-files/test_gap_6719_optional_chain_symbol.ts`:
- Around line 78-83: Extend the optional-chain shadowing coverage in shadowed()
with separate class Symbol and function Symbol declarations, verifying that
Symbol?.iterator continues resolving to each local binding rather than folding
the global symbol. Preserve the existing const Symbol case and expected
runtime-value assertions.

🪄 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: 4f72706d-bad8-4792-a5ad-831059be4a78

📥 Commits

Reviewing files that changed from the base of the PR and between f073bbf and 6328a79.

📒 Files selected for processing (3)

• crates/perry-hir/src/lower/expr_member.rs
• crates/perry-hir/src/lower/lower_expr/arm_optchain.rs
• test-files/test_gap_6719_optional_chain_symbol.ts

[proggeramlug]

Thanks @coderabbitai — both addressed in 46709b9:

1. Shadowing gate (actionable). Verified the finding is real: with lookup_local alone, class Symbol { static iterator = "x" }; Symbol?.iterator folded to the global well-known symbol (printed Symbol(Symbol.iterator) instead of "x") because class/function declarations aren't in locals. Switched the shared resolver to ctx.shadows_unqualified_global("Symbol") — the established idiom in this file (used for process), a strict superset covering local + function + imported-function + class bindings. This fixes the dot, bracket (symbols: computed Symbol["iterator"] returns undefined — breaks es2015 yield*/for-of/spread lowering #6676), and optional-chain (symbols: Symbol?.iterator (optional chaining) returns undefined — well-known symbol not resolved via optional access (follow-up #6676) #6719) forms in one place.

2. Test coverage (nitpick). Added class Symbol and function Symbol shadowing cases next to the existing const Symbol case; all three are byte-identical to Node 26.5.

Re-validated: gap test byte-identical to Node, and an in-worktree A/B (65 Symbol/optional-chaining gap tests, node held constant) is byte-neutral vs the base — only the target test flips fail→pass.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@proggeramlug Thanks for the thorough validation. Using ctx.shadows_unqualified_global("Symbol") is the correct shared guard: it preserves local, imported, function, and class shadowing across dot, bracket, and optional-chain forms. The added shadowing cases and byte-neutral A/B result provide strong regression coverage.

🐇 ✅

---

✏️ Learnings added

Learnt from: proggeramlug
URL: https://github.com/PerryTS/perry/pull/6729

Timestamp: 2026-07-20T17:42:21.708Z
Learning: In `crates/perry-hir/src/lower/expr_member.rs`, well-known global `Symbol` folding must use `LoweringContext::shadows_unqualified_global("Symbol")`, rather than only `lookup_local`, because `class Symbol` and `function Symbol` declarations are not represented in `locals`. This guard preserves local, imported, function, and class shadowing for dot, computed-member, and optional-chain Symbol access.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!