Merge pull request swiftlang#132 from dylansturg/multiline_closure_args

Wrap closure params in open/close breaks to indent inside parens.

Author: allevato

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

@@ -889,22 +889,43 @@ private final class TokenStreamCreator: SyntaxVisitor {
   }
 
   override func visit(_ node: ClosureSignatureSyntax) -> SyntaxVisitorContinueKind {
-    let consistency: GroupBreakStyle
-    if let input = node.input, input.is(ClosureParamListSyntax.self) {
-      consistency = argumentListConsistency()
-    } else {
-      consistency = .inconsistent
-    }
-
     before(node.firstToken, tokens: .open)
 
     if let input = node.input {
       // We unconditionally put a break before the `in` keyword below, so we should only put a break
       // after the capture list's right bracket if there are arguments following it or we'll end up
       // with an extra space if the line doesn't wrap.
       after(node.capture?.rightSquare, tokens: .break(.same))
-      before(input.firstToken, tokens: .open(consistency))
-      after(input.lastToken, tokens: .close)
+
+      // When it's parenthesized, the input is a `ParameterClauseSyntax`. Otherwise, it's a
+      // `ClosureParamListSyntax`. The parenthesized version is wrapped in open/close breaks so that
+      // the parens create an extra level of indentation.
+      if let parameterClause = input.as(ParameterClauseSyntax.self) {
+        // Whether we should prioritize keeping ") throws -> <return_type>" together. We can only do
+        // this if the closure has arguments.
+        let keepOutputTogether =
+          !parameterClause.parameterList.isEmpty && config.prioritizeKeepingFunctionOutputTogether
+
+        // Keep the output together by grouping from the right paren to the end of the output.
+        if keepOutputTogether {
+          // Due to visitation order, the matching .open break is added in ParameterClauseSyntax.
+          // Since the output clause is optional but the in-token is required, placing the .close
+          // before `inTok` ensures the close gets into the token stream.
+          before(node.inTok, tokens: .close)
+        } else  {
+          // Group outside of the parens, so that the argument list together, preferring to break
+          // between the argument list and the output.
+          before(input.firstToken, tokens: .open)
+          after(input.lastToken, tokens: .close)
+        }
+
+        arrangeParameterClause(parameterClause, forcesBreakBeforeRightParen: true)
+      } else {
+        // Group around the arguments, but don't use open/close breaks because there are no parens
+        // to create a new scope.
+        before(input.firstToken, tokens: .open(argumentListConsistency()))
+        after(input.lastToken, tokens: .close)
+      }
     }
 
     before(node.throwsTok, tokens: .break)

Tests/SwiftFormatPrettyPrintTests/ClosureExprTests.swift

@@ -14,6 +14,7 @@ public class ClosureExprTests: PrettyPrintTestCase {
       funcCall(closure: { s1, s2, s3, s4, s5, s6, s7, s8, s9, s10 in return s1 })
       funcCall(param1: 123, closure: { s1, s2, s3 in return s1 })
       funcCall(closure: { (s1: String, s2: String) -> Bool in return s1 > s2 })
+      funcCall(closure: { (s1: String, s2: String, s3: String, s4: String, s5: String) -> Bool in return s1 > s2 })
       """
 
     let expected =
@@ -53,6 +54,15 @@ public class ClosureExprTests: PrettyPrintTestCase {
         (s1: String, s2: String) -> Bool in
         return s1 > s2
       })
+      funcCall(closure: {
+        (
+          s1: String,
+          s2: String,
+          s3: String,
+          s4: String,
+          s5: String
+        ) -> Bool in return s1 > s2
+      })
 
       """
 
@@ -73,6 +83,7 @@ public class ClosureExprTests: PrettyPrintTestCase {
       funcCall(closure: { s1, s2, s3, s4, s5, s6, s7, s8, s9, s10 in return s1 })
       funcCall(param1: 123, closure: { s1, s2, s3 in return s1 })
       funcCall(closure: { (s1: String, s2: String) -> Bool in return s1 > s2 })
+      funcCall(closure: { (s1: String, s2: String, s3: String, s4: String, s5: String) -> Bool in return s1 > s2 })
       """
 
     let expected =
@@ -98,6 +109,12 @@ public class ClosureExprTests: PrettyPrintTestCase {
         (s1: String, s2: String) -> Bool in
         return s1 > s2
       })
+      funcCall(closure: {
+        (
+          s1: String, s2: String, s3: String,
+          s4: String, s5: String
+        ) -> Bool in return s1 > s2
+      })
 
       """
 
@@ -113,6 +130,7 @@ public class ClosureExprTests: PrettyPrintTestCase {
       funcCall(param1: 2) { $1 < $2 }
       funcCall(param1: 2) { s1, s2, s3 in return s1}
       funcCall(param1: 2) { s1, s2, s3, s4, s5 in return s1}
+      funcCall(param1: 2) { (s1: String, s2: String, s3: String, s4: String, s5: String) -> Bool in return s1 > s2 }
       """
 
     let expected =
@@ -125,6 +143,12 @@ public class ClosureExprTests: PrettyPrintTestCase {
       funcCall(param1: 2) {
         s1, s2, s3, s4, s5 in return s1
       }
+      funcCall(param1: 2) {
+        (
+          s1: String, s2: String, s3: String,
+          s4: String, s5: String
+        ) -> Bool in return s1 > s2
+      }
 
       """
 
@@ -345,4 +369,76 @@ public class ClosureExprTests: PrettyPrintTestCase {
 
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 60)
   }
+
+  public func testClosureOutputGrouping() {
+    let input =
+      """
+      funcCall(closure: {
+      (s1: String, s2: String, s3: String) throws -> Bool in return s1 > s2
+      })
+      funcCall(closure: {
+      (s1: String, s2: String, s3: String) throws -> AVeryLongReturnTypeThatOverflowsFiftyColumns in return s1 > s2
+      })
+      funcCall(closure: {
+      () throws -> Bool in return s1 > s2
+      })
+      funcCall(closure: {
+      () throws -> AVeryLongReturnTypeThatOverflowsFiftyColumns in return s1 > s2
+      })
+      """
+
+    let expectedNotGroupingOutput =
+      """
+      funcCall(closure: {
+        (s1: String, s2: String, s3: String) throws
+          -> Bool in return s1 > s2
+      })
+      funcCall(closure: {
+        (s1: String, s2: String, s3: String) throws
+          -> AVeryLongReturnTypeThatOverflowsFiftyColumns
+        in return s1 > s2
+      })
+      funcCall(closure: {
+        () throws -> Bool in return s1 > s2
+      })
+      funcCall(closure: {
+        () throws
+          -> AVeryLongReturnTypeThatOverflowsFiftyColumns
+        in return s1 > s2
+      })
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expectedNotGroupingOutput, linelength: 50)
+
+    let expectedKeepingOutputTogether =
+      """
+      funcCall(closure: {
+        (
+          s1: String, s2: String, s3: String
+        ) throws -> Bool in return s1 > s2
+      })
+      funcCall(closure: {
+        (
+          s1: String, s2: String, s3: String
+        ) throws
+          -> AVeryLongReturnTypeThatOverflowsFiftyColumns
+        in return s1 > s2
+      })
+      funcCall(closure: {
+        () throws -> Bool in return s1 > s2
+      })
+      funcCall(closure: {
+        () throws
+          -> AVeryLongReturnTypeThatOverflowsFiftyColumns
+        in return s1 > s2
+      })
+
+      """
+
+    var config = Configuration()
+    config.prioritizeKeepingFunctionOutputTogether = true
+    assertPrettyPrintEqual(
+      input: input, expected: expectedKeepingOutputTogether, linelength: 50, configuration: config)
+  }
 }