Allow comments before infix expressions to not split the operator. (#1433)
Allow comments before infix expressions to not split the operator.
Currently, unless captured by some SequenceBuilder or DelimitedListBuilder, comments (that aren't hanging of the previous token) get attached to the token immediately following them. For example:
```dart
value =
// comment
a + b;
```
Here, since there is a newline before `// comment`, it's not attached to the `=` and is instead attached as a leading comment to the `a` token owned by the identifier expression as the first operand to `+`.
That means that as far as the `+` expression is concerned, there is a newline in one of its operands, so it thinks it needs to split, leading to:
```dart
value =
// comment
a +
b;
```
But since the comment is, to a user, outside of the infix expression, that split looks pointless.
This fixes that. When leading comments appear right before any infix-like expression (binary operator, conditional operator, binary pattern, etc.), we hoist them out of the innermost operand that would otherwise claim them and make them precede the infix piece itself.
diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart
index b61c2a5..d6e9c1a 100644
--- a/lib/src/front_end/ast_node_visitor.dart
+++ b/lib/src/front_end/ast_node_visitor.dart
@@ -312,6 +312,10 @@
@override
Piece visitConditionalExpression(ConditionalExpression node) {
+ // Hoist any comments before the condition operand so they don't force the
+ // conditional expression to split.
+ var leadingComments = pieces.takeCommentsBefore(node.firstNonCommentToken);
+
var condition = nodePiece(node.condition);
var thenPiece = buildPiece((b) {
@@ -326,7 +330,7 @@
b.visit(node.elseExpression, context: NodeContext.conditionalBranch);
});
- var piece = InfixPiece([condition, thenPiece, elsePiece]);
+ var piece = InfixPiece(leadingComments, [condition, thenPiece, elsePiece]);
// If conditional expressions are directly nested, force them all to split,
// both parents and children.
diff --git a/lib/src/front_end/piece_factory.dart b/lib/src/front_end/piece_factory.dart
index 7a5d68b..472d610 100644
--- a/lib/src/front_end/piece_factory.dart
+++ b/lib/src/front_end/piece_factory.dart
@@ -817,6 +817,10 @@
/// separate tokens, as in `foo is! Bar`.
Piece createInfix(AstNode left, Token operator, AstNode right,
{bool hanging = false, Token? operator2}) {
+ // Hoist any comments before the first operand so they don't force the
+ // infix operator to split.
+ var leadingComments = pieces.takeCommentsBefore(left.firstNonCommentToken);
+
var leftPiece = buildPiece((b) {
b.visit(left);
if (hanging) {
@@ -836,7 +840,7 @@
b.visit(right);
});
- return InfixPiece([leftPiece, rightPiece]);
+ return InfixPiece(leadingComments, [leftPiece, rightPiece]);
}
/// Creates a chained infix operation: a binary operator expression, or
@@ -857,6 +861,10 @@
Piece createInfixChain<T extends AstNode>(
T node, BinaryOperation Function(T node) destructure,
{int? precedence, bool indent = true}) {
+ // Hoist any comments before the first operand so they don't force the
+ // infix operator to split.
+ var leadingComments = pieces.takeCommentsBefore(node.firstNonCommentToken);
+
var builder = AdjacentBuilder(this);
var operands = <Piece>[];
@@ -882,7 +890,7 @@
traverse(node);
operands.add(builder.build());
- return InfixPiece(operands, indent: indent);
+ return InfixPiece(leadingComments, operands, indent: indent);
}
/// Creates a [ListPiece] for the given bracket-delimited set of elements.
diff --git a/lib/src/front_end/piece_writer.dart b/lib/src/front_end/piece_writer.dart
index d2d0139..8441849 100644
--- a/lib/src/front_end/piece_writer.dart
+++ b/lib/src/front_end/piece_writer.dart
@@ -92,6 +92,12 @@
return commentPiece;
}
+ /// Applies any hanging comments before [token] to the preceding [CodePiece]
+ /// and takes and returns any remaining leading comments.
+ List<Piece> takeCommentsBefore(Token token) {
+ return _splitComments(_comments.takeCommentsBefore(token), token);
+ }
+
/// Creates a [CodePiece] for [token] and handles any comments that precede
/// it, which get attached either as hanging comments on the preceding
/// [CodePiece] or leading comments on this one.
diff --git a/lib/src/piece/infix.dart b/lib/src/piece/infix.dart
index c996e1d..5929172 100644
--- a/lib/src/piece/infix.dart
+++ b/lib/src/piece/infix.dart
@@ -9,6 +9,27 @@
///
/// a + b + c
class InfixPiece extends Piece {
+ /// Pieces for leading comments that appear before the first operand.
+ ///
+ /// We hoist these comments out from the first operand's first token so that
+ /// a newline in these comments doesn't erroneously force the infix operator
+ /// to split. For example:
+ ///
+ /// value =
+ /// // comment
+ /// a + b;
+ ///
+ /// Here, the `// comment` will be hoisted out and stored in
+ /// [_leadingComments] instead of being a leading comment in the [CodePiece]
+ /// for `a`. If we left the comment in `a`, then the newline after the line
+ /// comment would force the `+` operator to split yielding:
+ ///
+ /// value =
+ /// // comment
+ /// a +
+ /// b;
+ final List<Piece> _leadingComments;
+
/// The series of operands.
///
/// Since we don't split on both sides of the operator, the operators will be
@@ -20,13 +41,21 @@
/// Whether operands after the first should be indented if split.
final bool _indent;
- InfixPiece(this._operands, {bool indent = true}) : _indent = indent;
+ InfixPiece(this._leadingComments, this._operands, {bool indent = true})
+ : _indent = indent;
@override
List<State> get additionalStates => const [State.split];
@override
void format(CodeWriter writer, State state) {
+ // Comments before the operands don't force the operator to split.
+ writer.pushAllowNewlines(true);
+ for (var comment in _leadingComments) {
+ writer.format(comment);
+ }
+ writer.popAllowNewlines();
+
if (state == State.unsplit) {
writer.pushAllowNewlines(false);
} else if (_indent) {
@@ -52,6 +81,7 @@
@override
void forEachChild(void Function(Piece piece) callback) {
+ _leadingComments.forEach(callback);
_operands.forEach(callback);
}
diff --git a/test/tall/expression/binary_comment.stmt b/test/tall/expression/binary_comment.stmt
index 80e467e..524a496 100644
--- a/test/tall/expression/binary_comment.stmt
+++ b/test/tall/expression/binary_comment.stmt
@@ -86,4 +86,12 @@
// three
// four
// five
- b;
\ No newline at end of file
+ b;
+>>> Don't split infix operator with leading line comment before first operand.
+value =
+ // comment
+ a + b + c;
+<<<
+value =
+ // comment
+ a + b + c;
\ No newline at end of file
diff --git a/test/tall/expression/condition_comment.stmt b/test/tall/expression/condition_comment.stmt
index c928b5b..152c533 100644
--- a/test/tall/expression/condition_comment.stmt
+++ b/test/tall/expression/condition_comment.stmt
@@ -59,4 +59,12 @@
cond
? 1
: 2 // c
- ;
\ No newline at end of file
+ ;
+>>> Don't split with leading line comment before first operand.
+value =
+ // comment
+ a ? b : c;
+<<<
+value =
+ // comment
+ a ? b : c;
\ No newline at end of file
diff --git a/test/tall/expression/type_test_comment.stmt b/test/tall/expression/type_test_comment.stmt
index 99c4968..a862659 100644
--- a/test/tall/expression/type_test_comment.stmt
+++ b/test/tall/expression/type_test_comment.stmt
@@ -122,4 +122,20 @@
veryLongOperand is!/* c */VeryLongTypeName;
<<<
veryLongOperand
- is! /* c */ VeryLongTypeName;
\ No newline at end of file
+ is! /* c */ VeryLongTypeName;
+>>> Don't split `as` with leading line comment before first operand.
+value =
+ // comment
+ a as Foo;
+<<<
+value =
+ // comment
+ a as Foo;
+>>> Don't split `is` with leading line comment before first operand.
+value =
+ // comment
+ a is Foo;
+<<<
+value =
+ // comment
+ a is Foo;
\ No newline at end of file
diff --git a/test/tall/pattern/cast_comment.stmt b/test/tall/pattern/cast_comment.stmt
index 85f59b8..782c3e1 100644
--- a/test/tall/pattern/cast_comment.stmt
+++ b/test/tall/pattern/cast_comment.stmt
@@ -26,4 +26,14 @@
as // c
Type) {
;
+}
+>>> Don't split `as` with leading line comment before first operand.
+if (obj case
+ // comment
+ constant as Type) {;}
+<<<
+if (obj
+ case // comment
+ constant as Type) {
+ ;
}
\ No newline at end of file
diff --git a/test/tall/top_level/import_comment.unit b/test/tall/top_level/import_comment.unit
index 5b78670..2e38068 100644
--- a/test/tall/top_level/import_comment.unit
+++ b/test/tall/top_level/import_comment.unit
@@ -6,4 +6,12 @@
import 'foo.dart'
hide
First, //
- Second;
\ No newline at end of file
+ Second;
+>>> Don't split `==` because of leading comment before left operand.
+import 'uri.dart' if (
+ // comment
+config == 'value') 'c';
+<<<
+import 'uri.dart'
+ if (// comment
+ config == 'value') 'c';
\ No newline at end of file
