Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Call expression arguments formatting #2421

Closed
Tracked by #2403
MichaReiser opened this issue Apr 13, 2022 · 2 comments · Fixed by #2711
Closed
Tracked by #2403

Call expression arguments formatting #2421

MichaReiser opened this issue Apr 13, 2022 · 2 comments · Fixed by #2711
Assignees
Labels
A-Formatter Area: formatter I-Difficult Implementation: requires deep knowledge of the tools and the problem.

Comments

@MichaReiser
Copy link
Contributor

MichaReiser commented Apr 13, 2022

Prettier has sophisticated formatting for call arguments that prevents unnecessary line breaks. For example, it formats the following snipped without breaking the passed arrow function over multiple lines (except the body).

// Grouped if less than 3 arguments
testMethod(() => {
  expected(() =>
    moduleMocker.generateFromMetadata(moduleMocker.getMetadata(/a/))
  ).not.toThrow();
}, "string");

// Prettier:
// Grouped if less than 3 arguments
testMethod(() => {
  expected(() => moduleMocker.generateFromMetadata(moduleMocker.getMetadata(/a/))).not.toThrow();
}, "string");

Relevant test cases:

Expected

Output matches Prettier's output (or closely)

@MichaReiser MichaReiser added the A-Formatter Area: formatter label Apr 13, 2022
@ematipico ematipico added the I-Difficult Implementation: requires deep knowledge of the tools and the problem. label May 5, 2022
@ematipico
Copy link
Contributor

We need to study prettier' source and see what they are doing. They use Alternatives for their solution. So probably we should wait until we have a better IR that matches prettier's.

MichaReiser added a commit that referenced this issue May 23, 2022
Our JS Formatter is over-fitting on the test suite and it's very easy to introduce some stability issue when making any change to the formatter/printer. 

I was able to narrow down the main sources of the instability issues and fixed them as best I could. However, this PR introduces two regressions that I want to address in follow up PRs.

1. Trailing commas in call arguments: call-arguments now always print a trailing comma. Fixing this requires more extensive rules that use `BestFitting`. See #2421 for the follow-up PR. 
2. Formatting of inline comments. Inline comments aren't moved to the end anymore. This isn't ideal, I'm working on a follow up PR that re-defines the comment printing 

This PR removes `HardGroup`s. The main purpose of `HardGroup`s was to more deterministically handle inline comments. This is no longer needed because this PR removes this inline comment printing and we need to come up with a comment printing that doesn't require every node to be wrapped inside of a `HardGroup`. 

The "non-break" behaviour can be achieved by either just creating a sequence of tokens separated by spaces or using `BestFitting` where needed.
@ematipico
Copy link
Contributor

I am going to a look at this now

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Formatter Area: formatter I-Difficult Implementation: requires deep knowledge of the tools and the problem.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants