Skip to content

Commit

Permalink
Don't drop comments in collection literals with preserved newlines.
Browse files Browse the repository at this point in the history
A line comment in a collection literal triggers special formatting
where we allow multiple elements in a single line, like:

```
list = [
  // Comment.
  1, 2, 3
  4, 5,
  6,
];
```

The formatter models that by having a DelimitedListBuilder for the
entire collection literal. Then the elements that should be packed onto
a single line are formatted using a separate DelimitedListBuilder for
each line.

In the very rare case where you have a comment after the last element
on a line but *before* the subsequent comma, the comment would get
dropped. This is because the comment was sitting in the inner
DelimitedListBuilder for the line waiting to be put somewhere, but we
then discard that builder to start a new line or when the whole list is
done.

This fixes that. Whenever we're done with a line, we hoist any remaining
comments up to the outer DelimitedListBuilder. That way the comment gets
put after the comma at the end of the line.

Also bump the min SDK constraint to 3.4.0. I don't think dart_style
works with an older version because of analyzer's SDK constraints.

Fix #1584.
  • Loading branch information
munificent committed Oct 24, 2024
1 parent 89577e7 commit 914d1d6
Show file tree
Hide file tree
Showing 5 changed files with 491 additions and 4 deletions.
14 changes: 14 additions & 0 deletions lib/src/front_end/delimited_list_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,20 @@ final class DelimitedListBuilder {
_commentsBeforeComma = CommentSequence.empty;
}

/// Adds the contents of [lineBuilder] to this outer [DelimitedListBuilder].
///
/// This is used when preserving newlines inside a collection literal. The
/// [lineBuilder] will be used for the elements that should be packed onto a
/// single line, and this builder is for the rows that are each on their own
/// line.
void addLineBuilder(DelimitedListBuilder lineBuilder) {
// Add the elements of the line to this builder.
add(lineBuilder.build());

// Make sure that any trailing comments on the line aren't lost.
_commentsBeforeComma = lineBuilder._commentsBeforeComma;
}

/// Writes any comments appearing before [token] to the list.
void addCommentsBefore(Token token) {
// Handle comments between the preceding element and this one.
Expand Down
5 changes: 3 additions & 2 deletions lib/src/front_end/piece_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1082,7 +1082,7 @@ mixin PieceFactory {
elements[i - 1].endToken, element.beginToken)) {
// This element begins a new line. Add the elements on the previous
// line to the list builder and start a new line.
builder.add(lineBuilder.build());
builder.addLineBuilder(lineBuilder);
lineBuilder = DelimitedListBuilder(this, lineStyle);
atLineStart = true;
}
Expand All @@ -1097,7 +1097,8 @@ mixin PieceFactory {
atLineStart = false;
}

if (!atLineStart) builder.add(lineBuilder.build());
// Finish the last line if there is anything on it.
if (!atLineStart) builder.addLineBuilder(lineBuilder);
}

/// Writes a [VariablePiece] for a named or wildcard variable pattern.
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ description: >-
Provides an API and a CLI tool.
repository: https://github.com/dart-lang/dart_style
environment:
sdk: "^3.0.0"
sdk: "^3.4.0"

dependencies:
analyzer: '^6.5.0'
Expand Down
24 changes: 23 additions & 1 deletion test/tall/expression/list_comment.stmt
Original file line number Diff line number Diff line change
Expand Up @@ -143,4 +143,26 @@ var list = [
>>> Space between block comment and ",".
var list = [1,/* a */ 2 /* b */ , 3];
<<<
var list = [1, /* a */ 2 /* b */, 3];
var list = [1, /* a */ 2 /* b */, 3];
>>> Comment before comma with other comments.
var x = [
1 // Comment 1.
,
2 // Comment 2.
];
<<<
var x = [
1, // Comment 1.
2, // Comment 2.
];
>>> Comment before comma with other comments.
var x = [
1 // Comment 1.
,
// Comment 2.
];
<<<
var x = [
1, // Comment 1.
// Comment 2.
];
Loading

0 comments on commit 914d1d6

Please sign in to comment.