Combine SequencePiece and BlockPiece. (#1369)

Combine SequencePiece and BlockPiece.

SequencePiece is used whenever there is a series of AST nodes separated
by mandatory newlines: block bodies, members in class bodies, the top
level of a compilation unit, etc.

Since the top level of a compilation unit doesn't have any enclosing
curly braces or indentation, I originally designed it as two separate
pieces: SequencePiece has the series of elements and it can optionally
by wrapped in a BlockPiece that has the brackets and indentation.

It's a little weird, though, because BlockPiece is the one that decides
whether or not the contents split even though SequencePiece contains
the actual contents.

And it doesn't match ListPiece which uses a single unified Piece for
both the delimiters and contents.

So I changed it to be more like ListPiece which has both the brackets
and the contents, and the brackets are optional in places where they
aren't needed.

I think this makes the design a little more consistent and simpler.
(Also, I have some in-progress optimization work that *might* be easier
with SequencePiece organized this way.)

At some point, we might even be able to reuse code between SequencePiece
and ListPiece (and likewise Sequencebuilder and DelimitedListBuilder),
but I'm not worrying about that right now.

Co-authored-by: Nate Bosch <[email protected]>

Author: munificent

lib/src/front_end/ast_node_visitor.dart

@@ -12,7 +12,6 @@ import '../dart_formatter.dart';
 import '../piece/adjacent.dart';
 import '../piece/adjacent_strings.dart';
 import '../piece/assign.dart';
-import '../piece/block.dart';
 import '../piece/constructor.dart';
 import '../piece/for.dart';
 import '../piece/if.dart';
@@ -545,9 +544,9 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
 
       // If there are members, format it like a block where each constant and
       // member is on its own line.
-      var leftBracketPiece = tokenPiece(node.leftBracket);
-
       var sequence = SequenceBuilder(this);
+      sequence.leftBracket(node.leftBracket);
+
       for (var constant in node.constants) {
         sequence.addCommentsBefore(constant.firstNonCommentToken);
         sequence.add(createEnumConstant(constant,
@@ -567,13 +566,9 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
         if (node.hasNonEmptyBody) sequence.addBlank();
       }
 
-      // Place any comments before the "}" inside the block.
-      sequence.addCommentsBefore(node.rightBracket);
+      sequence.rightBracket(node.rightBracket);
 
-      var rightBracketPiece = tokenPiece(node.rightBracket);
-
-      builder.add(
-          BlockPiece(leftBracketPiece, sequence.build(), rightBracketPiece));
+      builder.add(sequence.build());
       return builder.build();
     }
   }
@@ -1661,57 +1656,54 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
 
   @override
   Piece visitSwitchStatement(SwitchStatement node) {
-    var leftBracket = buildPiece((b) {
+    return buildPiece((b) {
       b.add(startControlFlow(node.switchKeyword, node.leftParenthesis,
           node.expression, node.rightParenthesis));
       b.space();
-      b.token(node.leftBracket);
-    });
 
-    var sequence = SequenceBuilder(this);
-    for (var member in node.members) {
-      for (var label in member.labels) {
-        sequence.visit(label);
-      }
+      var sequence = SequenceBuilder(this);
+      sequence.leftBracket(node.leftBracket);
 
-      sequence.addCommentsBefore(member.keyword);
+      for (var member in node.members) {
+        for (var label in member.labels) {
+          sequence.visit(label);
+        }
 
-      var casePiece = buildPiece((b) {
-        b.token(member.keyword);
+        sequence.addCommentsBefore(member.keyword);
 
-        if (member is SwitchCase) {
-          b.space();
-          b.visit(member.expression);
-        } else if (member is SwitchPatternCase) {
-          if (member.guardedPattern.whenClause != null) {
-            throw UnimplementedError();
-          }
+        var casePiece = buildPiece((b) {
+          b.token(member.keyword);
 
-          b.space();
-          b.visit(member.guardedPattern.pattern);
-        } else {
-          assert(member is SwitchDefault);
-          // Nothing to do.
-        }
+          if (member is SwitchCase) {
+            b.space();
+            b.visit(member.expression);
+          } else if (member is SwitchPatternCase) {
+            if (member.guardedPattern.whenClause != null) {
+              throw UnimplementedError();
+            }
 
-        b.token(member.colon);
-      });
+            b.space();
+            b.visit(member.guardedPattern.pattern);
+          } else {
+            assert(member is SwitchDefault);
+            // Nothing to do.
+          }
 
-      // Don't allow any blank lines between the `case` line and the first
-      // statement in the case (or the next case if this case has no body).
-      sequence.add(casePiece, indent: Indent.none, allowBlankAfter: false);
+          b.token(member.colon);
+        });
 
-      for (var statement in member.statements) {
-        sequence.visit(statement, indent: Indent.block);
-      }
-    }
+        // Don't allow any blank lines between the `case` line and the first
+        // statement in the case (or the next case if this case has no body).
+        sequence.add(casePiece, indent: Indent.none, allowBlankAfter: false);
 
-    // Place any comments before the "}" inside the sequence.
-    sequence.addCommentsBefore(node.rightBracket);
-    var rightBracketPiece = tokenPiece(node.rightBracket);
+        for (var statement in member.statements) {
+          sequence.visit(statement, indent: Indent.block);
+        }
+      }
 
-    return BlockPiece(leftBracket, sequence.build(), rightBracketPiece,
-        alwaysSplit: node.members.isNotEmpty || sequence.mustSplit);
+      sequence.rightBracket(node.rightBracket);
+      b.add(sequence.build());
+    });
   }
 
   @override

lib/src/front_end/piece_factory.dart

@@ -6,13 +6,13 @@ import 'package:analyzer/dart/ast/token.dart';
 
 import '../ast_extensions.dart';
 import '../piece/assign.dart';
-import '../piece/block.dart';
 import '../piece/clause.dart';
 import '../piece/function.dart';
 import '../piece/infix.dart';
 import '../piece/list.dart';
 import '../piece/piece.dart';
 import '../piece/postfix.dart';
+import '../piece/sequence.dart';
 import '../piece/try.dart';
 import '../piece/type.dart';
 import '../piece/variable.dart';
@@ -64,8 +64,8 @@ mixin PieceFactory {
         style: const ListStyle(allowBlockElement: true));
   }
 
