Refactor ListPiece and DelimitedListBuilder. (#1295)

Refactor ListPiece and DelimitedListBuilder.

These classes started out just supporting argument lists, then grew to
collection literals, then parameter lists, then switch expressions,
and soon for loop headers.

Each time, I need to tweak how it works a little. (But not enough to
not use it completely, since the comment handling is complex and is
the same across all of those constructors.)

There was getting to be an unwieldy set of options being passed around.
Cleaned that up by defining a separate ListStyle that contains those
options and can be shuttled directly from DelimitedListBuilder to
ListPiece.

I made the before and after bracket pieces optional since I'll be using ListPiece in a future PR for the updaters list in a for loop, which isn't delimited.

Did a bunch of other minor clean-up.

Also, many of the docs were out of date. Fixed those.

Author: munificent

lib/src/back_end/code_writer.dart

@@ -126,13 +126,14 @@ class CodeWriter {
   ///
   /// If [space] is `true` and [condition] is `false`, writes a space.
   ///
+  /// If [blank] is `true`, writes an extra newline to produce a blank line.
+  ///
   /// If [indent] is given, sets the amount of block-level indentation for this
   /// and all subsequent newlines to [indent].
-  void splitIf(bool condition, {bool space = true, int? indent}) {
-    if (indent != null) setIndent(indent);
-
+  void splitIf(bool condition,
+      {bool space = true, bool blank = false, int? indent}) {
     if (condition) {
-      newline();
+      newline(blank: blank, indent: indent);
     } else if (space) {
       this.space();
     }

lib/src/front_end/ast_node_visitor.dart

@@ -11,6 +11,7 @@ import '../piece/block.dart';
 import '../piece/do_while.dart';
 import '../piece/if.dart';
 import '../piece/infix.dart';
+import '../piece/list.dart';
 import '../piece/piece.dart';
 import '../piece/variable.dart';
 import '../source_code.dart';
@@ -410,7 +411,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
         builder.leftDelimiter(node.leftDelimiter!);
       }
 
-      builder.add(node.parameters[i]);
+      builder.visit(node.parameters[i]);
     }
 
     builder.rightBracket(node.rightParenthesis, delimiter: node.rightDelimiter);
@@ -930,15 +931,16 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitSwitchExpression(SwitchExpression node) {
-    var list = DelimitedListBuilder.switchBody(this);
+    var list = DelimitedListBuilder(this,
+        const ListStyle(spaceWhenUnsplit: true, splitListIfBeforeSplits: true));
 
     createSwitchValue(node.switchKeyword, node.leftParenthesis, node.expression,
         node.rightParenthesis);
     writer.space();
     list.leftBracket(node.leftBracket);
 
     for (var member in node.cases) {
-      list.add(member);
+      list.visit(member);
     }
 
     list.rightBracket(node.rightBracket);

lib/src/front_end/delimited_list_builder.dart

@@ -20,7 +20,8 @@ import 'piece_factory.dart';
 class DelimitedListBuilder {
   final PieceFactory _visitor;
 
-  late final Piece _leftBracket;
+  /// The opening bracket before the elements, if any.
+  Piece? _leftBracket;
 
   /// The list of elements in the list.
   final List<ListElement> _elements = [];
@@ -29,83 +30,21 @@ class DelimitedListBuilder {
   /// next piece.
   final Set<ListElement> _blanksAfter = {};
 
-  late final Piece _rightBracket;
+  /// The closing bracket after the elements, if any.
+  Piece? _rightBracket;
 
-  /// Whether this list should have a trailing comma if it splits.
-  ///
-  /// This is true for most lists but false for type parameters, type arguments,
-  /// and switch values.
-  final bool _trailingComma;
-
-  /// The cost of splitting this list. Normally 1, but higher for some lists
-  /// that look worse when split.
-  final int _splitCost;
-
-  /// Whether this list should have spaces inside the bracket when it doesn't
-  /// split.
-  ///
-  /// This is false for most lists, but true for switch expression bodies:
-  ///
-  /// ```
-  /// v = switch (e) { 1 => 'one', 2 => 'two' };
-  /// //              ^                      ^
-  /// ```
-  final bool _spaceWhenUnsplit;
-
-  /// Whether a split in the [_before] piece should force the list to split too.
-  ///
-  /// See [ListPiece._splitListIfBeforeSplits] for more details.
-  final bool _splitListIfBeforeSplits;
+  final ListStyle _style;
 
   /// The list of comments following the most recently written element before
   /// any comma following the element.
   CommentSequence _commentsBeforeComma = CommentSequence.empty;
 
   /// Creates a new [DelimitedListBuilder] for an argument list, collection
   /// literal, etc.
-  DelimitedListBuilder(this._visitor)
-      : _trailingComma = true,
-        _splitCost = 1,
-        _spaceWhenUnsplit = false,
-        _splitListIfBeforeSplits = false;
-
-  /// Creates a new [DelimitedListBuilder] for a switch expression body.
-  DelimitedListBuilder.switchBody(this._visitor)
-      : _trailingComma = true,
-        _splitCost = 1,
-        _spaceWhenUnsplit = true,
-        _splitListIfBeforeSplits = true;
-
-  /// Creates a new [DelimitedListBuilder] for the value part of a switch
-  /// statement or expression:
-  ///
-  /// ```
-  /// switch (value) { ... }
-  /// //     ^^^^^^^
-  /// ```
-  DelimitedListBuilder.switchValue(this._visitor)
-      : _trailingComma = false,
-        _splitCost = 2,
-        _spaceWhenUnsplit = false,
-        _splitListIfBeforeSplits = false;
-
-  /// Creates a new [DelimitedListBuilder] for a type argument or type parameter
-  /// list.
-  DelimitedListBuilder.type(this._visitor)
-      : _trailingComma = false,
-        _splitCost = 2,
-        _spaceWhenUnsplit = false,
-        _splitListIfBeforeSplits = false;
-
-  ListPiece build() => ListPiece(
-      _leftBracket,
-      _elements,
-      _blanksAfter,
-      _rightBracket,
-      _splitCost,
-      _trailingComma,
-      _spaceWhenUnsplit,
-      _splitListIfBeforeSplits);
+  DelimitedListBuilder(this._visitor, [this._style = const ListStyle()]);
+
+  ListPiece build() =>
+      ListPiece(_leftBracket, _elements, _blanksAfter, _rightBracket, _style);
 
   /// Adds the opening [bracket] to the built list.
   ///
@@ -163,24 +102,37 @@ class DelimitedListBuilder {
     _rightBracket = _visitor.writer.pop();
   }
 
-  /// Adds [element] to the built list.
+  /// Adds [piece] to the built list.
   ///
-  /// Includes any comments that appear before element. Also includes the
-  /// subsequent comma, if any, and any comments that precede the comma.
-  void add(AstNode element) {
+  /// Use this when the piece is composed of more than one [AstNode] or [Token]
+  /// and [visit()] can't be used. When calling this, make sure to call
+  /// [addCommentsBefore()] for the first token in the [piece].
+  ///
+  /// Assumes there is no comma after this piece.
+  void add(Piece piece) {
+    _elements.add(ListElement(piece));
+    _commentsBeforeComma = CommentSequence.empty;
+  }
+
+  /// Writes any comments appearing before [token] to the list.
+  void addCommentsBefore(Token token) {
     // Handle comments between the preceding element and this one.
-    var commentsBeforeElement = _visitor.takeCommentsBefore(element.beginToken);
+    var commentsBeforeElement = _visitor.takeCommentsBefore(token);
     _addComments(commentsBeforeElement, hasElementAfter: true);
+  }
+
+  /// Adds [element] to the built list.
+  void visit(AstNode element) {
+    // Handle comments between the preceding element and this one.
+    addCommentsBefore(element.beginToken);
 
     // Traverse the element itself.
     _visitor.visit(element);
-    _elements.add(ListElement(_visitor.writer.pop()));
     _visitor.writer.split();
+    add(_visitor.writer.pop());
 
     var nextToken = element.endToken.next!;
-    if (nextToken.lexeme != ',') {
-      _commentsBeforeComma = CommentSequence.empty;
-    } else {
+    if (nextToken.lexeme == ',') {
       _commentsBeforeComma = _visitor.takeCommentsBefore(nextToken);
     }
   }
@@ -256,7 +208,7 @@ class DelimitedListBuilder {
     }
 
     // Comments that are neither hanging nor leading are treated like their own
-    // arguments.
+    // elements.
     for (var i = 0; i < separateComments.length; i++) {
       var comment = separateComments[i];
       if (separateComments.linesBefore(i) > 1 && _elements.isNotEmpty) {
@@ -268,7 +220,7 @@ class DelimitedListBuilder {
       _visitor.writer.split();
     }
 
-    // Leading comments are written before the next argument.
+    // Leading comments are written before the next element.
     for (var comment in leadingComments) {
       _visitor.writer.writeComment(comment);
       _visitor.writer.space();
@@ -311,15 +263,15 @@ class DelimitedListBuilder {
     CommentSequence leading
   }) _splitCommaComments(CommentSequence commentsBeforeElement,
       {required bool hasElementAfter}) {
-    // If we're on the final comma after the last argument, the comma isn't
+    // If we're on the final comma after the last element, the comma isn't
     // meaningful because there can't be leading comments after it.
     if (!hasElementAfter) {
       _commentsBeforeComma =
           _commentsBeforeComma.concatenate(commentsBeforeElement);
       commentsBeforeElement = CommentSequence.empty;
     }
 
-    // Edge case: A line comment on the same line as the preceding argument
+    // Edge case: A line comment on the same line as the preceding element
     // but after the comma is treated as hanging.
     if (commentsBeforeElement.isNotEmpty &&
         commentsBeforeElement[0].type == CommentType.line &&

lib/src/front_end/piece_factory.dart

@@ -11,6 +11,7 @@ import '../piece/function.dart';
 import '../piece/if.dart';
 import '../piece/import.dart';
 import '../piece/infix.dart';
+import '../piece/list.dart';
 import '../piece/piece.dart';
 import '../piece/postfix.dart';
 import 'ast_node_visitor.dart';
@@ -357,10 +358,11 @@ mixin PieceFactory implements CommentWriter {
 
   /// Creates a [ListPiece] for the given bracket-delimited set of elements.
   void createList(
-      Token leftBracket, Iterable<AstNode> elements, Token rightBracket) {
-    var builder = DelimitedListBuilder(this);
+      Token leftBracket, Iterable<AstNode> elements, Token rightBracket,
+      {ListStyle style = const ListStyle()}) {
+    var builder = DelimitedListBuilder(this, style);
     builder.leftBracket(leftBracket);
-    elements.forEach(builder.add);
+    elements.forEach(builder.visit);
     builder.rightBracket(rightBracket);
     writer.push(builder.build());
   }
@@ -370,14 +372,15 @@ mixin PieceFactory implements CommentWriter {
       Expression value, Token rightParenthesis) {
     // Format like an argument list since it is an expression surrounded by
     // parentheses.
-    var builder = DelimitedListBuilder.switchValue(this);
+    var builder = DelimitedListBuilder(
+        this, const ListStyle(commas: Commas.none, splitCost: 2));
 
     // Attach the `switch ` as part of the `(`.
     token(switchKeyword);
     writer.space();
 
     builder.leftBracket(leftParenthesis);
-    builder.add(value);
+    builder.visit(value);
     builder.rightBracket(rightParenthesis);
 
     writer.push(builder.build());
@@ -386,11 +389,8 @@ mixin PieceFactory implements CommentWriter {
   /// Creates a [ListPiece] for a type argument or type parameter list.
   void createTypeList(
       Token leftBracket, Iterable<AstNode> elements, Token rightBracket) {
-    var builder = DelimitedListBuilder.type(this);
-    builder.leftBracket(leftBracket);
-    elements.forEach(builder.add);
-    builder.rightBracket(rightBracket);
-    writer.push(builder.build());
+    return createList(leftBracket, elements, rightBracket,
+        style: const ListStyle(commas: Commas.nonTrailing, splitCost: 2));
   }
 
   /// Writes the parts of a formal parameter shared by all formal parameter