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 <[email protected]>

Author: munificent

CHANGELOG.md

@@ -3,6 +3,35 @@
 * Support dot shorthand syntax.
 * Enable language version 3.10.
 
+### Style changes
+
+This change only applies to code whose language version is 3.10 or higher:
+
+* When `trailing_commas` is `preserve`, preserve a trailing comma after the last
+  enum constant when members are present (#1678, #1729).
+
+  ```dart
+  // Before formatting:
+  enum { constant, ; member() {} }
+
+  // After formatting at language version 3.9 or lower:
+  enum {
+    constant;
+
+    member() {}
+  }
+
+  // After formatting at language version 3.10 or higher:
+  enum {
+    constant,
+    ;
+
+    member() {}
+  }
+  ```
+
+  (Thanks to jellynoone@ for this change.)
+
 ## 3.1.1
 
 * Update to latest analyzer and enable language version 3.9.

lib/src/front_end/ast_node_visitor.dart

@@ -5,6 +5,7 @@ import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/source/line_info.dart';
+import 'package:pub_semver/pub_semver.dart';
 
 import '../ast_extensions.dart';
 import '../back_end/code_writer.dart';
@@ -672,17 +673,34 @@ final class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
           var builder = SequenceBuilder(this);
           builder.leftBracket(node.leftBracket);
 
+          // In 3.10 and later, preserved trailing commas will also preserve a
+          // trailing comma in an enum with members. That in turn forces the
+          // `;` onto its own line after the last costant. Prior to 3.10, the
+          // behavior is the same as when preserved trailing commas is off
+          // where the last constant's comma is removed and the `;` is placed
+          // there instead.
+          var preserveTrailingComma =
+              formatter.trailingCommas == TrailingCommas.preserve &&
+              formatter.languageVersion >= Version(3, 10, 0);
+
           for (var constant in node.constants) {
+            var isLast = constant == node.constants.last;
             builder.addCommentsBefore(constant.firstNonCommentToken);
             builder.add(
               createEnumConstant(
                 constant,
-                isLastConstant: constant == node.constants.last,
-                semicolon: node.semicolon,
+                commaAfter: !isLast || preserveTrailingComma,
+                semicolon: isLast ? node.semicolon : null,
               ),
             );
           }
 
+          // If we are preserving the trailing comma, then put the `;` on its
+          // own line after the last constant.
+          if (preserveTrailingComma) {
+            builder.add(tokenPiece(node.semicolon!));
+          }
+
           // Insert a blank line between the constants and members.
           builder.addBlank();
 

lib/src/front_end/piece_factory.dart

@@ -387,13 +387,14 @@ mixin PieceFactory {
 
   /// Creates a [Piece] for an enum constant.
   ///
-  /// If the constant is in an enum declaration that also declares members, then
-  /// [semicolon] should be the `;` token before the members, and
-  /// [isLastConstant] is `true` if [node] is the last constant before the
-  /// members.
+  /// If [commaAfter] is `true`, then a comma after the constant is written, if
+  /// present. If the constant is the last constant in an enum declaration that
+  /// also declares members (and preserve trailing commas is off), then
+  /// [semicolon] is the `;` token before the members and will be written
+  /// after the constant.
   Piece createEnumConstant(
     EnumConstantDeclaration node, {
-    bool isLastConstant = false,
+    bool commaAfter = false,
     Token? semicolon,
   }) {
     return pieces.build(metadata: node.metadata, () {
@@ -404,17 +405,15 @@ mixin PieceFactory {
         pieces.visit(arguments.argumentList);
       }
 
-      if (semicolon != null) {
-        if (!isLastConstant) {
-          pieces.token(node.commaAfter);
-        } else {
-          // Discard the trailing comma if there is one since there is a
-          // semicolon to use as the separator, but preserve any comments before
-          // the discarded comma.
-          pieces.add(
-            pieces.tokenPiece(discardedToken: node.commaAfter, semicolon),
-          );
-        }
+      if (commaAfter) {
+        pieces.token(node.commaAfter);
+      } else if (semicolon != null) {
+        // Discard the trailing comma if there is one since there is a
+        // semicolon to use as the separator, but preserve any comments before
+        // the discarded comma.
+        pieces.add(
+          pieces.tokenPiece(discardedToken: node.commaAfter, semicolon),
+        );
       }
     });
   }

test/tall/declaration/enum.unit

@@ -51,6 +51,15 @@ enum E { a, b }
 enum E {a,b,c,;}
 <<<
 enum E { a, b, c }
+>>> Split values and add trailing comma but remove semicolon if no members.
+enum Primate{bonobo,chimp,gorilla,organutan;}
+<<<
+enum Primate {
+  bonobo,
+  chimp,
+  gorilla,
+  organutan,
+}
 >>> Argument lists in values.
 enum Args {
 first(),second(a,b,c),

test/tall/preserve_trailing_commas/enum.unit

@@ -25,16 +25,25 @@ enum E {e,;}
 enum E {
   e,
 }
->>> Remove trailing comma and split if there are members.
+>>> Trailing comma with members.
 enum E { a, b, c,; int x; }
-<<<
+<<< 3.9 Remove the trailing comma so the semicolon isn't on its own line.
 enum E {
   a,
   b,
   c;
 
   int x;
 }
+<<< 3.10 Preserve the trailing comma because that's the user intent.
+enum E {
+  a,
+  b,
+  c,
+  ;
+
+  int x;
+}
 >>> Force split in enum value argument list with trailing comma.
 enum Args {
 value(a,b,),