fix(codegen): parenthesize let/async for-of head target#23008
Conversation
Merge activity
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e78a43d4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Merging this PR will not alter performance
Comparing 
Footnotes
|
9e78a43 to
20c5017
Compare
|
Addressed the P2 review: replaced the |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20c50176c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
20c5017 to
96f8e8f
Compare
|
Addressed the second P2 review: the wrap now covers any for-of head whose emitted leading token is |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96f8e8faa1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
96f8e8f to
79b1fd5
Compare
|
Addressed the third P2 review: the helper now looks through |
e45f6b9 to
2fd3c24
Compare
## What
A `for...of` head may not start with the `let` token, nor be the bare identifier `async` (ECMA-262: `for ( [lookahead ∉ { let, async of }] LeftHandSideExpression of ...`). Such a head must be parenthesized.
## Approach
The wrap is decided **inside the for-of head printing** (`ForOfStatement::gen`), so the hot `IdentifierReference` path stays untouched. A small structural check determines whether the emitted head would start with `let`, mirroring codegen's output decisions:
- member / TS-wrapper / parenthesized expressions are walked to their leftmost object (`let.x`, `let.x[0]`, `(let).x`)
- `let[...]` already self-wraps to `(let)[...]`, so it is not double-wrapped
A bare non-`await` `async` head is handled with a direct check. `for...in` heads are never wrapped.
## Examples
| Input | Before | After |
| --- | --- | --- |
| `for ((let) of x)` | `for (let of x)` ❌ | `for ((let) of x)` ✅ |
| `for ((let.x) of x)` | `for (let.x of x)` ❌ | `for ((let.x) of x)` ✅ |
| `for (((let).x) of x)` | `for (let.x of x)` ❌ | `for ((let.x) of x)` ✅ |
| `for ((let[0]) of x)` | `for (let[0] of x)` ❌ | `for ((let)[0] of x)` ✅ |
| `for ((async) of x)` | `for ((async) of x)` | `for ((async) of x)` |
| `for (async in x)` | `for ((async) in x)` | `for (async in x)` ✅ |
All outputs verified with `node --check`.
2fd3c24 to
597ed85
Compare
### 💥 BREAKING CHANGES - ee4dc73 ast: [**BREAKING**] Add `#[non_exhaustive]` to AST nodes (#23046) (overlookmotel) - 4c35362 ast: [**BREAKING**] Add `AstBuilder::template_element_escape_raw` and `template_element_escape_raw_with_lone_surrogates` methods (#23047) (overlookmotel) ### 🚀 Features - b846ab2 react_compiler: Integrate the Rust port of the React Compiler (#22942) (Boshen) - 5b8dd68 parser: Report TS1255 for invalid class definite assertions (#22917) (camc314) - 85efabf semantic: Make building the class table optional, off by default (#22862) (Boshen) ### 🐛 Bug Fixes - 556acdc codegen: Parenthesize TS-cast assignment targets (#23112) (Boshen) - 37169ff codegen: Don't emit space between postfix `--` and `>` when minifying (#23036) (Boshen) - a4b1bf7 codegen: Drop redundant whitespace in minified TypeScript output (#23038) (Boshen) - cf53285 parser: Report reserved type-declaration names in the parser (#23035) (Boshen) - 4e44969 ast: Fix UB in `escape_template_element_raw` (#23052) (overlookmotel) - c543154 parser: Report comma operator in JSX expression in the parser (#23030) (Boshen) - 325c94f codegen: Tighten conditional-type and constructor-type whitespace when minifying (#23033) (Boshen) - 95dd3a2 parser: Report `import type` alias to a non-external reference in the parser (#23032) (Boshen) - 90180b8 codegen: Drop space after `:` in function return type when minifying (#23028) (Boshen) - 6da876e parser: Report `abstract` private class field in the parser (#23029) (Boshen) - 28467ce codegen: Don't emit space before a postfix update operand when minifying (#23027) (Boshen) - cb29926 codegen: Drop redundant space after `export default` when minifying (#23024) (Boshen) - 62965ae codegen: Drop redundant space after `else` when minifying (#23025) (Boshen) - 989230a parser: Report compound assignment to non-simple target in the parser (#23022) (Boshen) - 06f367c parser: Report `super.#field` private access in the parser (#23014) (Boshen) - 184edef codegen: Print space before `const`/`declare` enum modifier (#23013) (Boshen) - 4d722e0 parser: Report duplicate switch `default` clause in the parser (#23012) (Boshen) - 597ed85 codegen: Parenthesize `let`/`async` for-of head target (#23008) (Boshen) - 8b631bf codegen: Remove stray space before mapped type value colon (#23010) (Boshen) - c08407e codegen: Don't over-parenthesize `in` inside an arrow in a for-init (#23009) (Boshen) - 600cd6f codegen: Parenthesize lower-precedence `TSInstantiationExpression` operand (#23007) (Boshen) - 187e1a5 codegen: Don't leak space after comment-only JSX expression container (#23006) (Boshen) - 294c473 codegen: Don't over-parenthesize `TSTypeAssertion` operand (#23004) (Boshen) - 786d96f codegen: Give `TSTypeAssertion` unary precedence (#23002) (Boshen) - 1295882 parser: Report `new.target` and `import.meta` syntax errors in the parser (#23003) (Boshen) - d727b6b codegen: Parenthesize `await` expression as base of `**` (#23001) (Boshen) - 67dfa08 codegen: Keep parentheses around `new` callees containing a call (#22997) (Boshen) - 17e7cf3 parser: Disallow unerasable `as`/`satisfies` assertions (#22986) (Boshen) - beb46d3 parser: Commit to module goal on decorated exports (#22941) (Boshen) - 49e63f7 isolated-declarations: Require annotations for satisfies initializers (#22898) (camc314) - 8c93601 isolated-declarations: Allow unknown enum initializer in non-const enum (#22900) (camc314) ### ⚡ Performance - 7d89909 parser: Peek instead of lookahead for yield disambiguation (#23071) (Boshen) - bf872f0 parser: Skip arrow lookahead for a parenthesized literal (#23070) (Boshen) - d19fc54 parser: Guard type-argument speculation behind an angle-token check (#23069) (Boshen) - 8eb5507 parser: Skip redundant member-rest re-scan on call entry (#23068) (Boshen) - 883dfc1 parser: Skip parse_call_expression_rest when no call follows (#23063) (Boshen) - b171153 parser: Peek before the await-using lookahead (#23059) (Boshen) - 56f21bd parser: Use peek_token for the TS `asserts` type predicate (#23058) (Boshen) - 68805ac parser: Use peek_token instead of checkpoint/rewind for single-token decisions (#23056) (Boshen) - 1f9d8eb ast: `AstBuilder::template_element_escape_raw` avoid allocation if no escape required (#23053) (overlookmotel) - 502b04d semantic: Move cold function redeclaration handling into `#[cold]` function (#22973) (overlookmotel) ### 📚 Documentation - 275d318 napi/minifier: Point `target` to oxc docs (#23102) (camc314) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
What
A
for...ofhead may not start with thelettoken, nor be the bare identifierasync(ECMA-262:for ( [lookahead ∉ { let, async of }] LeftHandSideExpression of ...). Such a head must be parenthesized.Approach
The wrap is decided inside the for-of head printing (
ForOfStatement::gen), so the hotIdentifierReferencepath stays untouched. A small structural check determines whether the emitted head would start withlet, mirroring codegen's output decisions:let.x,let.x[0],(let).x)let[...]already self-wraps to(let)[...], so it is not double-wrappedA bare non-
awaitasynchead is handled with a direct check.for...inheads are never wrapped.Examples
for ((let) of x)for (let of x)❌for ((let) of x)✅for ((let.x) of x)for (let.x of x)❌for ((let.x) of x)✅for (((let).x) of x)for (let.x of x)❌for ((let.x) of x)✅for ((let[0]) of x)for (let[0] of x)❌for ((let)[0] of x)✅for ((async) of x)for ((async) of x)for ((async) of x)for (async in x)for ((async) in x)for (async in x)✅All outputs verified with
node --check.