Format enums (#1317)

Format enum declarations with and without members.

Enums are a little tricky because they are sort of like lists (i.e. ListPiece) and sort of like type declarations (i.e. BlockPiece). Enums without members should be formatted pretty much like any other delimited list where the declaration can collapse to one line (and discard any trailing comma) if it fits or expand to one element per line (and add a trailing comma) otherwise.

Enums with members are formatted pretty much like blocks where each constant  or member is on its own line.

There are a few wrinkles around trailing commas and the semicolon between the constants and members:

-   If the enum has no members, any trailing `;` is discarded. It doesn't do anything useful and I consider this morally equivalent to discarding other trailing commas.

-   If the enum has members, then a trailing comma after the last constant is redundant with the subsequent `;`. In that case, it discards the comma.

In both of these cases, we have to make sure we don't accidentally lose any comments around that discarded punctuation. So there are a lot of tests for all of those edge cases even though in practice no one writes code like that.

Discarding `;` on enums without members means that we treat `;` like whitespace, so I also loosened equalIgnoringWhitespace() to ignore semicolons too.

Since enums with and without members are formatted using different codepaths, there are a lot of mostly redundant-seeming tests for them.

Author: munificent

lib/src/ast_extensions.dart

@@ -5,6 +5,21 @@ import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
 
 extension AstNodeExtensions on AstNode {
+  /// The first token at the beginning of this AST node, not including any
+  /// tokens for leading doc comments.
+  ///
+  /// If [node] is an [AnnotatedNode], then [beginToken] includes the
+  /// leading doc comment, which we want to handle separately. So, in that
+  /// case, explicitly skip past the doc comment to the subsequent metadata
+  /// (if there is any), or the beginning of the code.
+  Token get firstNonCommentToken {
+    return switch (this) {
+      AnnotatedNode(metadata: [var annotation, ...]) => annotation.beginToken,
+      AnnotatedNode(firstTokenAfterCommentAndMetadata: var token) => token,
+      _ => beginToken
+    };
+  }
+
   /// The comma token immediately following this if there is one, or `null`.
   Token? get commaAfter {
     var next = endToken.next!;

lib/src/front_end/ast_node_visitor.dart

@@ -114,8 +114,11 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
   }
 
   @override
-  void visitArgumentList(ArgumentList node, {bool nestExpression = true}) {
-    throw UnimplementedError();
+  void visitArgumentList(ArgumentList node) {
+    createList(
+        leftBracket: node.leftParenthesis,
+        node.arguments,
+        rightBracket: node.rightParenthesis);
   }
 
   @override
@@ -400,12 +403,81 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitEnumConstantDeclaration(EnumConstantDeclaration node) {
-    throw UnimplementedError();
+    token(node.name);
+    if (node.arguments case var arguments?) {
+      visit(arguments.typeArguments);
+      visit(arguments.argumentList);
+    }
   }
 
   @override
   void visitEnumDeclaration(EnumDeclaration node) {
-    throw UnimplementedError();
+    if (node.metadata.isNotEmpty) throw UnimplementedError();
+
+    token(node.enumKeyword);
+    space();
+    token(node.name);
+    visit(node.typeParameters);
+    space();
+
+    if (node.members.isEmpty) {
+      // If there are no members, format the constants like a delimited list.
+      // This keeps the enum declaration on one line if it fits.
+      var builder = DelimitedListBuilder(
+          this,
+          const ListStyle(
+              spaceWhenUnsplit: true, splitListIfBeforeSplits: true));
+      builder.leftBracket(node.leftBracket);
+      node.constants.forEach(builder.visit);
+
+      builder.rightBracket(semicolon: node.semicolon, node.rightBracket);
+      pieces.give(builder.build());
+    } else {
+      // If there are members, format it like a block where each constant and
+      // member is on its own line.
+      token(node.leftBracket);
+      var leftBracketPiece = pieces.split();
+
+      var sequence = SequenceBuilder(this);
+      for (var constant in node.constants) {
+        sequence.addCommentsBefore(constant.firstNonCommentToken);
+        visit(constant);
+
+        if (constant != node.constants.last) {
+          commaAfter(constant);
+        } 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.
+          var trailingComma = constant.commaAfter;
+          if (trailingComma != null) writeCommentsBefore(trailingComma);
+
+          token(node.semicolon);
+        }
+
+        sequence.add(pieces.split());
+      }
+
+      // Insert a blank line between the constants and members.
+      sequence.addBlank();
+
+      for (var node in node.members) {
+        sequence.visit(node);
+
+        // If the node has a non-empty braced body, then require a blank line
+        // between it and the next node.
+        if (node.hasNonEmptyBody) sequence.addBlank();
+      }
+
+      // Place any comments before the "}" inside the block.
+      sequence.addCommentsBefore(node.rightBracket);
+
+      token(node.rightBracket);
+      var rightBracketPiece = pieces.take();
+
+      pieces.give(
+          BlockPiece(leftBracketPiece, sequence.build(), rightBracketPiece));
+    }
   }
 
   @override
@@ -757,7 +829,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
       visit(constructor.name);
     }
 
-    finishCall(node.argumentList);
+    visit(node.argumentList);
 
     // If there was a prefix or constructor name, then make a splittable piece.
     if (operations.isNotEmpty) {
@@ -883,7 +955,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
     visit(node.methodName);
     visit(node.typeArguments);
-    finishCall(node.argumentList);
+    visit(node.argumentList);
   }
 
   @override

lib/src/front_end/delimited_list_builder.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
+import 'package:dart_style/src/ast_extensions.dart';
 
 import '../comment_type.dart';
 import '../piece/list.dart';
@@ -75,8 +76,13 @@ class DelimitedListBuilder {
   /// ```
   ///
   /// Here, [bracket] will be `)` and [delimiter] will be `}`.
-  void rightBracket(Token bracket, {Token? delimiter}) {
+  ///
+  /// If [semicolon] is given, it is the optional `;` in an enum declaration
+  /// after the enum constants when there are no subsequent members. Comments
+  /// before the `;` are kept, but the `;` itself is discarded.
+  void rightBracket(Token bracket, {Token? delimiter, Token? semicolon}) {
     // Handle comments after the last element.
+    var commentsBefore = _visitor.takeCommentsBefore(bracket);
 
     // Merge the comments before the delimiter (if there is one) and the
     // bracket. If there is a delimiter, this will move comments between it and
@@ -89,11 +95,16 @@ class DelimitedListBuilder {
     // // After:
     // f([parameter /* comment */]) {}
     // ```
-    var commentsBefore = _visitor.takeCommentsBefore(bracket);
     if (delimiter != null) {
       commentsBefore =
           _visitor.takeCommentsBefore(delimiter).concatenate(commentsBefore);
     }
+
+    if (semicolon != null) {
+      commentsBefore =
+          _visitor.takeCommentsBefore(semicolon).concatenate(commentsBefore);
+    }
+
     _addComments(commentsBefore, hasElementAfter: false);
 
     _visitor.token(delimiter);
@@ -123,7 +134,7 @@ class DelimitedListBuilder {
   /// Adds [element] to the built list.
   void visit(AstNode element) {
     // Handle comments between the preceding element and this one.
-    addCommentsBefore(element.beginToken);
+    addCommentsBefore(element.firstNonCommentToken);
 
     // Traverse the element itself.
     _visitor.visit(element);

lib/src/front_end/piece_factory.dart

@@ -608,15 +608,6 @@ mixin PieceFactory implements CommentWriter {
         isValueDelimited: rightHandSide.isDelimited));
   }
 
-  /// Writes the argument list part of a constructor, function, or method call
-  /// after the name has been written.
-  void finishCall(ArgumentList argumentList) {
-    createList(
-        leftBracket: argumentList.leftParenthesis,
-        argumentList.arguments,
-        rightBracket: argumentList.rightParenthesis);
-  }
-
   /// Finishes writing a named function declaration or anonymous function
   /// expression after the return type (if any) and name (if any) has been
   /// written.

lib/src/front_end/sequence_builder.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
+import 'package:dart_style/src/ast_extensions.dart';
 
 import '../constants.dart';
 import '../piece/piece.dart';
@@ -46,17 +47,7 @@ class SequenceBuilder {
   /// Visits [node] and adds the resulting [Piece] to this sequence, handling
   /// any comments or blank lines that appear before it.
   void visit(AstNode node, {int? indent}) {
-    var token = switch (node) {
-      // If [node] is an [AnnotatedNode], then [beginToken] includes the
-      // leading doc comment, which we want to handle separately. So, in that
-      // case, explicitly skip past the doc comment to the subsequent metadata
-      // (if there is any), or the beginning of the code.
-      AnnotatedNode(metadata: [var annotation, ...]) => annotation.beginToken,
-      AnnotatedNode() => node.firstTokenAfterCommentAndMetadata,
-      _ => node.beginToken
-    };
-
-    addCommentsBefore(token);
+    addCommentsBefore(node.firstNonCommentToken);
     _visitor.visit(node);
     add(_visitor.pieces.split(), indent: indent);
   }
@@ -96,7 +87,6 @@ class SequenceBuilder {
       if (_elements.isNotEmpty && comments.isHanging(i)) {
         // Attach the comment to the previous token.
         _visitor.space();
-
         _visitor.pieces.writeComment(comment, hanging: true);
       } else {
         // Write the comment as its own sequence piece.

lib/src/string_compare.dart

@@ -19,6 +19,9 @@
 /// add and remove trailing commas. It treats, `[`, `]`, `{`, and `}` as
 /// whitespace characters because the formatter may move a comment if it
 /// appears near the closing delimiter of an optional parameter section.
+///
+/// It treats `;` as whitespace because a trailing `;` inside an enum
+/// declaration that has no members will be removed.
 bool _isWhitespace(int c) {
   // Not using a set or something more elegant because this code is on the hot
   // path and this large expression is significantly faster than a set lookup.
@@ -27,6 +30,7 @@ bool _isWhitespace(int c) {
       c == 0x005d || // Treat `]` as "whitespace".
       c == 0x007b || // Treat `{` as "whitespace".
       c == 0x007d || // Treat `}` as "whitespace".
+      c == 0x003b || // Treat `;` as "whitespace".
       c >= 0x0009 && c <= 0x000d || // Control characters.
       c == 0x0020 || // SPACE.
       c == 0x0085 || // Control characters.

test/declaration/enum.unit

@@ -0,0 +1,117 @@
+40 columns                              |
+>>> Single value.
+enum Unity {one}
+<<<
+enum Unity { one }
+>>> Multiple unsplit values.
+enum Primate{
+
+  bonobo,
+
+
+
+  chimp,
+
+
+  gorilla
+
+
+
+}
+<<<
+enum Primate { bonobo, chimp, gorilla }
+>>> Remove trailing comma if values don't split.
+enum Primate{bonobo,chimp,}
+<<<
+enum Primate { bonobo, chimp }
+>>> Split values and insert trailing comma.
+enum Primate{bonobo,chimp,gorilla,organutan}
+<<<
+enum Primate {
+  bonobo,
+  chimp,
+  gorilla,
+  organutan,
+}
+>>> Insert blank line before and after enum declaration.
+var x = 1;
+enum A { a }
+var y = 2;
+<<<
+var x = 1;
+
+enum A { a }
+
+var y = 2;
+>>> Remove semicolon if there are no members.
+enum E { a, b; }
+<<<
+enum E { a, b }
+>>> Remove trailing comma and semicolon if unsplit.
+enum E {a,b,c,;}
+<<<
+enum E { a, b, c }
+>>> Argument lists in values.
+enum Args {
+first(),second(a,b,c),
+third(named:1,2,another:3)
+}
+<<<
+enum Args {
+  first(),
+  second(a, b, c),
+  third(named: 1, 2, another: 3),
+}
+>>> Split argument lists in values.
+enum Args {
+firstEnumValue(longArgument,anotherArgument),
+secondEnumValue(longArgument,anotherArgument,aThirdArgument,theLastOne),
+thirdGenericOne<FirstTypeArgument,
+SecondTypeArgument<NestedTypeArgument,AnotherNestedTypeArgument>,
+LastTypeArgument>(namedArgument: firstValue,anotherNamed:argumentValue)
+}
+<<<
+enum Args {
+  firstEnumValue(
+    longArgument,
+    anotherArgument,
+  ),
+  secondEnumValue(
+    longArgument,
+    anotherArgument,
+    aThirdArgument,
+    theLastOne,
+  ),
+  thirdGenericOne<
+    FirstTypeArgument,
+    SecondTypeArgument<
+      NestedTypeArgument,
+      AnotherNestedTypeArgument
+    >,
+    LastTypeArgument
+  >(
+    namedArgument: firstValue,
+    anotherNamed: argumentValue,
+  ),
+}
+>>> Generic enum type.
+enum MagicNumbers< T    extends num   ,   S> {
+  one(1), two(2),pi<double,String>(3.14159)
+}
+<<<
+enum MagicNumbers<T extends num, S> {
+  one(1),
+  two(2),
+  pi<double, String>(3.14159),
+}
+>>> Split in type parameters forces body to split.
+enum SomeEnumType<LongTypeParameterName, Another> { a, b, c }
+<<<
+enum SomeEnumType<
+  LongTypeParameterName,
+  Another
+> {
+  a,
+  b,
+  c,
+}