Skip to content
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

fix(js_formatter): fix SimpleArgument checks for call and member expressions #1057

Merged
merged 2 commits into from
Dec 5, 2023

Conversation

faultyserver
Copy link
Contributor

Summary

When I implemented some previous fixes for call argument grouping in #777, I made it use SimpleArgument, since that's the same thing Prettier does. I also expanded some of the things that it matched in #757, but there were still a few more things that were being decided on incorrectly:

  • Computed member expressions, like array[0], were not considered simple, because it only checked static member expressions.
  • All calls with a single argument were considered simple, even if the argument to that call was not simple, like Math.floor(1 + 2).
  • Calls with any number of arguments could be considered simple, rather than limiting to the current depth.

This PR addresses all of those errors and matches Prettier much more consistently in all of the cases I've checked.

Test Plan

Added a bunch of spec tests to cover most of the cases. There are plenty more that aren't covered, though, but they are covered by the normal simple_arguments tests.

Copy link

netlify bot commented Dec 5, 2023

Deploy Preview for biomejs canceled.

Name Link
🔨 Latest commit 7d29098
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/656f6b4116eb0f0008559f28

@github-actions github-actions bot added A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages labels Dec 5, 2023
@faultyserver faultyserver merged commit 78ad1d1 into main Dec 5, 2023
18 checks passed
@faultyserver faultyserver deleted the faulty/call-arg-groups branch December 5, 2023 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants