Format for statements. (#1296)

Format for statements.

This handles:

```
for (initializer; condition; increment) ...
for (var variable; condition; increment) ...
```

It doesn't add support for:

```
// No patterns yet:
for (var [pattern]; condition; increment) ...

// No for-each yet:
for (existingVariable in iterable) ...
for (var newVariable in iterable) ...
// No for-each or patterns yet:
for (var [pattern] in iterable) ...
```

This one was a lot of work. There were almost no tests for for loops
already and the style approach I took here is different from the old
style. Since the clauses are formatted more like a list now, I wanted
to reuse DelimitedListBuilder but it took some work to get that playing
nice with clauses that aren't comma separated.

That's what the previous refactoring PR is to enable.

After this, supporting for-each and for-with-patterns should be (I
hope!) relatively straightforward.

Took Nate's suggestion to use pattern matching for the edge case.

Author: munificent

lib/src/front_end/ast_node_visitor.dart

@@ -2,13 +2,15 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // 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:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/source/line_info.dart';
 
 import '../constants.dart';
 import '../dart_formatter.dart';
 import '../piece/block.dart';
 import '../piece/do_while.dart';
+import '../piece/for.dart';
 import '../piece/if.dart';
 import '../piece/infix.dart';
 import '../piece/list.dart';
@@ -425,37 +427,102 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
 
   @override
   void visitForStatement(ForStatement node) {
-    throw UnimplementedError();
+    token(node.forKeyword);
+    writer.split();
+    var forKeyword = writer.pop();
+
+    // Treat the for loop parts sort of as an argument list where each clause
+    // is a separate argument. This means that when they split, they split like:
+    //
+    // ```
+    // for (
+    //   initializerClause;
+    //   conditionClause;
+    //   incrementClause
+    // ) {
+    //   body;
+    // }
+    // ```
+    var partsList =
+        DelimitedListBuilder(this, const ListStyle(commas: Commas.none));
+    partsList.leftBracket(node.leftParenthesis);
+
+    var forLoopParts = node.forLoopParts;
+    switch (forLoopParts) {
+      // Edge case: A totally empty for loop is formatted just as `(;;)` with
+      // no splits or spaces anywhere.
+      case ForPartsWithExpression(
+            initialization: null,
+            leftSeparator: Token(precedingComments: null),
+            condition: null,
+            rightSeparator: Token(precedingComments: null),
+            updaters: NodeList(isEmpty: true),
+          )
+          when node.rightParenthesis.precedingComments == null:
+        token(forLoopParts.leftSeparator);
+        token(forLoopParts.rightSeparator);
+
+      case ForPartsWithDeclarations():
+        partsList.addCommentsBefore(forLoopParts.variables.beginToken);
+        visit(forLoopParts.variables);
+        finishForParts(forLoopParts, partsList);
+
+      case ForPartsWithExpression(:var initialization?):
+        partsList.addCommentsBefore(initialization.beginToken);
+        visit(initialization);
+        finishForParts(forLoopParts, partsList);
+
+      case ForPartsWithExpression():
+        // No initializer part.
+        partsList.addCommentsBefore(forLoopParts.leftSeparator);
+        finishForParts(forLoopParts, partsList);
+
+      case ForPartsWithPattern():
+      case ForEachPartsWithDeclaration():
+      case ForEachPartsWithIdentifier():
+      case ForEachPartsWithPattern():
+        throw UnimplementedError();
+    }
+
+    partsList.rightBracket(node.rightParenthesis);
+    writer.split();
+    var forPartsPiece = partsList.build();
+
+    visit(node.body);
+    var body = writer.pop();
+
+    writer.push(ForPiece(forKeyword, forPartsPiece, body,
+        hasBlockBody: node.body is Block));
   }
 
   @override
   void visitForEachPartsWithDeclaration(ForEachPartsWithDeclaration node) {
-    throw UnimplementedError();
+    assert(false, 'This node is handled by visitForStatement().');
   }
 
   @override
   void visitForEachPartsWithIdentifier(ForEachPartsWithIdentifier node) {
-    throw UnimplementedError();
+    assert(false, 'This node is handled by visitForStatement().');
   }
 
   @override
   void visitForEachPartsWithPattern(ForEachPartsWithPattern node) {
-    throw UnimplementedError();
+    assert(false, 'This node is handled by visitForStatement().');
   }
 
   @override
   void visitForPartsWithDeclarations(ForPartsWithDeclarations node) {
-    throw UnimplementedError();
+    assert(false, 'This node is handled by visitForStatement().');
   }
 
   @override
   void visitForPartsWithExpression(ForPartsWithExpression node) {
-    throw UnimplementedError();
+    assert(false, 'This node is handled by visitForStatement().');
   }
 
   @override
   void visitForPartsWithPattern(ForPartsWithPattern node) {
-    throw UnimplementedError();
+    assert(false, 'This node is handled by visitForStatement().');
   }
 
   @override
@@ -647,8 +714,10 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>
     visit(node.methodName);
     visit(node.typeArguments);
 
-    createList(node.argumentList.leftParenthesis, node.argumentList.arguments,
-        node.argumentList.rightParenthesis);
+    createList(
+        leftBracket: node.argumentList.leftParenthesis,
+        node.argumentList.arguments,
+        rightBracket: node.argumentList.rightParenthesis);
   }
 
   @override

lib/src/front_end/piece_factory.dart

