Format metadata on patterns in for loops. (#1183)

Also fix a couple of minor issues I noticed when testing against the
Dart SDK repo:

- Gracefully handle empty switch expressions.
- Handle sync* and async* functions with => bodies.

Author: munificent

CHANGELOG.md

@@ -1,6 +1,7 @@
 # 2.2.5-dev
 
 * Format patterns and related features.
+* Handle `sync*` and `async*` functions with `=>` bodies.
 * Allow switch statements where all case bodies are on the same line as the
   case when they all fit.
 * Fix bug where parameter metadata wouldn't always split when it should.

lib/src/source_visitor.dart

@@ -1071,8 +1071,10 @@ class SourceVisitor extends ThrowingAstVisitor {
     // Space after the parameter list.
     space();
 
-    // The "async" or "sync" keyword.
-    token(node.keyword, after: space);
+    // The "async" or "sync" keyword and "*".
+    token(node.keyword);
+    token(node.star);
+    if (node.keyword != null || node.star != null) space();
 
     // Try to keep the "(...) => " with the start of the body for anonymous
     // functions.
@@ -1507,6 +1509,7 @@ class SourceVisitor extends ThrowingAstVisitor {
   @override
   void visitForEachPartsWithPattern(ForEachPartsWithPattern node) {
     builder.startBlockArgumentNesting();
+    visitNodes(node.metadata, between: split, after: split);
     token(node.keyword);
     space();
     visit(node.pattern);
@@ -1524,7 +1527,7 @@ class SourceVisitor extends ThrowingAstVisitor {
     builder.startRule();
 
     var declaration = node.variables;
-    visitMetadata(declaration.metadata);
+    visitNodes(declaration.metadata, between: split, after: split);
     modifier(declaration.keyword);
     visit(declaration.type, after: space);
 
@@ -1547,8 +1550,18 @@ class SourceVisitor extends ThrowingAstVisitor {
   @override
   void visitForPartsWithPattern(ForPartsWithPattern node) {
     builder.startBlockArgumentNesting();
-    visit(node.variables);
+    builder.nestExpression();
+
+    var declaration = node.variables;
+    visitNodes(declaration.metadata, between: split, after: split);
+    token(declaration.keyword);
+    space();
+    visit(declaration.pattern);
+    _visitAssignment(declaration.equals, declaration.expression);
+
+    builder.unnest();
     builder.endBlockArgumentNesting();
+
     _visitForPartsFromLeftSeparator(node);
   }
 
@@ -2714,7 +2727,10 @@ class SourceVisitor extends ThrowingAstVisitor {
 
     visitCommaSeparatedNodes(node.cases, between: split);
 
-    var hasTrailingComma = node.cases.last.commaAfter != null;
+    // A switch with no cases isn't syntactically valid, but handle it
+    // gracefully instead of crashing.
+    var hasTrailingComma =
+        node.cases.isNotEmpty && node.cases.last.commaAfter != null;
     _endBody(node.rightBracket, forceSplit: hasTrailingComma);
   }
 

test/whitespace/functions.unit

@@ -72,6 +72,22 @@ main()sync  *{var lambda = ()sync *{};}
 main() sync* {
   var lambda = () sync* {};
 }
+>>> sync* function with arrow body
+main()sync  *  =>  [];
+<<<
+main() sync* => [];
+>>> sync* getter with arrow body
+Iterable get main  sync  *  =>  [];
+<<<
+Iterable get main sync* => [];
+>>> async* function with arrow body
+main()async  *  =>  [];
+<<<
+main() async* => [];
+>>> async* getter with arrow body
+Stream get main  async  *  =>  [];
+<<<
+Stream get main async* => [];
 >>> trailing comma in single parameter list
 function(parameter   ,   ) {;}
 <<<

test/whitespace/metadata.unit

@@ -318,6 +318,105 @@ main() {
     ;
   }
 }
+>>> single metadata on for loop
+main() {
+  for (   @a    var i = x;;) {;}
+}
+<<<
+main() {
+  for (@a var i = x;;) {
+    ;
+  }
+}
+>>> multiple metadata on for loop
+main() {
+  for (   @a  @b    var i = x;;) {;}
+}
+<<<
+main() {
+  for (@a @b var i = x;;) {
+    ;
+  }
+}
+>>> long metadata on for loop
+main() {
+  for (@Annotation @VeryLongMetadataAnnotation(1, 2) var i = veryLong.iterator + expression;;) {;}
+}
+<<<
+main() {
+  for (@Annotation
+      @VeryLongMetadataAnnotation(1, 2)
+      var i = veryLong.iterator +
+          expression;;) {
+    ;
+  }
+}
+>>> single metadata on pattern for-in loop
+main() {
+  for (   @a    var [i] in list) {;}
+}
+<<<
+main() {
+  for (@a var [i] in list) {
+    ;
+  }
+}
+>>> multiple metadata on pattern for-in loop
+main() {
+  for (   @a  @b    var [i] in list) {;}
+}
+<<<
+main() {
+  for (@a @b var [i] in list) {
+    ;
+  }
+}
+>>> long metadata on pattern for-in loop
+main() {
+  for (@Annotation @VeryLongMetadataAnnotation(1, 2) var [i] in veryLong.iterator + expression) {;}
+}
+<<<
+main() {
+  for (@Annotation
+      @VeryLongMetadataAnnotation(1, 2)
+      var [i] in veryLong.iterator +
+          expression) {
+    ;
+  }
+}
+>>> single metadata on pattern for loop
+main() {
+  for (   @a    var [i] = x;;) {;}
+}
+<<<
+main() {
+  for (@a var [i] = x;;) {
+    ;
+  }
+}
+>>> multiple metadata on pattern for loop
+main() {
+  for (   @a  @b    var [i] = x;;) {;}
+}
+<<<
+main() {
+  for (@a @b var [i] = x;;) {
+    ;
+  }
+}
+>>> long metadata on pattern for loop
+main() {
+  for (@Annotation @VeryLongMetadataAnnotation(1, 2) var [i] = veryLong.iterator + expression;;) {;}
+}
+<<<
+main() {
+  for (@Annotation
+      @VeryLongMetadataAnnotation(1, 2)
+      var [i] = veryLong.iterator +
+          expression;;) {
+    ;
+  }
+}
 >>> metadata on enum cases
 enum Foo { a, @meta b, @meta1 @meta2 c}
 <<<

test/whitespace/switch.stmt

@@ -290,4 +290,8 @@ switch (obj) {
   case 's'.length:
   case 1 is int:
   case 1 is! int:
-}
+}
+>>> empty switch expression (error but handle gracefully)
+var x = switch(y) {};
+<<<
+var x = switch (y) {  };