Skip to content

Commit 8040e06

Browse files
munificentmunificent
authored
Fill in a bunch of corners of if-case and patterns. (#1382)
- Format if-case elements in collection literals. - Format `when` clauses in if-case. - Format wildcard patterns (which are basically the same as variables). - Format constant patterns with a leading `const`. - Migrated over a bunch of other patterns tests that can be run now. To handle if-case and when clauses, I ended up creating a new IfCasePiece since it because different enough from AssignPiece to not make sense to share code any more. This doesn't add support for guard clauses in switch cases yet. I'll do that separately.
1 parent 25dc786 commit 8040e06

14 files changed

+706
-105
lines changed

‎lib/src/ast_extensions.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,7 @@ extension PatternExtensions on DartPattern {
388388
///
389389
/// See [ExpressionExtensions.canBlockSplit].
390390
bool get canBlockSplit => switch (this) {
391+
ConstantPattern(:var expression) => expression.canBlockSplit,
391392
ListPattern(:var elements, :var rightBracket) =>
392393
elements.canSplit(rightBracket),
393394
MapPattern(:var elements, :var rightBracket) =>

‎lib/src/front_end/ast_node_visitor.dart

Lines changed: 18 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -220,18 +220,6 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
220220
return ChainBuilder(this, node).build();
221221
}
222222

223-
@override
224-
Piece visitCaseClause(CaseClause node) {
225-
return buildPiece((b) {
226-
b.token(node.caseKeyword);
227-
if (node.guardedPattern.whenClause != null) {
228-
throw UnimplementedError();
229-
}
230-
b.space();
231-
b.visit(node.guardedPattern.pattern);
232-
});
233-
}
234-
235223
@override
236224
Piece visitCastPattern(CastPattern node) {
237225
throw UnimplementedError();
@@ -362,8 +350,10 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
362350

363351
@override
364352
Piece visitConstantPattern(ConstantPattern node) {
365-
if (node.constKeyword != null) throw UnimplementedError();
366-
return nodePiece(node.expression);
353+
return buildPiece((b) {
354+
b.token(node.constKeyword, spaceAfter: true);
355+
b.visit(node.expression);
356+
});
367357
}
368358

369359
@override
@@ -453,15 +443,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
453443

454444
@override
455445
Piece visitDeclaredVariablePattern(DeclaredVariablePattern node) {
456-
var header = buildPiece((b) {
457-
b.modifier(node.keyword);
458-
b.visit(node.type);
459-
});
460-
return VariablePiece(
461-
header,
462-
[tokenPiece(node.name)],
463-
hasType: node.type != null,
464-
);
446+
return createPatternVariable(node.keyword, node.type, node.name);
465447
}
466448

467449
@override
@@ -866,8 +848,12 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
866848

867849
var condition = buildPiece((b) {
868850
b.token(precedingElse, spaceAfter: true);
869-
b.add(startControlFlow(ifElement.ifKeyword, ifElement.leftParenthesis,
870-
ifElement.expression, ifElement.rightParenthesis));
851+
b.add(createIfCondition(
852+
ifElement.ifKeyword,
853+
ifElement.leftParenthesis,
854+
ifElement.expression,
855+
ifElement.caseClause,
856+
ifElement.rightParenthesis));
871857

872858
// Make the `...` part of the header so that IfPiece can correctly
873859
// constrain the inner collection literal's ListPiece to split.
@@ -947,43 +933,12 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
947933
void traverse(Token? precedingElse, IfStatement ifStatement) {
948934
var condition = buildPiece((b) {
949935
b.token(precedingElse, spaceAfter: true);
950-
b.token(ifStatement.ifKeyword);
951-
b.space();
952-
b.token(ifStatement.leftParenthesis);
953-
954-
// If the condition needs to split, we prefer splitting before the
955-
// `case` keyword, like:
956-
//
957-
// if (obj
958-
// case 123456789012345678901234567890) {
959-
// body;
960-
// }
961-
var expressionPiece = nodePiece(ifStatement.expression);
962-
if (ifStatement.caseClause case var caseClause?) {
963-
var caseClausePiece = nodePiece(caseClause);
964-
// If the case clause can have block formatting, then a newline in
965-
// it doesn't force the if-case to split before the `case` keyword,
966-
// like:
967-
//
968-
// if (obj case [
969-
// first,
970-
// second,
971-
// third,
972-
// ]) {
973-
// ;
974-
// }
975-
var allowInnerSplit = caseClause.guardedPattern.pattern.canBlockSplit;
976-
b.add(AssignPiece(
977-
expressionPiece,
978-
caseClausePiece,
979-
allowInnerSplit: allowInnerSplit,
980-
indentInValue: true,
981-
));
982-
} else {
983-
b.add(expressionPiece);
984-
}
985-
986-
b.token(ifStatement.rightParenthesis);
936+
b.add(createIfCondition(
937+
ifStatement.ifKeyword,
938+
ifStatement.leftParenthesis,
939+
ifStatement.expression,
940+
ifStatement.caseClause,
941+
ifStatement.rightParenthesis));
987942
b.space();
988943
});
989944

@@ -1865,7 +1820,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
18651820

18661821
@override
18671822
Piece visitWildcardPattern(WildcardPattern node) {
1868-
throw UnimplementedError();
1823+
return createPatternVariable(node.keyword, node.type, node.name);
18691824
}
18701825

18711826
@override

‎lib/src/front_end/piece_factory.dart

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import '../ast_extensions.dart';
88
import '../piece/assign.dart';
99
import '../piece/clause.dart';
1010
import '../piece/function.dart';
11+
import '../piece/if_case.dart';
1112
import '../piece/infix.dart';
1213
import '../piece/list.dart';
1314
import '../piece/piece.dart';
@@ -458,6 +459,45 @@ mixin PieceFactory {
458459
isReturnTypeFunctionType: returnType is GenericFunctionType);
459460
}
460461

462+
/// Creates a piece for the header -- everything from the `if` keyword to the
463+
/// closing `)` -- of an if statement, if element, if-case statement, or
464+
/// if-case element.
465+
Piece createIfCondition(Token ifKeyword, Token leftParenthesis,
466+
Expression expression, CaseClause? caseClause, Token rightParenthesis) {
467+
return buildPiece((b) {
468+
b.token(ifKeyword);
469+
b.space();
470+
b.token(leftParenthesis);
471+
472+
var expressionPiece = nodePiece(expression);
473+
474+
if (caseClause != null) {
475+
var casePiece = buildPiece((b) {
476+
b.token(caseClause.caseKeyword);
477+
b.space();
478+
b.visit(caseClause.guardedPattern.pattern);
479+
});
480+
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+
}
489+
490+
b.add(IfCasePiece(expressionPiece, casePiece, guardPiece,
491+
canBlockSplitPattern:
492+
caseClause.guardedPattern.pattern.canBlockSplit));
493+
} else {
494+
b.add(expressionPiece);
495+
}
496+
497+
b.token(rightParenthesis);
498+
});
499+
}
500+
461501
/// Creates a [TryPiece] for try statement.
462502
Piece createTry(TryStatement tryStatement) {
463503
var piece = TryPiece();
@@ -691,6 +731,24 @@ mixin PieceFactory {
691731
return builder.build();
692732
}
693733

734+
/// Create a [VariablePiece] for a named or wildcard variable pattern.
735+
Piece createPatternVariable(
736+
Token? keyword, TypeAnnotation? type, Token name) {
737+
// If it's a wildcard with no declaration keyword or type, there is just a
738+
// name token.
739+
if (keyword == null && type == null) return tokenPiece(name);
740+
741+
var header = buildPiece((b) {
742+
b.modifier(keyword);
743+
b.visit(type);
744+
});
745+
return VariablePiece(
746+
header,
747+
[tokenPiece(name)],
748+
hasType: type != null,
749+
);
750+
}
751+
694752
/// Creates an [AdjacentPiece] for a given record type field.
695753
Piece createRecordTypeField(RecordTypeAnnotationField node) {
696754
// TODO(tall): Format metadata.

‎lib/src/piece/assign.dart

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -48,20 +48,8 @@ class AssignPiece extends Piece {
4848
/// split at the assignment operator.
4949
final bool _allowInnerSplit;
5050

51-
/// Whether there's an extra indent needed in the [value] piece when it
52-
/// splits, like:
53-
//
54-
// if (obj
55-
// case SomeLongTypeName
56-
// longVariableName) {
57-
// ;
58-
// }
59-
final bool _indentInValue;
60-
61-
AssignPiece(this.target, this.value,
62-
{bool allowInnerSplit = false, bool indentInValue = false})
63-
: _allowInnerSplit = allowInnerSplit,
64-
_indentInValue = indentInValue;
51+
AssignPiece(this.target, this.value, {bool allowInnerSplit = false})
52+
: _allowInnerSplit = allowInnerSplit;
6553

6654
// TODO(tall): The old formatter allows the first operand of a split
6755
// conditional expression to be on the same line as the `=`, as in:
@@ -112,18 +100,8 @@ class AssignPiece extends Piece {
112100

113101
writer.format(target);
114102
writer.splitIf(state == _atOperator);
115-
116-
// We need extra indentation when there's no inner splitting of the value.
117-
if (!_allowInnerSplit && _indentInValue) {
118-
writer.pushIndent(Indent.expression, canCollapse: true);
119-
}
120-
121103
writer.format(value);
122104

123-
if (!_allowInnerSplit && _indentInValue) {
124-
writer.popIndent();
125-
}
126-
127105
if (state != State.unsplit) writer.popIndent();
128106

129107
writer.popAllowNewlines();

‎lib/src/piece/if_case.dart

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
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 the contents inside the parentheses for an if-case statement or
10+
/// element: the expression, `case`, pattern, and `when` clause, if any.
11+
///
12+
/// They can split a few different ways:
13+
///
14+
/// [State.unsplit] All on one line:
15+
///
16+
/// if (obj case pattern when cond) ...
17+
///
18+
/// The pattern may also be block-formatted in this state:
19+
///
20+
/// if (obj case [
21+
/// subpattern,
22+
/// ] when cond) ...
23+
///
24+
/// [_beforeWhen] Split before the guard clause but not the pattern:
25+
///
26+
/// if (obj case pattern
27+
/// when cond) ...
28+
///
29+
/// [_beforeCase] Split before the `case` clause but not the guard:
30+
///
31+
/// if (obj
32+
/// case pattern when cond) ...
33+
///
34+
/// [_beforeCaseAndWhen] Split before both `case` and `when`:
35+
///
36+
/// if (obj
37+
/// case pattern
38+
/// when cond) ...
39+
class IfCasePiece extends Piece {
40+
/// Split before the `when` guard clause.
41+
static const State _beforeWhen = State(1);
42+
43+
/// Split before the `case` pattern clause.
44+
static const State _beforeCase = State(2);
45+
46+
/// Split before the `case` pattern clause and the `when` guard clause.
47+
static const State _beforeCaseAndWhen = State(3);
48+
49+
/// The value expression being matched.
50+
final Piece _value;
51+
52+
/// The pattern the value is matched against along with the leading `case`.
53+
final Piece _pattern;
54+
55+
/// If there is a `when` clause, that clause.
56+
final Piece? _guard;
57+
58+
/// Whether the pattern can be block formatted.
59+
final bool _canBlockSplitPattern;
60+
61+
IfCasePiece(this._value, this._pattern, this._guard,
62+
{required bool canBlockSplitPattern})
63+
: _canBlockSplitPattern = canBlockSplitPattern;
64+
65+
@override
66+
List<State> get additionalStates => [
67+
if (_guard != null) _beforeWhen,
68+
_beforeCase,
69+
if (_guard != null) _beforeCaseAndWhen
70+
];
71+
72+
@override
73+
void format(CodeWriter writer, State state) {
74+
var allowNewlineInValue = false;
75+
var allowNewlineInPattern = false;
76+
var allowNewlineInGuard = false;
77+
78+
switch (state) {
79+
case State.unsplit:
80+
// When not splitting before `case` or `when`, we only allow newlines
81+
// in block-formatted patterns.
82+
allowNewlineInPattern = _canBlockSplitPattern;
83+
84+
case _beforeWhen:
85+
// Allow newlines only in the guard if we split before `when`.
86+
allowNewlineInGuard = true;
87+
88+
case _beforeCase:
89+
// Only allow the guard on the same line as the pattern if it doesn't
90+
// split.
91+
allowNewlineInValue = true;
92+
allowNewlineInPattern = true;
93+
94+
case _beforeCaseAndWhen:
95+
allowNewlineInValue = true;
96+
allowNewlineInPattern = true;
97+
allowNewlineInGuard = true;
98+
}
99+
100+
if (state != State.unsplit) writer.pushIndent(Indent.expression);
101+
102+
writer.pushAllowNewlines(allowNewlineInValue);
103+
writer.format(_value);
104+
writer.popAllowNewlines();
105+
106+
// The case clause and pattern.
107+
writer.pushAllowNewlines(allowNewlineInPattern);
108+
writer.splitIf(state == _beforeCase || state == _beforeCaseAndWhen);
109+
110+
if (!_canBlockSplitPattern) {
111+
writer.pushIndent(Indent.expression, canCollapse: true);
112+
}
113+
114+
writer.format(_pattern);
115+
116+
if (!_canBlockSplitPattern) writer.popIndent();
117+
writer.popAllowNewlines();
118+
119+
// The guard clause.
120+
if (_guard case var guard?) {
121+
writer.pushAllowNewlines(allowNewlineInGuard);
122+
writer.splitIf(state == _beforeWhen || state == _beforeCaseAndWhen);
123+
writer.format(guard);
124+
writer.popAllowNewlines();
125+
}
126+
127+
if (state != State.unsplit) writer.popIndent();
128+
}
129+
130+
@override
131+
void forEachChild(void Function(Piece piece) callback) {
132+
callback(_value);
133+
callback(_pattern);
134+
if (_guard case var guard?) callback(guard);
135+
}
136+
}

0 commit comments

Comments
 (0)