feat(es/parser): support no_paren parser option
#11359
Conversation
CPunisher
changed the title
no_paren syntax optionno_paren parser option
🦋 Changeset detectedLatest commit: 4b3aa19 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
This PR adds a no_paren syntax option to the SWC ECMAScript parser that allows eliminating ParenExpr AST nodes during parsing, aligning with the ESTree specification which doesn't include a ParenthesizedExpression type.
Key Changes:
- Added
no_parenboolean field to bothEsSyntaxandTsSyntaxstructs with corresponding serialization support - Introduced
NO_PARENflag toSyntaxFlagsbitflags and implemented a helper method to check the flag - Modified the parenthesized expression parser to return the inner expression directly when
no_parenis enabled, bypassingParenExprwrapping
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/swc_ecma_parser/src/syntax.rs | Adds no_paren configuration field to both ES and TS syntax structs, implements flag conversion, and adds a helper method |
| crates/swc_ecma_parser/src/parser/expr.rs | Implements the core logic to unwrap parenthesized expressions when the no_paren option is enabled |
| crates/swc_ecma_transforms_base/src/fixer.rs | Adds documentation pointing to the new no_paren option as an alternative to paren_remover |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Binary Sizes
Commit: 1ffd713 |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
CodSpeed Performance ReportMerging #11359 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kdy1
left a comment
kdy1
left a comment
There was a problem hiding this comment.
I have some concerns regarding comments, but I think it can be resolved in the follow-up PR for actually using this feature.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if self.syntax().no_paren() { | ||
| return Ok(expr); |
There was a problem hiding this comment.
No tests have been added to verify the no_paren option works correctly. Consider adding tests that:
- Parse code with
no_paren: trueand verify ParenExpr nodes are not created - Parse code with
no_paren: falseand verify ParenExpr nodes are created - Test both single expressions
(a)and sequence expressions(a, b)with the option enabled
Example test locations:
crates/swc_ecma_parser/src/parser/tests.rsfor unit testscrates/swc_ecma_parser/tests/js.rsor similar for integration tests
**Description:** This is introduced in #11359. But I find some problems that are not easy to fix. So I revert it for now.
2 participants
Description:
There's no
ParenExprast type in estree spec: https://github.com/estree/estree/blob/master/es5.mdThis PR is a simple and useful implementation to eliminate the usage of
paren_remover.