Skip to content

Commit fe55b0d

Browse files
authored
Format map and set literals. (#1283)
I'm formatting map entries the same way that assignments and variable declarations are handled, so that we get nice "block-like" formatting for delimited expressions, as in: ``` var map = { list: [ 1, 2, ] }; ``` While I was at it, I also updated named arguments to do the same thing: ``` function( list: [ 1, 2, ] ); ``` Also handle the "const" keyword on lists. I missed that when adding initial support for lists.
1 parent f1788b5 commit fe55b0d

File tree

13 files changed

+386
-46
lines changed

13 files changed

+386
-46
lines changed

lib/src/ast_extensions.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ extension ExpressionExtensions on Expression {
113113
ParenthesizedExpression(:var expression) => expression.isDelimited,
114114
ListLiteral() => true,
115115
MethodInvocation() => true,
116-
// TODO(tall): Map and set literals.
116+
SetOrMapLiteral() => true,
117117
// TODO(tall): Record literals.
118118
// TODO(tall): Instance creation expressions (`new` and `const`).
119119
// TODO(tall): Switch expressions.

lib/src/front_end/ast_node_visitor.dart

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -526,28 +526,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
526526

527527
@override
528528
void visitListLiteral(ListLiteral node) {
529-
visit(node.typeArguments);
530-
531-
var builder = DelimitedListBuilder(this);
532-
builder.leftBracket(node.leftBracket);
533-
534-
// TODO(tall): Support a line comment inside a list literal as a signal to
535-
// preserve internal newlines. So if you have:
536-
//
537-
// ```
538-
// var list = [
539-
// 1, 2, 3, // comment
540-
// 4, 5, 6,
541-
// ];
542-
// ```
543-
//
544-
// The formatter will preserve the newline after element 3 and the lack of
545-
// them after the other elements.
546-
547-
node.elements.forEach(builder.add);
548-
549-
builder.rightBracket(node.rightBracket);
550-
writer.push(builder.build());
529+
createCollection(node, node.leftBracket, node.elements, node.rightBracket);
551530
}
552531

553532
@override
@@ -567,7 +546,8 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
567546

568547
@override
569548
void visitMapLiteralEntry(MapLiteralEntry node) {
570-
throw UnimplementedError();
549+
visit(node.key);
550+
finishAssignment(node.separator, node.value);
571551
}
572552

573553
@override
@@ -611,9 +591,8 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
611591

612592
@override
613593
void visitNamedExpression(NamedExpression node) {
614-
visit(node.name);
615-
writer.space();
616-
visit(node.expression);
594+
visit(node.name.label);
595+
finishAssignment(node.name.colon, node.expression);
617596
}
618597

619598
@override
@@ -805,7 +784,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
805784

806785
@override
807786
void visitSetOrMapLiteral(SetOrMapLiteral node) {
808-
throw UnimplementedError();
787+
createCollection(node, node.leftBracket, node.elements, node.rightBracket);
809788
}
810789

811790
@override
@@ -920,7 +899,9 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
920899
@override
921900
void visitVariableDeclaration(VariableDeclaration node) {
922901
token(node.name);
923-
finishAssignment(node.equals, node.initializer);
902+
if ((node.equals, node.initializer) case (var equals?, var initializer?)) {
903+
finishAssignment(equals, initializer);
904+
}
924905
}
925906

926907
@override

lib/src/front_end/piece_factory.dart

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import '../piece/piece.dart';
1313
import '../piece/postfix.dart';
1414
import 'ast_node_visitor.dart';
1515
import 'comment_writer.dart';
16+
import 'delimited_list_builder.dart';
1617
import 'piece_writer.dart';
1718
import 'sequence_builder.dart';
1819

@@ -74,6 +75,34 @@ mixin PieceFactory implements CommentWriter {
7475
alwaysSplit: nodes.isNotEmpty));
7576
}
7677

