Merge pull request swiftlang#151 from dylansturg/no_lonely_commas

Absorb trailing comma of function call args in grouped exprs.

Author: allevato

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

@@ -699,6 +699,9 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
     before(node.firstToken, tokens: .open)
     after(node.colon, tokens: .break(.continue, newlines: .elective(ignoresDiscretionary: true)))
     after(node.lastToken, tokens: .close)
+    if let trailingComma = node.trailingComma {
+      closingDelimiterTokens.insert(trailingComma)
+    }
   }
 
   override func visit(_ node: ArrayExprSyntax) -> SyntaxVisitorContinueKind {
@@ -729,6 +732,9 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
   override func visit(_ node: ArrayElementSyntax) -> SyntaxVisitorContinueKind {
     before(node.firstToken, tokens: .open)
     after(node.lastToken, tokens: .close)
+    if let trailingComma = node.trailingComma {
+      closingDelimiterTokens.insert(trailingComma)
+    }
     return .visitChildren
   }
 
@@ -766,6 +772,9 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
     before(node.firstToken, tokens: .open)
     after(node.colon, tokens: .break(.continue, newlines: .elective(ignoresDiscretionary: true)))
     after(node.lastToken, tokens: .close)
+    if let trailingComma = node.trailingComma {
+      closingDelimiterTokens.insert(trailingComma)
+    }
     return .visitChildren
   }
 
@@ -837,6 +846,9 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
 
     let shouldGroupAroundArgument = !isCompactSingleFunctionCallArgument(arguments)
     for argument in arguments {
+      if let trailingComma = argument.trailingComma {
+        closingDelimiterTokens.insert(trailingComma)
+      }
       arrangeAsFunctionCallArgument(argument, shouldGroup: shouldGroupAroundArgument)
     }
   }
@@ -1626,6 +1638,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
       // If this is one of multiple comma-delimited bindings, move any pending close breaks to
       // follow the comma so that it doesn't get separated from the tokens before it.
       closeAfterToken = trailingComma
+      closingDelimiterTokens.insert(trailingComma)
     }
 
     if closeAfterToken != nil && closesNeeded > 0 {

Tests/SwiftFormatPrettyPrintTests/ArrayDeclTests.swift

@@ -111,4 +111,27 @@ final class ArrayDeclTests: PrettyPrintTestCase {
     XCTAssertDiagnosed(.removeTrailingComma, line: 1, column: 17)
     XCTAssertDiagnosed(.addTrailingComma, line: 4, column: 26)
   }
+
+  func testGroupsTrailingComma() {
+    let input =
+      """
+      let a = [
+        condition ? firstOption : secondOption,
+        bar(),
+      ]
+      """
+
+    let expected =
+      """
+      let a = [
+        condition
+          ? firstOption
+          : secondOption,
+        bar(),
+      ]
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 32)
+  }
 }

Tests/SwiftFormatPrettyPrintTests/DictionaryDeclTests.swift

@@ -133,4 +133,27 @@ final class DictionaryDeclTests: PrettyPrintTestCase {
 
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 20)
   }
+
+  func testGroupsTrailingComma() {
+    let input =
+      """
+      let d = [
+        key: cond ? firstOption : secondOption,
+        key2: bar(),
+      ]
+      """
+
+    let expected =
+      """
+      let d = [
+        key: cond
+          ? firstOption
+          : secondOption,
+        key2: bar(),
+      ]
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 32)
+  }
 }

Tests/SwiftFormatPrettyPrintTests/FunctionCallTests.swift

@@ -249,4 +249,25 @@ final class FunctionCallTests: PrettyPrintTestCase {
 
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 20)
   }
+
+  func testGroupsTrailingComma() {
+    let input =
+      """
+      foo(
+        image: useLongName ? image(named: .longNameImage) : image(named: .veryLongNameImageZ),
+        bar: bar)
+      """
+
+    let expected =
+      """
+      foo(
+        image: useLongName
+          ? image(named: .longNameImage)
+          : image(named: .veryLongNameImageZ),
+        bar: bar)
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 70)
+  }
 }

Tests/SwiftFormatPrettyPrintTests/ObjectLiteralExprTests.swift

@@ -110,4 +110,25 @@ final class ObjectLiteralExprTests: PrettyPrintTestCase {
     config.lineBreakBeforeEachArgument = false
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 38, configuration: config)
   }
+
+  func testGroupsTrailingComma() {
+    let input =
+      """
+      #imageLiteral(
+        image: useLongName ? image(named: .longNameImage) : image(named: .veryLongNameImageZ),
+        bar: bar)
+      """
+
+    let expected =
+      """
+      #imageLiteral(
+        image: useLongName
+          ? image(named: .longNameImage)
+          : image(named: .veryLongNameImageZ),
+        bar: bar)
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 70)
+  }
 }

Tests/SwiftFormatPrettyPrintTests/PatternBindingTests.swift

@@ -43,4 +43,27 @@ final class PatternBindingTests: PrettyPrintTestCase {
 
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 28)
   }
