dart-lang / dart_style

An opinionated formatter/linter for Dart code
https://pub.dev/packages/dart_style
BSD 3-Clause "New" or "Revised" License
645 stars 118 forks source link

Remove pushAllowNewlines() and popAllowNewlines() from CodeWriter. #1438

Closed munificent closed 5 months ago

munificent commented 5 months ago

One of the main ways that formatting rules are expressed is by a parent piece controlling whether a child piece may contain newlines when the parent is in a certain state. The API for this was stateful: Pieces could push or pop onto a stack to say whether they allow or disallow newlines. When child pieces are formatted, we use the top of the stack to determine whether the solution should be invalidated if the child ended up containing a newline.

I thought there was a compelling need for this API to be stateful because if a parent piece doesn't push anything onto the stack, then it will inherit the constraint of its parent. In that way, newlines would percolate up until reaching a piece that cared. It turns out that isn't necessary: Each parent piece will either directly invalidate based on its immediate children or not.

So I got rid of the stack and the stateful API completely. When calling CodeWriter.format() to format a child piece, the parent passes in right then whether or not it allows a newline in the child. At least according to the current tests, that seems to be sufficient.

I think this simplifies reasoning about the formatting code in each piece, because you don't have to mentally track the allow newline stack.

Also, I'm working on some larger changes to be able to express "a newline is allowed here but only from block splitting, not expression splitting", which we need to prevent some annoying incorrect format. I believe this will get the codebase in a better state to be able to express that.