@@ -108,7 +108,7 @@ mixin PieceFactory implements CommentWriter {
     // The formatter will preserve the newline after element 3 and the lack of
     // them after the other elements.
 
-    createList(leftBracket, elements, rightBracket);
+    createList(leftBracket: leftBracket, elements, rightBracket: rightBracket);
   }
 
   /// Creates metadata annotations for a directive.
@@ -357,13 +357,14 @@ mixin PieceFactory implements CommentWriter {
   }
 
   /// Creates a [ListPiece] for the given bracket-delimited set of elements.
-  void createList(
-      Token leftBracket, Iterable<AstNode> elements, Token rightBracket,
-      {ListStyle style = const ListStyle()}) {
+  void createList(Iterable<AstNode> elements,
+      {Token? leftBracket,
+      Token? rightBracket,
+      ListStyle style = const ListStyle()}) {
     var builder = DelimitedListBuilder(this, style);
-    builder.leftBracket(leftBracket);
+    if (leftBracket != null) builder.leftBracket(leftBracket);
     elements.forEach(builder.visit);
-    builder.rightBracket(rightBracket);
+    if (rightBracket != null) builder.rightBracket(rightBracket);
     writer.push(builder.build());
   }
 
@@ -389,7 +390,10 @@ mixin PieceFactory implements CommentWriter {
   /// Creates a [ListPiece] for a type argument or type parameter list.
   void createTypeList(
       Token leftBracket, Iterable<AstNode> elements, Token rightBracket) {
-    return createList(leftBracket, elements, rightBracket,
+    return createList(
+        leftBracket: leftBracket,
+        elements,
+        rightBracket: rightBracket,
         style: const ListStyle(commas: Commas.nonTrailing, splitCost: 2));
   }
 
@@ -427,6 +431,34 @@ mixin PieceFactory implements CommentWriter {
         isValueDelimited: rightHandSide.isDelimited));
   }
 
+  /// Writes the condition and updaters parts of a [ForParts] after the
+  /// subclass's initializer clause has been written.
+  void finishForParts(ForParts forLoopParts, DelimitedListBuilder partsList) {
+    token(forLoopParts.leftSeparator);
+    writer.split();
+    partsList.add(writer.pop());
+
+    // The condition clause.
+    if (forLoopParts.condition case var conditionExpression?) {
+      partsList.addCommentsBefore(conditionExpression.beginToken);
+      visit(conditionExpression);
+    } else {
+      partsList.addCommentsBefore(forLoopParts.rightSeparator);
+    }
+
+    token(forLoopParts.rightSeparator);
+    writer.split();
+    partsList.add(writer.pop());
+
+    // The update clauses.
+    if (forLoopParts.updaters.isNotEmpty) {
+      partsList.addCommentsBefore(forLoopParts.updaters.first.beginToken);
+      createList(forLoopParts.updaters,
+          style: const ListStyle(commas: Commas.nonTrailing));
+      partsList.add(writer.pop());
+    }
+  }
+
   /// Writes an optional modifier that precedes other code.
   void modifier(Token? keyword) {
     token(keyword, after: writer.space);

lib/src/piece/for.dart

@@ -0,0 +1,65 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+import '../back_end/code_writer.dart';
+import '../constants.dart';
+import 'piece.dart';
+
+/// A piece for a for statement.
+class ForPiece extends Piece {
+  /// The `for` keyword.
+  final Piece _forKeyword;
+
+  /// The part inside `( ... )`, including the parentheses themselves, at the
+  /// header of a for statement.
+  final Piece _forParts;
+
+  final Piece _body;
+
+  /// Whether the body of the loop is a block versus some other statement. If
+  /// the body isn't a block, then we allow a discretionary split after the
+  /// loop parts, as in:
+  ///
+  /// ```
+  /// for (;;)
+  ///   print("ok");
+  /// ```
+  final bool _hasBlockBody;
+
+  ForPiece(this._forKeyword, this._forParts, this._body,
+      {required bool hasBlockBody})
+      : _hasBlockBody = hasBlockBody;
+
+  /// If there is at least one else or else-if clause, then it always splits.
+  @override
+  List<State> get additionalStates => [if (!_hasBlockBody) State.split];
+
+  @override
+  void format(CodeWriter writer, State state) {
+    if (!_hasBlockBody && state == State.unsplit) {
+      writer.setAllowNewlines(false);
+    }
+
+    writer.format(_forKeyword);
+    writer.space();
+    writer.format(_forParts);
+
+    if (_hasBlockBody) {
+      writer.space();
+    } else {
+      writer.splitIf(state == State.split, indent: Indent.block);
+    }
+
+    writer.format(_body);
+  }
+
+  @override
+  void forEachChild(void Function(Piece piece) callback) {
+    callback(_forKeyword);
+    callback(_forParts);
+    callback(_body);
+  }
+
+  @override
+  String toString() => 'For';
+}

test/statement/for.stmt

@@ -0,0 +1,17 @@
+40 columns                              |
+### Test for loop formatting that isn't affected by the for loop parts.
+>>> No clauses.
+for  (  ;  ;  )  {  body;  }
+<<<
+for (;;) {
+  body;
+}
+>>> Single line for body without braces.
+for (i = 0; i < 10; i) something(i);
+<<<
+for (i = 0; i < 10; i) something(i);
+>>> Split for body without braces.
+for (i = 0; i < 10; i) somethingLonger(i);
+<<<
+for (i = 0; i < 10; i)
+  somethingLonger(i);