Skip to content

Commit d94cb10

Browse files
committed
Avoid splitting chains containing only properties.
I was trying out the new formatter on a corpus after #1687 and saw a bunch of diffs like: ```dart // Before: Widget( name: some.property.chain, ) // After: Widget( name: some .property .chain, ) ``` While the new headline style looks nice for most method call chains, it looks weird if it's only just a series of properties. In particular, a single `foo.bar` looks really silly when split. This PR bumps the split cost of a chain up if it only contains properties. That leads the solver to prefer splitting the surrounding construct when possible.
1 parent da2d0fa commit d94cb10

File tree

3 files changed

+63
-14
lines changed

3 files changed

+63
-14
lines changed

CHANGELOG.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,23 @@
22

33
* Format null-aware elements.
44

5+
* Avoid splitting chains containing only properties.
6+
7+
```dart
8+
// Before:
9+
variable = target
10+
.property
11+
.another;
12+
13+
// After:
14+
variable =
15+
target.property.another;
16+
```
17+
18+
Note that this only applies to `.` chains that are only properties. If there
19+
are method calls in the chain, then it prefers to split the chain instead of
20+
splitting at `=`, `:`, or `=>`.
21+
522
* Allow the target or property chain part of a split method chain on the RHS of
623
`=`, `:`, and `=>` (#1466).
724

lib/src/piece/chain.dart

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -110,19 +110,33 @@ final class ChainPiece extends Piece {
110110

111111
@override
112112
int stateCost(State state) {
113-
// If the chain is a cascade, lower the cost so that we prefer splitting
114-
// the cascades instead of the target. Prefers:
115-
//
116-
// [element1, element2]
117-
// ..cascade();
118-
//
119-
// Over:
120-
//
121-
// [
122-
// element1,
123-
// element2,
124-
// ]..cascade();
125-
if (state == State.split) return _isCascade ? 0 : 1;
113+
if (state == State.split) {
114+
// If the chain is a cascade, lower the cost so that we prefer splitting
115+
// the cascades instead of the target. Prefers:
116+
//
117+
// [element1, element2]
118+
// ..cascade();
119+
//
120+
// Over:
121+
//
122+
// [
123+
// element1,
124+
// element2,
125+
// ]..cascade();
126+
if (_isCascade) return 0;
127+
128+
// If the chain is only properties, try to keep them together. Prefers:
129+
//
130+
// variable =
131+
// target.property.another;
132+
//
133+
// Over:
134+
//
135+
// variable = target
136+
// .property
137+
// .another;
138+
if (_leadingProperties == _calls.length) return 2;
139+
}
126140

127141
return super.stateCost(state);
128142
}

test/tall/invocation/chain_property.stmt

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,22 @@ avian
5050
.equine
5151
.feline
5252
.piscine
53-
.method();
53+
.method();
54+
>>> Prefer splitting an assignment over splitting a pure property chain.
55+
variable = avian.bovine.canine.equine.feline;
56+
<<<
57+
variable =
58+
avian.bovine.canine.equine.feline;
59+
>>> Don't prefer splitting an assignment if there are methods in the chain.
60+
variable = avian.bovine.canine.equine.feline();
61+
<<<
62+
variable = avian.bovine.canine.equine
63+
.feline();
64+
>>> Don't prefer splitting an assignment if there are methods in the chain.
65+
variable = avian.bovine().canine.equine().feline;
66+
<<<
67+
variable = avian
68+
.bovine()
69+
.canine
70+
.equine()
71+
.feline;

0 commit comments

Comments
 (0)