+
+  func testGroupingIncludesTrailingComma() {
+    let input =
+      """
+      let foo =  veryLongCondition
+        ? firstOption
+        : secondOption,
+        bar = bar()
+      """
+
+    let expected =
+      """
+      let
+        foo =
+          veryLongCondition
+          ? firstOption
+          : secondOption,
+        bar = bar()
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 18)
+  }
 }

Tests/SwiftFormatPrettyPrintTests/SubscriptExprTests.swift

@@ -85,4 +85,25 @@ final class SubscriptExprTests: PrettyPrintTestCase {
 
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
   }
+
+  func testGroupsTrailingComma() {
+    let input =
+      """
+      myCollection[
+        image: useLongName ? image(named: .longNameImage) : image(named: .veryLongNameImageZ),
+        bar: bar]
+      """
+
+    let expected =
+      """
+      myCollection[
+        image: useLongName
+          ? image(named: .longNameImage)
+          : image(named: .veryLongNameImageZ),
+        bar: bar]
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 70)
+  }
 }

Tests/SwiftFormatPrettyPrintTests/TupleDeclTests.swift

@@ -78,4 +78,27 @@ final class TupleDeclTests: PrettyPrintTestCase {
 
     assertPrettyPrintEqual(input: input, expected: expected, linelength: 25)
   }
+
+  func testGroupsTrailingComma() {
+    let input =
+      """
+      let t = (
+        condition ? firstOption : secondOption,
+        bar()
+      )
+      """
+
+    let expected =
+      """
+      let t = (
+        condition
+          ? firstOption
+          : secondOption,
+        bar()
+      )
+
+      """
+
+    assertPrettyPrintEqual(input: input, expected: expected, linelength: 32)
+  }
 }

Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift

@@ -24,6 +24,7 @@ extension ArrayDeclTests {
     static let __allTests__ArrayDeclTests = [
         ("testArrayOfFunctions", testArrayOfFunctions),
         ("testBasicArrays", testBasicArrays),
+        ("testGroupsTrailingComma", testGroupsTrailingComma),
         ("testNoTrailingCommasInTypes", testNoTrailingCommasInTypes),
         ("testTrailingCommaDiagnostics", testTrailingCommaDiagnostics),
         ("testWhitespaceOnlyDoesNotChangeTrailingComma", testWhitespaceOnlyDoesNotChangeTrailingComma),
@@ -181,6 +182,7 @@ extension DictionaryDeclTests {
     // to regenerate.
     static let __allTests__DictionaryDeclTests = [
         ("testBasicDictionaries", testBasicDictionaries),
+        ("testGroupsTrailingComma", testGroupsTrailingComma),
         ("testIgnoresDiscretionaryNewlineAfterColon", testIgnoresDiscretionaryNewlineAfterColon),
         ("testNoTrailingCommasInTypes", testNoTrailingCommasInTypes),
         ("testTrailingCommaDiagnostics", testTrailingCommaDiagnostics),
@@ -270,6 +272,7 @@ extension FunctionCallTests {
         ("testBasicFunctionCalls_packArguments", testBasicFunctionCalls_packArguments),
         ("testDiscretionaryLineBreakBeforeClosingParenthesis", testDiscretionaryLineBreakBeforeClosingParenthesis),
         ("testDiscretionaryLineBreaksAreSelfCorrecting", testDiscretionaryLineBreaksAreSelfCorrecting),
+        ("testGroupsTrailingComma", testGroupsTrailingComma),
         ("testIgnoresDiscretionaryLineBreakAfterColon", testIgnoresDiscretionaryLineBreakAfterColon),
         ("testNestedFunctionCallExprSequences", testNestedFunctionCallExprSequences),
         ("testSingleUnlabeledArgumentWithDelimiters", testSingleUnlabeledArgumentWithDelimiters),
@@ -473,6 +476,7 @@ extension ObjectLiteralExprTests {
     static let __allTests__ObjectLiteralExprTests = [
         ("testColorLiteral_noPackArguments", testColorLiteral_noPackArguments),
         ("testColorLiteral_packArguments", testColorLiteral_packArguments),
+        ("testGroupsTrailingComma", testGroupsTrailingComma),
         ("testImageLiteral_noPackArguments", testImageLiteral_noPackArguments),
         ("testImageLiteral_packArguments", testImageLiteral_packArguments),
     ]
@@ -508,6 +512,7 @@ extension PatternBindingTests {
     // to regenerate.
     static let __allTests__PatternBindingTests = [
         ("testBindingIncludingTypeAnnotation", testBindingIncludingTypeAnnotation),
+        ("testGroupingIncludesTrailingComma", testGroupingIncludesTrailingComma),
         ("testIgnoresDiscretionaryNewlineAfterColon", testIgnoresDiscretionaryNewlineAfterColon),
     ]
 }
@@ -660,6 +665,7 @@ extension SubscriptExprTests {
     static let __allTests__SubscriptExprTests = [
         ("testBasicSubscriptGetters", testBasicSubscriptGetters),
         ("testBasicSubscriptSetters", testBasicSubscriptSetters),
+        ("testGroupsTrailingComma", testGroupsTrailingComma),
         ("testSubscriptGettersWithTrailingClosures", testSubscriptGettersWithTrailingClosures),
         ("testSubscriptSettersWithTrailingClosures", testSubscriptSettersWithTrailingClosures),
     ]
@@ -715,6 +721,7 @@ extension TupleDeclTests {
     // to regenerate.
     static let __allTests__TupleDeclTests = [
         ("testBasicTuples", testBasicTuples),
+        ("testGroupsTrailingComma", testGroupsTrailingComma),
         ("testIgnoresDiscretionaryNewlineAfterColon", testIgnoresDiscretionaryNewlineAfterColon),
         ("testLabeledTuples", testLabeledTuples),
     ]