Handle trailing commas in for-loop updaters. (#1616)

Fix #1354.

Author: munificent

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## 3.0.1-wip
 
+* Handle trailing commas in for-loop updaters (#1354).
 * Handle comments and metadata before variables more gracefully (#1604).
 * Ensure comment formatting is idempotent (#1606).
 * Better indentation of leading comments on property accesses in binary operator

lib/src/front_end/delimited_list_builder.dart

@@ -143,18 +143,18 @@ final class DelimitedListBuilder {
     pieces.forEach(add);
   }
 
-  /// Adds the contents of [lineBuilder] to this outer [DelimitedListBuilder].
+  /// Adds the contents of [inner] 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) {
+  /// This is used when a [DelimiterListBuilder] is building a piece that will
+  /// then become an element in a surrounding [DelimitedListBuilder]. It ensures
+  /// that any comments around a trailing comma after [inner] don't get lost and
+  /// are instead hoisted up to be captured by this builder.
+  void addInnerBuilder(DelimitedListBuilder inner) {
     // Add the elements of the line to this builder.
-    add(lineBuilder.build());
+    add(inner.build());
 
     // Make sure that any trailing comments on the line aren't lost.
-    _commentsBeforeComma = lineBuilder._commentsBeforeComma;
+    _commentsBeforeComma = inner._commentsBeforeComma;
   }
 
   /// Writes any comments appearing before [token] to the list.

lib/src/front_end/piece_factory.dart

@@ -420,7 +420,16 @@ mixin PieceFactory {
         // The update clauses.
         if (forParts.updaters.isNotEmpty) {
           partsList.addCommentsBefore(forParts.updaters.first.beginToken);
-          partsList.add(createCommaSeparated(forParts.updaters));
+
+          // Create a nested list builder for the updaters so that they can
+          // remain unsplit even while the clauses split.
+          var updaterBuilder = DelimitedListBuilder(
+              this, const ListStyle(commas: Commas.nonTrailing));
+          forParts.updaters.forEach(updaterBuilder.visit);
+
+          // Add the updater builder to the clause builder so that any comments
+          // around a trailing comma after the updaters don't get dropped.
+          partsList.addInnerBuilder(updaterBuilder);
         }
 
         partsList.rightBracket(rightParenthesis);
@@ -1076,7 +1085,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.addLineBuilder(lineBuilder);
+        builder.addInnerBuilder(lineBuilder);
         lineBuilder = DelimitedListBuilder(this, lineStyle);
         atLineStart = true;
       }
@@ -1092,7 +1101,7 @@ mixin PieceFactory {
     }
 
     // Finish the last line if there is anything on it.
-    if (!atLineStart) builder.addLineBuilder(lineBuilder);
+    if (!atLineStart) builder.addInnerBuilder(lineBuilder);
   }
 
   /// Writes a [VariablePiece] for a named or wildcard variable pattern.

test/tall/expression/collection_for.stmt

@@ -170,4 +170,18 @@ var list = [
     inc
   )
     element,
+];
+>>> Trailing comma in increments.
+var list = [
+  for (
+    x = 1;
+    true;
+    x += 1, x += 2,
+  )
+    element,
+];
+<<<
+var list = [
+  for (x = 1; true; x += 1, x += 2)
+    element,
 ];

test/tall/regression/1300/1354.stmt

@@ -0,0 +1,12 @@
+>>>
+void main() {
+  for (int i; i < 10; print("foo"), ++i, print("bar"),) {
+    break;
+  }
+}
+<<<
+void main() {
+  for (int i; i < 10; print("foo"), ++i, print("bar")) {
+    break;
+  }
+}

test/tall/statement/for_comment.stmt

@@ -223,6 +223,22 @@ for (
 ) {
   body;
 }
+>>> Preserve comments around discarded increment trailing comma.
+for (
+  init;
+  cond;
+  incr /* c1 */ , /* c2 */
+) {
+  body;
+}
+<<<
+for (
+  init;
+  cond;
+  incr /* c1 */ /* c2 */
+) {
+  body;
+}
 >>> Line comment before first `;` in fully empty clauses.
 for  (  // comment
 ;  ;  )  {  body;  }

test/tall/statement/for_initializer.stmt

@@ -155,4 +155,19 @@ for (
   second = 2,
   third = 3,
   fourth = 4
-) {}
+) {}
+>>> Discard trailing comma in unsplit increments.
+for (foo; bar; first = 1, second = 2,) {}
+<<<
+for (foo; bar; first = 1, second = 2) {}
+>>> Discard trailing comma in split increments.
+for (foo; bar; first = 1, second = 2, third = 3, fourth = 4,) {}
+<<<
+for (
+  foo;
+  bar;
+  first = 1,
+  second = 2,
+  third = 3,
+  fourth = 4
+) {}