Skip to content

Commit 7278eff

Browse files
Preserve trailing commas enum with members (#1750)
Preserve trailing commas for enums with members when commas are preserved (#1703) This PR changes the formatter to have it maintain trailing commas of last enum constants when there are members. So: ```dart enum E { a, b, c,; int x; } ``` Becomes: ```dart enum E { a, b, c, ; int x; } ``` Instead of: ```dart enum E { a, b, c; int x; } ``` Fixes #1678 Fixes #1729 Additionally, a similar style choice was presented in #1660 (comment) **Affected users**: - Those passing the `trailing_commas: preserve` config option and adding a trailing comma to their enum cases. - Users who don't already pass a trailing comma, aren't affected. So this PR shouldn't have undesirable effects on existing users. **Benefits**: - Maintains developer's intent - Less churn when adding additional member cases (Personally, I always add a trailing semicolon to avoid churn and to also make it even clearer this is an advanced enum with members.) **Downsides**: - 7 additional logical lines of code to maintain **Additional considerations**: I also considered implementing this for `enum E { a, b,; }`, however, that would require more drastic changes to the formatter as far as I can tell. Additionally, in this case the trailing semicolon provides fewer benefits: - churn when adding new members is the same if there was a trailing semicolon or not - giving the enum other members also only amounts to new lines being added rather than changed so again, no real churn. **Notes** - One test was changed due to this being a change in behavior. - This was surprisingly easy and enjoyable to implement (compliment to the existing code base). - I didn't make any changes to the `CHANGELOG.md` since this trailing commas preservation hasn't shipped yet so its behaviour is still undefined. Co-authored-by: jellynoone <77585130+jellynoone@users.noreply.github.com>
1 parent 5d2ad56 commit 7278eff

File tree

5 files changed

+84
-20
lines changed

5 files changed

+84
-20
lines changed

CHANGELOG.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,35 @@
33
* Support dot shorthand syntax.
44
* Enable language version 3.10.
55

6+
### Style changes
7+
8+
This change only applies to code whose language version is 3.10 or higher:
9+
10+
* When `trailing_commas` is `preserve`, preserve a trailing comma after the last
11+
enum constant when members are present (#1678, #1729).
12+
13+
```dart
14+
// Before formatting:
15+
enum { constant, ; member() {} }
16+
17+
// After formatting at language version 3.9 or lower:
18+
enum {
19+
constant;
20+
21+
member() {}
22+
}
23+
24+
// After formatting at language version 3.10 or higher:
25+
enum {
26+
constant,
27+
;
28+
29+
member() {}
30+
}
31+
```
32+
33+
(Thanks to jellynoone@ for this change.)
34+
635
## 3.1.1
736

837
* Update to latest analyzer and enable language version 3.9.

lib/src/front_end/ast_node_visitor.dart

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import 'package:analyzer/dart/ast/ast.dart';
55
import 'package:analyzer/dart/ast/token.dart';
66
import 'package:analyzer/dart/ast/visitor.dart';
77
import 'package:analyzer/source/line_info.dart';
8+
import 'package:pub_semver/pub_semver.dart';
89

910
import '../ast_extensions.dart';
1011
import '../back_end/code_writer.dart';
@@ -672,17 +673,34 @@ final class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
672673
var builder = SequenceBuilder(this);
673674
builder.leftBracket(node.leftBracket);
674675

676+
// In 3.10 and later, preserved trailing commas will also preserve a
677+
// trailing comma in an enum with members. That in turn forces the
678+
// `;` onto its own line after the last costant. Prior to 3.10, the
679+
// behavior is the same as when preserved trailing commas is off
680+
// where the last constant's comma is removed and the `;` is placed
681+
// there instead.
682+
var preserveTrailingComma =
683+
formatter.trailingCommas == TrailingCommas.preserve &&
684+
formatter.languageVersion >= Version(3, 10, 0);
685+
675686
for (var constant in node.constants) {
687+
var isLast = constant == node.constants.last;
676688
builder.addCommentsBefore(constant.firstNonCommentToken);
677689
builder.add(
678690
createEnumConstant(
679691
constant,
680-
isLastConstant: constant == node.constants.last,
681-
semicolon: node.semicolon,
692+
commaAfter: !isLast || preserveTrailingComma,
693+
semicolon: isLast ? node.semicolon : null,
682694
),
683695
);
684696
}
685697

698+
// If we are preserving the trailing comma, then put the `;` on its
699+
// own line after the last constant.
700+
if (preserveTrailingComma) {
701+
builder.add(tokenPiece(node.semicolon!));
702+
}
703+
686704
// Insert a blank line between the constants and members.
687705
builder.addBlank();
688706

lib/src/front_end/piece_factory.dart

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -387,13 +387,14 @@ mixin PieceFactory {
387387

388388
/// Creates a [Piece] for an enum constant.
389389
///
390-
/// If the constant is in an enum declaration that also declares members, then
391-
/// [semicolon] should be the `;` token before the members, and
392-
/// [isLastConstant] is `true` if [node] is the last constant before the
393-
/// members.
390+
/// If [commaAfter] is `true`, then a comma after the constant is written, if
391+
/// present. If the constant is the last constant in an enum declaration that
392+
/// also declares members (and preserve trailing commas is off), then
393+
/// [semicolon] is the `;` token before the members and will be written
394+
/// after the constant.
394395
Piece createEnumConstant(
395396
EnumConstantDeclaration node, {
396-
bool isLastConstant = false,
397+
bool commaAfter = false,
397398
Token? semicolon,
398399
}) {
399400
return pieces.build(metadata: node.metadata, () {
@@ -404,17 +405,15 @@ mixin PieceFactory {
404405
pieces.visit(arguments.argumentList);
405406
}
406407

407-
if (semicolon != null) {
408-
if (!isLastConstant) {
409-
pieces.token(node.commaAfter);
410-
} else {
411-
// Discard the trailing comma if there is one since there is a
412-
// semicolon to use as the separator, but preserve any comments before
413-
// the discarded comma.
414-
pieces.add(
415-
pieces.tokenPiece(discardedToken: node.commaAfter, semicolon),
416-
);
417-
}
408+
if (commaAfter) {
409+
pieces.token(node.commaAfter);
410+
} else if (semicolon != null) {
411+
// Discard the trailing comma if there is one since there is a
412+
// semicolon to use as the separator, but preserve any comments before
413+
// the discarded comma.
414+
pieces.add(
415+
pieces.tokenPiece(discardedToken: node.commaAfter, semicolon),
416+
);
418417
}
419418
});
420419
}

test/tall/declaration/enum.unit

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,15 @@ enum E { a, b }
5151
enum E {a,b,c,;}
5252
<<<
5353
enum E { a, b, c }
54+
>>> Split values and add trailing comma but remove semicolon if no members.
55+
enum Primate{bonobo,chimp,gorilla,organutan;}
56+
<<<
57+
enum Primate {
58+
bonobo,
59+
chimp,
60+
gorilla,
61+
organutan,
62+
}
5463
>>> Argument lists in values.
5564
enum Args {
5665
first(),second(a,b,c),

test/tall/preserve_trailing_commas/enum.unit

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,25 @@ enum E {e,;}
2525
enum E {
2626
e,
2727
}
28-
>>> Remove trailing comma and split if there are members.
28+
>>> Trailing comma with members.
2929
enum E { a, b, c,; int x; }
30-
<<<
30+
<<< 3.9 Remove the trailing comma so the semicolon isn't on its own line.
3131
enum E {
3232
a,
3333
b,
3434
c;
3535

3636
int x;
3737
}
38+
<<< 3.10 Preserve the trailing comma because that's the user intent.
39+
enum E {
40+
a,
41+
b,
42+
c,
43+
;
44+
45+
int x;
46+
}
3847
>>> Force split in enum value argument list with trailing comma.
3948
enum Args {
4049
value(a,b,),

0 commit comments

Comments
 (0)