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 <nbosch@google.com>
diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart
index cbdf18e..e13598a 100644
--- a/lib/src/front_end/ast_node_visitor.dart
+++ b/lib/src/front_end/ast_node_visitor.dart
@@ -12,7 +12,6 @@
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 @@
// 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 @@
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 @@
@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);
-
- var casePiece = buildPiece((b) {
- b.token(member.keyword);
-
- if (member is SwitchCase) {
- b.space();
- b.visit(member.expression);
- } else if (member is SwitchPatternCase) {
- if (member.guardedPattern.whenClause != null) {
- throw UnimplementedError();
- }
-
- b.space();
- b.visit(member.guardedPattern.pattern);
- } else {
- assert(member is SwitchDefault);
- // Nothing to do.
+ for (var member in node.members) {
+ for (var label in member.labels) {
+ sequence.visit(label);
}
- b.token(member.colon);
- });
+ sequence.addCommentsBefore(member.keyword);
- // 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);
+ var casePiece = buildPiece((b) {
+ b.token(member.keyword);
- for (var statement in member.statements) {
- sequence.visit(statement, indent: Indent.block);
+ if (member is SwitchCase) {
+ b.space();
+ b.visit(member.expression);
+ } else if (member is SwitchPatternCase) {
+ if (member.guardedPattern.whenClause != null) {
+ throw UnimplementedError();
+ }
+
+ b.space();
+ b.visit(member.guardedPattern.pattern);
+ } else {
+ assert(member is SwitchDefault);
+ // Nothing to do.
+ }
+
+ b.token(member.colon);
+ });
+
+ // 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);
+
+ for (var statement in member.statements) {
+ sequence.visit(statement, indent: Indent.block);
+ }
}
- }
- // Place any comments before the "}" inside the sequence.
- sequence.addCommentsBefore(node.rightBracket);
- var rightBracketPiece = tokenPiece(node.rightBracket);
-
- return BlockPiece(leftBracket, sequence.build(), rightBracketPiece,
- alwaysSplit: node.members.isNotEmpty || sequence.mustSplit);
+ sequence.rightBracket(node.rightBracket);
+ b.add(sequence.build());
+ });
}
@override
diff --git a/lib/src/front_end/piece_factory.dart b/lib/src/front_end/piece_factory.dart
index f9f8a44..69f0f03 100644
--- a/lib/src/front_end/piece_factory.dart
+++ b/lib/src/front_end/piece_factory.dart
@@ -6,13 +6,13 @@
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 @@
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 @@
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 @@
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
diff --git a/lib/src/front_end/sequence_builder.dart b/lib/src/front_end/sequence_builder.dart
index 7529032..3d0715d 100644
--- a/lib/src/front_end/sequence_builder.dart
+++ b/lib/src/front_end/sequence_builder.dart
@@ -24,9 +24,15 @@
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 @@
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 @@
}
// Write the comment as its own sequence piece.
- add(comment);
+ _add(Indent.none, comment);
}
}
@@ -108,4 +137,11 @@
// 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));
+ }
}
diff --git a/lib/src/piece/block.dart b/lib/src/piece/block.dart
deleted file mode 100644
index d6cc9a1..0000000
--- a/lib/src/piece/block.dart
+++ /dev/null
@@ -1,58 +0,0 @@
-// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-
-import '../back_end/code_writer.dart';
-import '../constants.dart';
-import 'piece.dart';
-import 'sequence.dart';
-
-/// A piece for a series of statements or members inside a block or declaration
-/// body.
-class BlockPiece extends Piece {
- /// The opening delimiter.
- final Piece leftBracket;
-
- /// The sequence of members, statements, and sequence-level comments.
- final SequencePiece contents;
-
- /// The closing delimiter.
- final Piece rightBracket;
-
- /// If [alwaysSplit] is true, then the block should always split its contents.
- /// This is true for most blocks, but false for enums and blocks containing
- /// only inline block comments.
- BlockPiece(this.leftBracket, this.contents, this.rightBracket,
- {bool alwaysSplit = true}) {
- if (alwaysSplit) pin(State.split);
- }
-
- @override
- List<State> get additionalStates => [if (contents.isNotEmpty) State.split];
-
- @override
- void format(CodeWriter writer, State state) {
- writer.format(leftBracket);
-
- if (state == State.split) {
- if (contents.isNotEmpty) {
- writer.newline(indent: Indent.block);
- writer.format(contents);
- }
-
- writer.newline(indent: Indent.none);
- } else {
- writer.setAllowNewlines(false);
- writer.format(contents);
- }
-
- writer.format(rightBracket);
- }
-
- @override
- void forEachChild(void Function(Piece piece) callback) {
- callback(leftBracket);
- callback(contents);
- callback(rightBracket);
- }
-}
diff --git a/lib/src/piece/sequence.dart b/lib/src/piece/sequence.dart
index 039a2a6..9e68444 100644
--- a/lib/src/piece/sequence.dart
+++ b/lib/src/piece/sequence.dart
@@ -3,6 +3,7 @@
// 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
@@ -10,16 +11,32 @@
///
/// 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;
- /// Whether this sequence has any contents.
- bool get isNotEmpty => _elements.isNotEmpty;
+ /// The closing delimiter, if any.
+ final Piece? _rightBracket;
+
+ @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 @@
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