-  /// Creates a [BlockPiece] for a given bracket-delimited block or declaration
-  /// body.
+  /// Creates a [SequencePiece] for a given bracket-delimited block or
+  /// declaration body.
   ///
   /// If [forceSplit] is `true`, then the block will split even if empty. This
   /// is used, for example, with empty blocks in `if` statements followed by
@@ -76,9 +76,9 @@ mixin PieceFactory {
   Piece createBody(
       Token leftBracket, List<AstNode> contents, Token rightBracket,
       {bool forceSplit = false}) {
-    var leftBracketPiece = tokenPiece(leftBracket);
-
     var sequence = SequenceBuilder(this);
+    sequence.leftBracket(leftBracket);
+
     for (var node in contents) {
       sequence.visit(node);
 
@@ -87,16 +87,11 @@ mixin PieceFactory {
       if (node.hasNonEmptyBody) sequence.addBlank();
     }
 
-    // Place any comments before the "}" inside the block.
-    sequence.addCommentsBefore(rightBracket);
-
-    var rightBracketPiece = tokenPiece(rightBracket);
-
-    return BlockPiece(leftBracketPiece, sequence.build(), rightBracketPiece,
-        alwaysSplit: forceSplit || contents.isNotEmpty || sequence.mustSplit);
+    sequence.rightBracket(rightBracket);
+    return sequence.build(forceSplit: forceSplit);
   }
 
-  /// Creates a [BlockPiece] for a given [Block].
+  /// Creates a [SequencePiece] for a given [Block].
   ///
   /// If [forceSplit] is `true`, then the block will split even if empty. This
   /// is used, for example, with empty blocks in `if` statements followed by

lib/src/front_end/sequence_builder.dart

@@ -24,9 +24,15 @@ import 'piece_factory.dart';
 class SequenceBuilder {
   final PieceFactory _visitor;
 
+  /// The opening bracket before the elements, if any.
+  Piece? _leftBracket;
+
   /// The series of elements in the sequence.
   final List<SequenceElement> _elements = [];
 
+  /// The closing bracket after the elements, if any.
+  Piece? _rightBracket;
+
   /// Whether a blank line should be allowed after the current element.
   bool _allowBlank = false;
 
@@ -35,16 +41,39 @@ class SequenceBuilder {
   bool _mustSplit = false;
   bool get mustSplit => _mustSplit;
 
-  SequencePiece build() => SequencePiece(_elements);
+  SequencePiece build({bool forceSplit = false}) {
+    var piece = SequencePiece(_elements,
+        leftBracket: _leftBracket, rightBracket: _rightBracket);
+
+    if (mustSplit || forceSplit) piece.pin(State.split);
+
+    return piece;
+  }
+
+  /// Adds the opening [bracket] to the built sequence.
+  void leftBracket(Token bracket) {
+    _leftBracket = _visitor.tokenPiece(bracket);
+  }
+
+  /// Adds the closing [bracket] to the built sequence along with any comments
+  /// that precede it.
+  void rightBracket(Token bracket) {
+    // Place any comments before the bracket inside the block.
+    addCommentsBefore(bracket);
+    _rightBracket = _visitor.tokenPiece(bracket);
+  }
 
   /// Adds [piece] to this sequence.
   ///
   /// The caller should have already called [addCommentsBefore()] with the
   /// first token in [piece].
   void add(Piece piece, {int? indent, bool allowBlankAfter = true}) {
-    _elements.add(SequenceElement(indent ?? Indent.none, piece));
+    _add(indent ?? Indent.none, piece);
 
     _allowBlank = allowBlankAfter;
+
+    // There is at least one non-comment element in the sequence, so it splits.
+    _mustSplit = true;
   }
 
   /// Visits [node] and adds the resulting [Piece] to this sequence, handling
@@ -98,7 +127,7 @@ class SequenceBuilder {
         }
 
         // Write the comment as its own sequence piece.
-        add(comment);
+        _add(Indent.none, comment);
       }
     }
 
@@ -108,4 +137,11 @@ class SequenceBuilder {
     // Write a blank before the token if there should be one.
     if (comments.linesBeforeNextToken > 1) addBlank();
   }
+
+  void _add(int indent, Piece piece) {
+    // If there are brackets, add a level of block indentation.
+    if (_leftBracket != null) indent += Indent.block;
+
+    _elements.add(SequenceElement(indent, piece));
+  }
 }

lib/src/piece/block.dart

@@ -3,23 +3,40 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import '../back_end/code_writer.dart';
+import '../constants.dart';
 import 'piece.dart';
 
 /// A piece for a series of statements or members inside a block or declaration
 /// body.
 ///
 /// Usually constructed using a [SequenceBuilder].
 class SequencePiece extends Piece {
+  /// The opening delimiter, if any.
+  final Piece? _leftBracket;
+
   /// The series of members or statements.
   final List<SequenceElement> _elements;
 
-  SequencePiece(this._elements);
+  SequencePiece(this._elements, {Piece? leftBracket, Piece? rightBracket})
+      : _leftBracket = leftBracket,
+        _rightBracket = rightBracket;
+
+  /// The closing delimiter, if any.
+  final Piece? _rightBracket;
 
-  /// Whether this sequence has any contents.
-  bool get isNotEmpty => _elements.isNotEmpty;
+  @override
+  List<State> get additionalStates => [if (_elements.isNotEmpty) State.split];
 
   @override
   void format(CodeWriter writer, State state) {
+    writer.setAllowNewlines(state == State.split);
+
+    if (_leftBracket case var leftBracket?) {
+      writer.format(leftBracket);
+      writer.splitIf(state == State.split,
+          space: false, indent: _elements.firstOrNull?.indent ?? 0);
+    }
+
     for (var i = 0; i < _elements.length; i++) {
       var element = _elements[i];
       writer.format(element.piece);
@@ -34,16 +51,25 @@ class SequencePiece extends Piece {
             blank: element.blankAfter, indent: _elements[i + 1].indent);
       }
     }
+
+    if (_rightBracket case var rightBracket?) {
+      writer.splitIf(state == State.split, space: false, indent: Indent.none);
+      writer.format(rightBracket);
+    }
   }
 
   @override
   void forEachChild(void Function(Piece piece) callback) {
+    if (_leftBracket case var leftBracket?) callback(leftBracket);
+
     for (var element in _elements) {
       callback(element.piece);
       for (var comment in element.hangingComments) {
         callback(comment);
       }
     }
+
+    if (_rightBracket case var rightBracket?) callback(rightBracket);
   }
 
   @override