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

Closures have an `input` clause which can either be parenthesized or not. When the clause is parenthesized, it's a different type of syntax. We want the formatter to create extra indentation inside of the parens in that case, just like it would for function call argument lists. To do this, I have added an open and a close break inside of the parens to create a block scope around the closure parameters.

Author: dylansturg

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)
+  }
 }