78+
/// Creates a [ListPiece] for a collection literal.
79+
void createCollection(TypedLiteral literal, Token leftBracket,
80+
List<AstNode> elements, Token rightBracket) {
81+
modifier(literal.constKeyword);
82+
visit(literal.typeArguments);
83+
84+
var builder = DelimitedListBuilder(this);
85+
builder.leftBracket(leftBracket);
86+
87+
// TODO(tall): Support a line comment inside a collection literal as a
88+
// signal to preserve internal newlines. So if you have:
89+
//
90+
// ```
91+
// var list = [
92+
// 1, 2, 3, // comment
93+
// 4, 5, 6,
94+
// ];
95+
// ```
96+
//
97+
// The formatter will preserve the newline after element 3 and the lack of
98+
// them after the other elements.
99+
100+
elements.forEach(builder.add);
101+
102+
builder.rightBracket(rightBracket);
103+
writer.push(builder.build());
104+
}
105+
77106
/// Creates metadata annotations for a directive.
78107
///
79108
/// Always forces the annotations to be on a previous line.
@@ -237,23 +266,22 @@ mixin PieceFactory implements CommentWriter {
237266
/// * Variable declaration
238267
/// * Constructor initializer
239268
///
240-
/// This method assumes the code to the left of the `=` has already been
241-
/// visited.
269+
/// This is also used for map literal entries and named arguments which are
270+
/// also sort of like bindings. In that case, [operator] is the `:`.
242271
///
243-
/// Does nothing if [equalsOperator] is `null`.
244-
void finishAssignment(Token? equalsOperator, Expression? rightHandSide) {
245-
if (equalsOperator == null) return;
246-
247-
writer.space();
248-
token(equalsOperator);
272+
/// This method assumes the code to the left of the `=` or `:` has already
273+
/// been visited.
274+
void finishAssignment(Token operator, Expression rightHandSide) {
275+
if (operator.type == TokenType.EQ) writer.space();
276+
token(operator);
249277
var target = writer.pop();
250278
writer.split();
251279

252280
visit(rightHandSide);
253281

254282
var initializer = writer.pop();
255283
writer.push(AssignPiece(target, initializer,
256-
isValueDelimited: rightHandSide!.isDelimited));
284+
isValueDelimited: rightHandSide.isDelimited));
257285
}
258286

259287
/// Writes an optional modifier that precedes other code.

lib/src/front_end/piece_writer.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ class PieceWriter {
152152
if (!hanging) _pendingSplit = false;
153153
}
154154

155-
/// Writes a mandatory newline from a comment in the current [TextPiece].
155+
/// Writes a mandatory newline from a comment to the current [TextPiece].
156156
void writeNewline() {
157157
_pendingNewline = true;
158158
}

lib/src/piece/assign.dart

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,13 @@ import '../constants.dart';
66
import 'piece.dart';
77

