Skip to content

Commit cb61086

Browse files
committed
Reorganize the code for selecting a block argument.
To determine which argument list in an argument list should be block formatted, if any, we need to look at all of them. We can't do it eagerly because the heuristics are generally of the form "If only one argument is like X, then it is block formatted." But before this commit, DelimitedListBuilder never saw all arguments at once. It was given them one at a time. That meant that it had to stuff the information needed to select a block argument (BlockFormat) into ListPiece. This is kind of dumb because once we've selected a block argument, that data is no longer needed, but ListPiece lives all the way through the rest of the formatting process. Fortunately, the only expressions that support block arguments are argument lists, and those *do* add all of the arguments to the DelimitedListBuilder at once. So this commit adds a method to DelimitedListBuilder to pass them all in. There are no behavior changes in this commit. This is just rearranging code (and saving a bit of memory) to get ready for tweaking the heuristics.
1 parent 58e96a9 commit cb61086

File tree

3 files changed

+58
-46
lines changed

3 files changed

+58
-46
lines changed

lib/src/front_end/delimited_list_builder.dart

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,6 @@ class DelimitedListBuilder {
6262
});
6363
}
6464

65-
if (_style.allowBlockElement) _setBlockElementFormatting();
66-
6765
var piece =
6866
ListPiece(_leftBracket, _elements, _blanksAfter, _rightBracket, _style);
6967
if (_mustSplit || forceSplit) piece.pin(State.split);
@@ -134,8 +132,8 @@ class DelimitedListBuilder {
134132
/// [addCommentsBefore()] for the first token in the [piece].
135133
///
136134
/// Assumes there is no comma after this piece.
137-
void add(Piece piece, [BlockFormat format = BlockFormat.none]) {
138-
_elements.add(ListElementPiece(_leadingComments, piece, format));
135+
void add(Piece piece) {
136+
_elements.add(ListElementPiece(_leadingComments, piece));
139137
_leadingComments.clear();
140138
_commentsBeforeComma = CommentSequence.empty;
141139
}
@@ -152,25 +150,33 @@ class DelimitedListBuilder {
152150
// Handle comments between the preceding element and this one.
153151
addCommentsBefore(element.firstNonCommentToken);
154152

155-
// See if it's an expression that supports block formatting.
156-
var format = switch (element) {
157-
AdjacentStrings(indentStrings: true) =>
158-
BlockFormat.indentedAdjacentStrings,
159-
AdjacentStrings() => BlockFormat.unindentedAdjacentStrings,
160-
Expression() => element.blockFormatType,
161-
DartPattern() when element.canBlockSplit => BlockFormat.collection,
162-
_ => BlockFormat.none,
163-
};
164-
165153
// Traverse the element itself.
166-
add(_visitor.nodePiece(element), format);
154+
add(_visitor.nodePiece(element));
167155

168156
var nextToken = element.endToken.next!;
169157
if (nextToken.lexeme == ',') {
170158
_commentsBeforeComma = _visitor.comments.takeCommentsBefore(nextToken);
171159
}
172160
}
173161

162+
/// Visits a list of [elements].
163+
///
164+
/// If [allowBlockArgument] is `true`, then allows one element to receive
165+
/// block formatting if appropriate, as in:
166+
///
167+
/// function(argument, [
168+
/// block,
169+
/// like,
170+
/// ], argument);
171+
void visitAll(List<AstNode> elements, {bool allowBlockArgument = false}) {
172+
for (var i = 0; i < elements.length; i++) {
173+
var element = elements[i];
174+
visit(element);
175+
}
176+
177+
if (allowBlockArgument) _setBlockArgument(elements);
178+
}
179+
174180
/// Inserts an inner left delimiter between two elements.
175181
///
176182
/// This is used for parameter lists when there are both mandatory and
@@ -398,6 +404,14 @@ class DelimitedListBuilder {
398404
);
399405
}
400406

407+
/// Given an argument list, determines which if any of the arguments should
408+
/// get special block-like formatting as in the list literal in:
409+
///
410+
/// function(argument, [
411+
/// block,
412+
/// like,
413+
/// ], argument);
414+
///
401415
/// Looks at the [BlockFormat] types of all of the elements to determine if
402416
/// one of them should be block formatted.
403417
///
@@ -426,14 +440,29 @@ class DelimitedListBuilder {
426440
///
427441
/// Stores the result of this calculation by setting flags on the
428442
/// [ListElement]s.
429-
void _setBlockElementFormatting() {
430-
// TODO(tall): These heuristics will probably need some iteration.
443+
void _setBlockArgument(List<AstNode> arguments) {
444+
var firstIsUnindentedAdjacentStrings = false;
431445
var functions = <int>[];
432446
var collections = <int>[];
433447
var adjacentStrings = <int>[];
434448

435-
for (var i = 0; i < _elements.length; i++) {
436-
switch (_elements[i].blockFormat) {
449+
for (var i = 0; i < arguments.length; i++) {
450+
var argument = arguments[i];
451+
// See if it's an expression that supports block formatting.
452+
var format = switch (argument) {
453+
AdjacentStrings(indentStrings: true) =>
454+
BlockFormat.indentedAdjacentStrings,
455+
AdjacentStrings() => BlockFormat.unindentedAdjacentStrings,
456+
Expression() => argument.blockFormatType,
457+
DartPattern() when argument.canBlockSplit => BlockFormat.collection,
458+
_ => BlockFormat.none,
459+
};
460+
461+
if (i == 0 && format == BlockFormat.unindentedAdjacentStrings) {
462+
firstIsUnindentedAdjacentStrings = true;
463+
}
464+
465+
switch (format) {
437466
case BlockFormat.function:
438467
functions.add(i);
439468
case BlockFormat.collection:
@@ -471,6 +500,7 @@ class DelimitedListBuilder {
471500
}
472501
}
473502

503+
// TODO(tall): These heuristics will probably need some iteration.
474504
switch ((functions, collections, adjacentStrings)) {
475505
// Only allow block formatting in an argument list containing adjacent
476506
// strings when:
@@ -487,7 +517,7 @@ class DelimitedListBuilder {
487517
case ([1], _, [0]):
488518
// The adjacent strings.
489519
_elements[0].allowNewlinesWhenUnsplit = true;
490-
if (_elements[0].blockFormat == BlockFormat.unindentedAdjacentStrings) {
520+
if (firstIsUnindentedAdjacentStrings) {
491521
_elements[0].indentWhenBlockFormatted = true;
492522
}
493523

@@ -503,8 +533,5 @@ class DelimitedListBuilder {
503533
case ([], [var element], _):
504534
_elements[element].allowNewlinesWhenUnsplit = true;
505535
}
506-
507-
// If we get here, there are no block element, or it's ambiguous as to
508-
// which one should be it so none are.
509536
}
510537
}

lib/src/front_end/piece_factory.dart

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ mixin PieceFactory {
123123
leftBracket: leftBracket,
124124
elements,
125125
rightBracket: rightBracket,
126-
style: const ListStyle(allowBlockElement: true));
126+
allowBlockArgument: true);
127127
}
128128

129129
/// Writes a bracket-delimited block or declaration body.
@@ -974,7 +974,8 @@ mixin PieceFactory {
974974
{required Token leftBracket,
975975
required Token rightBracket,
976976
ListStyle style = const ListStyle(),
977-
bool preserveNewlines = false}) {
977+
bool preserveNewlines = false,
978+
bool allowBlockArgument = false}) {
978979
// If the list is completely empty, write the brackets directly inline so
979980
// that we create fewer pieces.
980981
if (!elements.canSplit(rightBracket)) {
@@ -990,7 +991,7 @@ mixin PieceFactory {
990991
if (preserveNewlines && elements.containsLineComments(rightBracket)) {
991992
_preserveNewlinesInCollection(elements, builder);
992993
} else {
993-
elements.forEach(builder.visit);
994+
builder.visitAll(elements, allowBlockArgument: allowBlockArgument);
994995
}
995996

996997
builder.rightBracket(rightBracket);

lib/src/piece/list.dart

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -252,9 +252,6 @@ final class ListElementPiece extends Piece {
252252

253253
final Piece? _content;
254254

255-
/// What kind of block formatting can be applied to this element.
256-
final BlockFormat blockFormat;
257-
258255
/// Whether newlines are allowed in this element when this list is unsplit.
259256
///
260257
/// This is generally only true for a single "block" element, as in:
@@ -312,16 +309,13 @@ final class ListElementPiece extends Piece {
312309
/// delimiter (here `,` and 2).
313310
int _commentsBeforeDelimiter = 0;
314311

315-
ListElementPiece(
316-
List<Piece> leadingComments, Piece element, BlockFormat format)
312+
ListElementPiece(List<Piece> leadingComments, Piece element)
317313
: _leadingComments = [...leadingComments],
318-
_content = element,
319-
blockFormat = format;
314+
_content = element;
320315

321316
ListElementPiece.comment(Piece comment)
322317
: _leadingComments = const [],
323-
_content = null,
324-
blockFormat = BlockFormat.none {
318+
_content = null {
325319
_hangingComments.add(comment);
326320
}
327321

@@ -459,18 +453,8 @@ class ListStyle {
459453
/// // ^ ^
460454
final bool spaceWhenUnsplit;
461455

462-
/// Whether an element in the list is allowed to have block-like formatting,
463-
/// as in:
464-
///
465-
/// function(argument, [
466-
/// block,
467-
/// like,
468-
/// ], argument);
469-
final bool allowBlockElement;
470-
471456
const ListStyle(
472457
{this.commas = Commas.trailing,
473458
this.splitCost = Cost.normal,
474-
this.spaceWhenUnsplit = false,
475-
this.allowBlockElement = false});
459+
this.spaceWhenUnsplit = false});
476460
}

0 commit comments

Comments
 (0)