Handle existing commas in single element collections.

The `whitespaceOnly` mode in the PrettyPrinter handle existing commas in single element collections incorrectly by always removing the comma. This happened because an earlier fix stopped placing `commaDelimitedRegion[Start|End]` tokens around single element collections, while still ignoring any existing trailing comma token. Rather than move the `whitespaceOnly` decision deep into the TokenStreamCreator, I've revised that previous fix so that all logic to decide when to keep and remove commas is localized inside the PrettyPrinter.

This was causing crashes when linting code that had a trailing comma in a single element collection, because `whitespaceOnly` mode was incorrectly causing non-whitespace changes to occur.

Fixes SR-12782

Author: dylansturg

Sources/SwiftFormatPrettyPrint/PrettyPrint.swift

@@ -541,12 +541,18 @@ public class PrettyPrinter {
     case .commaDelimitedRegionStart:
       commaDelimitedRegionStack.append(openCloseBreakCompensatingLineNumber)
 
-    case .commaDelimitedRegionEnd(let hasTrailingComma):
+    case .commaDelimitedRegionEnd(let hasTrailingComma, let isSingleElement):
       guard let startLineNumber = commaDelimitedRegionStack.popLast() else {
         fatalError("Found trailing comma end with no corresponding start.")
       }
 
-      let shouldHaveTrailingComma = startLineNumber != openCloseBreakCompensatingLineNumber
+      // We need to specifically disable trailing commas on elements of single item collections.
+      // The syntax library can't distinguish a collection's initializer (where the elements are
+      // types) from a literal (where the elements are the contents of a collection instance).
+      // We never want to add a trailing comma in an initializer so we disable trailing commas on
+      // single element collections.
+      let shouldHaveTrailingComma =
+        startLineNumber != openCloseBreakCompensatingLineNumber && !isSingleElement
       if shouldHaveTrailingComma && !hasTrailingComma {
         diagnose(.addTrailingComma)
       } else if !shouldHaveTrailingComma && hasTrailingComma {
@@ -653,14 +659,15 @@ public class PrettyPrinter {
       case .commaDelimitedRegionStart:
         lengths.append(0)
 
-      case .commaDelimitedRegionEnd:
+      case .commaDelimitedRegionEnd(_, let isSingleElement):
         // The token's length is only necessary when a comma will be printed, but it's impossible to
         // know at this point whether the region-start token will be on the same line as this token.
         // Without adding this length to the total, it would be possible for this comma to be
         // printed in column `maxLineLength`. Unfortunately, this can cause breaks to fire
         // unnecessarily when the enclosed tokens comma would fit within `maxLineLength`.
-        total += 1
-        lengths.append(1)
+        let length = isSingleElement ? 0 : 1
+        total += length
+        lengths.append(length)
       }
     }
 

Sources/SwiftFormatPrettyPrint/Token.swift

@@ -185,8 +185,8 @@ enum Token {
   case commaDelimitedRegionStart
 
   /// Marks the end of a comma delimited collection, where a trailing comma should be inserted
-  /// if and only if the collection spans multiple lines.
-  case commaDelimitedRegionEnd(hasTrailingComma: Bool)
+  /// if and only if the collection spans multiple lines and has multiple elements.
+  case commaDelimitedRegionEnd(hasTrailingComma: Bool, isSingleElement: Bool)
 
   /// Starts a scope where `contextual` breaks have consistent behavior.
   case contextualBreakingStart

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

@@ -795,16 +795,12 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
       if let trailingComma = lastElement.trailingComma {
         ignoredTokens.insert(trailingComma)
       }
-      // The syntax library can't distinguish an array initializer (where the elements are types)
-      // from an array literal (where the elements are the contents of the array). We never want to
-      // add a trailing comma in the initializer case and there's always exactly 1 element, so we
-      // never add a trailing comma to 1 element arrays.
-      if let firstElement = node.first, firstElement != lastElement {
-        before(firstElement.firstToken, tokens: .commaDelimitedRegionStart)
-        let endToken =
-          Token.commaDelimitedRegionEnd(hasTrailingComma: lastElement.trailingComma != nil)
-        after(lastElement.expression.lastToken, tokens: [endToken])
-      }
+      before(node.first?.firstToken, tokens: .commaDelimitedRegionStart)
+      let endToken =
+        Token.commaDelimitedRegionEnd(
+          hasTrailingComma: lastElement.trailingComma != nil,
+          isSingleElement: node.first == lastElement)
+      after(lastElement.expression.lastToken, tokens: [endToken])
     }
     return .visitChildren
   }
@@ -842,16 +838,12 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
       if let trailingComma = lastElement.trailingComma {
         ignoredTokens.insert(trailingComma)
       }
-      // The syntax library can't distinguish a dictionary initializer (where the elements are
-      // types) from a dictionary literal (where the elements are the contents of the dictionary).
-      // We never want to add a trailing comma in the initializer case and there's always exactly 1
-      //  element, so we never add a trailing comma to 1 element dictionaries.
-      if let firstElement = node.first, let lastElement = node.last, firstElement != lastElement {
-        before(firstElement.firstToken, tokens: .commaDelimitedRegionStart)
-        let endToken =
-          Token.commaDelimitedRegionEnd(hasTrailingComma: lastElement.trailingComma != nil)
-        after(lastElement.lastToken, tokens: endToken)
-      }
+      before(node.first?.firstToken, tokens: .commaDelimitedRegionStart)
+      let endToken =
+        Token.commaDelimitedRegionEnd(
+          hasTrailingComma: lastElement.trailingComma != nil,
+          isSingleElement: node.first == node.last)
+      after(lastElement.lastToken, tokens: endToken)
     }
     return .visitChildren
   }

Tests/SwiftFormatPrettyPrintTests/ArrayDeclTests.swift

@@ -103,6 +103,9 @@ final class ArrayDeclTests: PrettyPrintTestCase {
   func testWhitespaceOnlyDoesNotChangeTrailingComma() {
     let input =
       """
+      let a = [
+        "String",
+      ]
       let a = [1, 2, 3,]
       let a: [String] = [
         "One", "Two", "Three", "Four", "Five",

Tests/SwiftFormatPrettyPrintTests/DictionaryDeclTests.swift

@@ -92,6 +92,9 @@ final class DictionaryDeclTests: PrettyPrintTestCase {
   func testWhitespaceOnlyDoesNotChangeTrailingComma() {
     let input =
       """
+      let a = [
+        1: "a",
+      ]
       let a = [1: "a", 2: "b", 3: "c",]
       let a: [Int: String] = [
         1: "a", 2: "b", 3: "c", 4: "d", 5: "e", 6: "f",