88
/// A piece for any construct where `=` is followed by an expression: variable
9-
/// initializer, assignment, constructor initializer, etc. Assignments can be
10-
/// formatted three ways:
9+
/// initializer, assignment, constructor initializer, etc.
10+
///
11+
/// This piece is also used for map entries and named arguments where `:` is
12+
/// followed by an expression or element because those also want to support the
13+
/// "block-like" formatting of delimited expressions on the right.
14+
///
15+
/// These constructs can be formatted three ways:
1116
///
1217
/// [State.initial] No split at all:
1318
///
@@ -59,15 +64,15 @@ class AssignPiece extends Piece {
5964

6065
@override
6166
void format(CodeWriter writer, State state) {
62-
writer.format(target);
63-
64-
// A split inside the value forces splitting at the "=" unless it's a
65-
// delimited expression.
67+
// A split in either child piece forces splitting after the "=" unless it's
68+
// a delimited expression.
6669
if (state == State.initial) writer.setAllowNewlines(false);
6770

6871
// Don't indent a split delimited expression.
6972
if (state != _insideValue) writer.setIndent(Indent.expression);
7073

74+
writer.format(target);
75+
7176
writer.splitIf(state == _atEquals);
7277
writer.format(value);
7378
}

test/expression/list.stmt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
[];
44
<<<
55
[];
6+
>>> Const.
7+
const [ 1 , 2 , 3 ] ;
8+
<<<
9+
const [1, 2, 3];
610
>>> Exactly page width.
711
[ first , second , third , fourth , seventh ] ;
812
<<<
@@ -69,6 +73,15 @@
6973
< int > [ 1 , 2 , 3 ];
7074
<<<
7175
<int>[1, 2, 3];
76+
>>> Split list but not type arguments.
77+
<Map<int, String>>
78+
[firstElement, secondElement, thirdElement];
79+
<<<
80+
<Map<int, String>>[
81+
firstElement,
82+
secondElement,
83+
thirdElement,
84+
];
7285
>>> Split type arguments but not list.
7386
<Map<VeryLongTypeName, AnotherLongTypeName>>[1, 2, 3];
7487
<<<

test/expression/map.stmt

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
40 columns |
2+
>>> Empty.
3+
( { } );
4+
<<<
5+
({});
6+
>>> Const.
7+
const { first : 1 , second : 2 } ;
8+
<<<
9+
const {first: 1, second: 2};
10+
>>> Exactly page width.
11+
( { first : 1 , second : 2 , third : 3 , f : 4 } );
12+
<<<
13+
({first: 1, second: 2, third: 3, f: 4});
14+
>>> Split.
15+
({first: 1, second: 2, third: 3, fourth: 4,});
16+
<<<
17+
({
18+
first: 1,
19+
second: 2,
20+
third: 3,
21+
fourth: 4,
22+
});
23+
>>> Add trailing comma if split.
24+
({first: 1, second: 2, third: 3, fourth: 4});
25+
<<<
26+
({
27+
first: 1,
28+
second: 2,
29+
third: 3,
30+
fourth: 4,
31+
});
32+
>>> Remove trailing comma if unsplit.
33+
({first: 1, second: 2, third: 3,});
34+
<<<
35+
({first: 1, second: 2, third: 3});
36+
>>> Prefer to split entry instead of key.
37+
({
38+
first + second + third + fourth: fifth
39+
});
40+
<<<
41+
({
42+
first + second + third + fourth:
43+
fifth,
44+
});
45+
>>> Prefer to split entry instead of value.
46+
({
47+
first: second + third + fourth + fifth
48+
});
49+
<<<
50+
({
51+
first:
52+
second + third + fourth + fifth,
53+
});
54+
>>> Split in key forces entry to split.
55+
({
56+
first + second + third + fourth + fifth: sixth
57+
});
58+
<<<
59+
({
60+
first +
61+
second +
62+
third +
63+
fourth +
64+
fifth:
65+
sixth,
66+
});
67+
>>> Split in value forces entry to split.
68+
({
69+
first: second + third + fourth + fifth + sixth
70+
});
71+
<<<
72+
({
73+
first:
74+
second +
75+
third +
76+
fourth +
77+
fifth +
78+
sixth,
79+
});
80+
>>> Remove blank lines around entries.
81+
({
82+
83+
84+
firstElement: 1,
85+
86+
87+
88+
secondElement: 2,
89+
90+
91+
92+
thirdElement: 3
93+
94+
95+
});
96+
<<<
97+
({
98+
firstElement: 1,
99+
secondElement: 2,
100+
thirdElement: 3,
101+
});
102+
>>> With type arguments.
103+
< int , String > { 1 : 'one' , 2 : 'two' };
104+
<<<
105+
<int, String>{1: 'one', 2: 'two'};
106+
>>> Split map but not type arguments.
107+
<int, String>{firstElement: 'one', secondElement: 'two'};
108+
<<<
109+
<int, String>{
110+
firstElement: 'one',
111+
secondElement: 'two',
112+
};
113+
>>> Split type arguments but not map.
114+
<VeryLongTypeName, AnotherReallyLongTypeName>{1: 'one', 2: 'two'};
115+
<<<
116+
<
117+
VeryLongTypeName,
118+
AnotherReallyLongTypeName
119+
>{1: 'one', 2: 'two'};
120+
>>> Split type arguments and map.
121+
<VeryLongTypeName, AnotherReallyLongTypeName>
122+
{1: 'value one', 2: 'value two', 3: 'value three'};
123+
<<<
124+
<
125+
VeryLongTypeName,
126+
AnotherReallyLongTypeName
127+
>{
128+
1: 'value one',
129+
2: 'value two',
130+
3: 'value three',
131+
};
132+
>>> Split in nested type argument.
133+
<List<NotSplit>, Map<VeryLongTypeName, AnotherLongTypeName>>{1: 'one', 2: 'two'};
134+
<<<
135+
<
136+
List<NotSplit>,
137+
Map<
138+
VeryLongTypeName,
139+
AnotherLongTypeName
140+
>
141+
>{1: 'one', 2: 'two'};

0 commit comments

Comments
 (0)