Skip to content

Commit d37c9e2

Browse files
munificentmunificent
authored
Format guard clauses in switches. (#1383)
There were enough tweaks in the formatting specific to the two contexts where the guards can appear that I ultimately decided to make new Piece classes for switch expression cases and switch statement cases. I was hoping for more code reuse, but I think it would have been harder to maintain if I jammed all of this into a single Piece class. I also slightly tweaked the indentation style of switch expression cases. When I'd first added support for switch expressions in the new formatter, I changed the style from what the old formatter had, for reasons that aren't entirely clear. Maybe I thought it made it more consistent to indent +4? Either way, once guard clauses came into play, it became clear that the old style made more sense because it gives a clearer indentation level for the `when` clause when it splits. So this change also makes switch expression formatting more similar to the old style.
1 parent 8040e06 commit d37c9e2

File tree

9 files changed

+696
-37
lines changed

9 files changed

+696
-37
lines changed

‎lib/src/front_end/ast_node_visitor.dart

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import '../dart_formatter.dart';
1212
import '../piece/adjacent.dart';
1313
import '../piece/adjacent_strings.dart';
1414
import '../piece/assign.dart';
15+
import '../piece/case.dart';
1516
import '../piece/constructor.dart';
1617
import '../piece/for.dart';
1718
import '../piece/if.dart';
@@ -1005,8 +1006,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
10051006

10061007
@override
10071008
Piece visitIndexExpression(IndexExpression node) {
1008-
Piece? targetPiece;
1009-
if (node.target case var target?) targetPiece = nodePiece(target);
1009+
var targetPiece = optionalNodePiece(node.target);
10101010
return createIndexExpression(targetPiece, node);
10111011
}
10121012

@@ -1135,9 +1135,16 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
11351135

11361136
@override
11371137
Piece visitLogicalOrPattern(LogicalOrPattern node) {
1138+
// If a logical or pattern is the outermost pattern in a switch expression
1139+
// case, we want to format it like parallel cases and not indent the
1140+
// subsequent operands.
1141+
var indent = node.parent is! GuardedPattern ||
1142+
node.parent!.parent is! SwitchExpressionCase;
1143+
11381144
return createInfixChain<LogicalOrPattern>(
11391145
node,
11401146
precedence: node.operator.type.precedence,
1147+
indent: indent,
11411148
(expression) => (
11421149
expression.leftOperand,
11431150
expression.operator,
@@ -1627,10 +1634,16 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
16271634

16281635
@override
16291636
Piece visitSwitchExpressionCase(SwitchExpressionCase node) {
1630-
if (node.guardedPattern.whenClause != null) throw UnimplementedError();
1637+
var patternPiece = nodePiece(node.guardedPattern.pattern);
16311638

1632-
return createAssignment(
1633-
node.guardedPattern.pattern, node.arrow, node.expression);
1639+
var guardPiece = optionalNodePiece(node.guardedPattern.whenClause);
1640+
var arrowPiece = tokenPiece(node.arrow);
1641+
var bodyPiece = nodePiece(node.expression);
1642+
1643+
return CaseExpressionPiece(patternPiece, guardPiece, arrowPiece, bodyPiece,
1644+
canBlockSplitPattern: node.guardedPattern.pattern.canBlockSplit,
1645+
patternIsLogicalOr: node.guardedPattern.pattern is LogicalOrPattern,
1646+
canBlockSplitBody: node.expression.canBlockSplit);
16341647
}
16351648

16361649
@override
@@ -1653,19 +1666,21 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
16531666
var casePiece = buildPiece((b) {
16541667
b.token(member.keyword);
16551668

1656-
if (member is SwitchCase) {
1657-
b.space();
1658-
b.visit(member.expression);
1659-
} else if (member is SwitchPatternCase) {
1660-
if (member.guardedPattern.whenClause != null) {
1661-
throw UnimplementedError();
1662-
}
1669+
switch (member) {
1670+
case SwitchCase():
1671+
b.space();
1672+
b.visit(member.expression);
1673+
case SwitchPatternCase():
1674+
b.space();
16631675

1664-
b.space();
1665-
b.visit(member.guardedPattern.pattern);
1666-
} else {
1667-
assert(member is SwitchDefault);
1668-
// Nothing to do.
1676+
var patternPiece = nodePiece(member.guardedPattern.pattern);
1677+
var guardPiece =
1678+
optionalNodePiece(member.guardedPattern.whenClause);
1679+
1680+
b.add(CaseStatementPiece(patternPiece, guardPiece));
1681+
1682+
case SwitchDefault():
1683+
break; // Nothing to do.
16691684
}
16701685

16711686
b.token(member.colon);
@@ -1803,6 +1818,15 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
18031818
});
18041819
}
18051820

1821+
@override
1822+
Piece visitWhenClause(WhenClause node) {
1823+
return buildPiece((b) {
1824+
b.token(node.whenKeyword);
1825+
b.space();
1826+
b.visit(node.expression);
1827+
});
1828+
}
1829+
18061830
@override
18071831
Piece visitWhileStatement(WhileStatement node) {
18081832
var condition = buildPiece((b) {
@@ -1858,4 +1882,13 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
18581882

18591883
return result;
18601884
}
1885+
1886+
/// Visits [node] and creates a piece from it if not `null`.
1887+
///
1888+
/// Otherwise returns `null`.
1889+
@override
1890+
Piece? optionalNodePiece(AstNode? node) {
1891+
if (node == null) return null;
1892+
return nodePiece(node);
1893+
}
18611894
}

‎lib/src/front_end/piece_factory.dart

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ mixin PieceFactory {
5555

5656
Piece nodePiece(AstNode node, {bool commaAfter = false});
5757

58+
Piece? optionalNodePiece(AstNode? node);
59+
5860
/// Creates a [ListPiece] for an argument list.
5961
Piece createArgumentList(
6062
Token leftBracket, Iterable<AstNode> elements, Token rightBracket) {
@@ -478,14 +480,8 @@ mixin PieceFactory {
478480
b.visit(caseClause.guardedPattern.pattern);
479481
});
480482

481-
Piece? guardPiece;
482-
if (caseClause.guardedPattern.whenClause case var whenClause?) {
483-
guardPiece = buildPiece((b) {
484-
b.token(whenClause.whenKeyword);
485-
b.space();
486-
b.visit(whenClause.expression);
487-
});
488-
}
483+
var guardPiece =
484+
optionalNodePiece(caseClause.guardedPattern.whenClause);
489485

490486
b.add(IfCasePiece(expressionPiece, casePiece, guardPiece,
491487
canBlockSplitPattern:
@@ -690,7 +686,7 @@ mixin PieceFactory {
690686
/// same precedence.
691687
Piece createInfixChain<T extends AstNode>(
692688
T node, BinaryOperation Function(T node) destructure,
693-
{int? precedence}) {
689+
{int? precedence, bool indent = true}) {
694690
var builder = AdjacentBuilder(this);
695691
var operands = <Piece>[];
696692

@@ -716,7 +712,7 @@ mixin PieceFactory {
716712
traverse(node);
717713
operands.add(builder.build());
718714

719-
return InfixPiece(operands);
715+
return InfixPiece(operands, indent: indent);
720716
}
721717

722718
/// Creates a [ListPiece] for the given bracket-delimited set of elements.

‎lib/src/piece/case.dart

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import '../back_end/code_writer.dart';
6+
import '../constants.dart';
7+
import 'piece.dart';
8+
9+
/// Piece for a case pattern, guard, and body in a switch expression.
10+
class CaseExpressionPiece extends Piece {
11+
/// Split after the `=>` before the body.
12+
static const State _beforeBody = State(1);
13+
14+
/// Split before the `when` guard clause.
15+
static const State _beforeWhen = State(2);
16+
17+
/// Split before the `when` guard clause and after the `=>`.
18+
static const State _beforeWhenAndBody = State(3);
19+
20+
/// The pattern the value is matched against along with the leading `case`.
21+
final Piece _pattern;
22+
23+
/// If there is a `when` clause, that clause.
24+
final Piece? _guard;
25+
26+
/// The `=>` token separating the pattern and body.
27+
final Piece _arrow;
28+
29+
/// The case body expression.
30+
final Piece _body;
31+
32+
/// Whether the pattern can be block formatted.
33+
final bool _canBlockSplitPattern;
34+
35+
/// Whether the outermost pattern is a logical-or pattern.
36+
///
37+
/// We format these specially to make them look like parallel cases:
38+
///
39+
/// switch (obj) {
40+
/// firstPattern ||
41+
/// secondPattern ||
42+
/// thirdPattern =>
43+
/// body;
44+
/// }
45+
final bool _patternIsLogicalOr;
46+
47+
/// Whether the body expression can be block formatted.
48+
final bool _canBlockSplitBody;
49+
50+
CaseExpressionPiece(this._pattern, this._guard, this._arrow, this._body,
51+
{required bool canBlockSplitPattern,
52+
required bool patternIsLogicalOr,
53+
required bool canBlockSplitBody})
54+
: _canBlockSplitPattern = canBlockSplitPattern,
55+
_patternIsLogicalOr = patternIsLogicalOr,
56+
_canBlockSplitBody = canBlockSplitBody;
57+
58+
@override
59+
List<State> get additionalStates => [
60+
_beforeBody,
61+
if (_guard != null) ...[_beforeWhen, _beforeWhenAndBody],
62+
];
63+
64+
@override
65+
void format(CodeWriter writer, State state) {
66+
var allowNewlineInPattern = false;
67+
var allowNewlineInGuard = false;
68+
var allowNewlineInBody = false;
69+
70+
switch (state) {
71+
case State.unsplit:
72+
allowNewlineInBody = _canBlockSplitBody;
73+
break;
74+
75+
case _beforeBody:
76+
allowNewlineInPattern = _guard == null || _patternIsLogicalOr;
77+
allowNewlineInBody = true;
78+
79+
case _beforeWhen:
80+
// Allow newlines only in the pattern if we split before `when`.
81+
allowNewlineInPattern = true;
82+
83+
case _beforeWhenAndBody:
84+
allowNewlineInPattern = true;
85+
allowNewlineInGuard = true;
86+
allowNewlineInBody = true;
87+
}
88+
89+
// If there is a split guard, then indent the pattern past it.
90+
var indentPatternForGuard = !_canBlockSplitPattern &&
91+
!_patternIsLogicalOr &&
92+
(state == _beforeWhen || state == _beforeWhenAndBody);
93+
94+
if (indentPatternForGuard) writer.pushIndent(Indent.expression);
95+
96+
writer.pushAllowNewlines(allowNewlineInPattern);
97+
writer.format(_pattern);
98+
writer.popAllowNewlines();
99+
100+
if (indentPatternForGuard) writer.popIndent();
101+
102+
if (_guard case var guard?) {
103+
writer.pushIndent(Indent.expression);
104+
writer.pushAllowNewlines(allowNewlineInGuard);
105+
writer.splitIf(state == _beforeWhen || state == _beforeWhenAndBody);
106+
writer.format(guard);
107+
writer.popAllowNewlines();
108+
writer.popIndent();
109+
}
110+
111+
writer.space();
112+
writer.format(_arrow);
113+
114+
if (state != State.unsplit) writer.pushIndent(Indent.block);
115+
116+
writer.splitIf(state == _beforeBody || state == _beforeWhenAndBody);
117+
writer.pushAllowNewlines(allowNewlineInBody);
118+
writer.format(_body);
119+
writer.popAllowNewlines();
120+
121+
if (state != State.unsplit) writer.popIndent();
122+
}
123+
124+
@override
125+
void forEachChild(void Function(Piece piece) callback) {
126+
callback(_pattern);
127+
if (_guard case var guard?) callback(guard);
128+
callback(_arrow);
129+
callback(_body);
130+
}
131+
}
132+
133+
/// Piece for a case pattern and guard in a switch statement.
134+
///
135+
/// Unlike [CaseExpressionPiece], this doesn't include the case body, because
136+
/// in a statement, the body is formatted as separate elements in the
137+
/// surrounding sequence.
138+
///
139+
/// This just handles splitting between the pattern and guard.
140+
///
141+
/// [State.unsplit] No split before the guard:
142+
///
143+
/// case pattern when condition:
144+
///
145+
/// [State.split] Split before the `when`:
146+
///
147+
/// case someVeryLongPattern ||
148+
/// anotherSubpattern
149+
/// when longGuardCondition &&
150+
/// anotherOperand:
151+
class CaseStatementPiece extends Piece {
152+
/// The pattern the value is matched against along with the leading `case`.
153+
final Piece _pattern;
154+
155+
/// If there is a `when` clause, that clause.
156+
final Piece? _guard;
157+
158+
CaseStatementPiece(this._pattern, this._guard);
159+
160+
@override
161+
List<State> get additionalStates => [
162+
if (_guard != null) State.split,
163+
];
164+
165+
@override
166+
void format(CodeWriter writer, State state) {
167+
writer.pushAllowNewlines(_guard == null || state == State.split);
168+
169+
// If there is a guard, then indent the pattern past it.
170+
if (_guard != null) writer.pushIndent(Indent.expression);
171+
writer.format(_pattern);
172+
if (_guard != null) writer.popIndent();
173+
174+
if (_guard case var guard?) {
175+
writer.pushIndent(Indent.expression);
176+
writer.splitIf(state == State.split);
177+
writer.format(guard);
178+
writer.popIndent();
179+
}
180+
181+
writer.popAllowNewlines();
182+
}
183+
184+
@override
185+
void forEachChild(void Function(Piece piece) callback) {
186+
callback(_pattern);
187+
if (_guard case var guard?) callback(guard);
188+
}
189+
}

‎lib/src/piece/infix.dart

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ class InfixPiece extends Piece {
1717
/// A leading operator like `foo as int` becomes "Infix(`foo`, `as int`)".
1818
final List<Piece> _operands;
1919

20-
InfixPiece(this._operands);
20+
/// Whether operands after the first should be indented if split.
21+
final bool _indent;
22+
23+
InfixPiece(this._operands, {bool indent = true}) : _indent = indent;
2124

2225
@override
2326
List<State> get additionalStates => const [State.split];
@@ -26,7 +29,7 @@ class InfixPiece extends Piece {
2629
void format(CodeWriter writer, State state) {
2730
if (state == State.unsplit) {
2831
writer.pushAllowNewlines(false);
29-
} else {
32+
} else if (_indent) {
3033
writer.pushIndent(Indent.expression);
3134
}
3235

@@ -42,7 +45,7 @@ class InfixPiece extends Piece {
4245

4346
if (state == State.unsplit) {
4447
writer.popAllowNewlines();
45-
} else {
48+
} else if (_indent) {
4649
writer.popIndent();
4750
}
4851
}

0 commit comments

Comments
 (0)