-
-
Notifications
You must be signed in to change notification settings - Fork 737
refactor(formatter): revert all changes from removing AstKind::Argument
#15675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(formatter): revert all changes from removing AstKind::Argument
#15675
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
CodSpeed Performance ReportMerging #15675 will not alter performanceComparing Summary
Footnotes
|
9b611c8 to
1815d8d
Compare
1815d8d to
4682dc9
Compare
|
@taearls, I am sad to say I am going to revert them, as I found that even the changes fixed all of the prettier tests. However, they are not the ideal way, and they introduced more problems. So, to avoid wasting time correcting them one by one, I decided to revert them and re-fix them. If you are interested in how I fix them, see #15676. Anyway, I really appreciate your work! This is not an easy task, but you already did a good job! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR reverts all formatter changes from #13902 that removed AstKind::Argument. The reversion causes significant test regressions across multiple test suites, introducing formatting issues and incomplete implementations marked with TODO comments.
Key Changes:
- Reverts changes to argument handling, parenthesization logic, and type parameters
- Removes optimizations for nested call/new expressions
- Adds multiple TODO comments indicating incomplete functionality
- Causes major test regressions: TypeScript compatibility drops from 90.38% to 83.25%, JavaScript from 93.37% to 72.94%
Reviewed Changes
Copilot reviewed 42 out of 46 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tasks/prettier_conformance/snapshots/prettier.ts.snap.md | Test compatibility regression: 90.38% → 83.25% (43 new failures) |
| tasks/prettier_conformance/snapshots/prettier.js.snap.md | Test compatibility regression: 93.37% → 72.94% (154 new failures) |
| tasks/coverage/snapshots/formatter_typescript.snap | Minor regression: 99.88% → 99.66% (21 new mismatches, 11 new parse errors) |
| tasks/coverage/snapshots/formatter_test262.snap | Major regression: 99.98% → 98.66% (588 new failures) |
| tasks/coverage/snapshots/formatter_babel.snap | Minor regression: 99.22% → 99.18% (1 new mismatch) |
| crates/oxc_formatter/src/write/call_arguments.rs | Removes compact formatting logic and argument analysis optimizations |
| crates/oxc_formatter/src/write/arrow_function_expression.rs | Reverts arrow chain building logic and comment handling |
| crates/oxc_formatter/src/parentheses/expression.rs | Removes extensive parenthesization logic for arguments |
| crates/oxc_formatter/src/utils/mod.rs | Removes is_expression_used_as_call_argument helper |
| Multiple test fixture snapshots | Show unwanted parentheses being added to function arguments |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merge activity
|
4682dc9 to
0bba124
Compare
I understand. No worries! Your changes are much cleaner and better integrated into the existing architecture, so I get it. I'll keep these patterns in mind for future work. Thanks for sharing |

Revert all changes in the formatter that were introduced in #13902, as none of them are the ideal solution for the change in the formatter.