Merge pull request #666 from allevato/various-fixes

Various fixes for some 509.0.0 bugs.

Author: allevato

Sources/SwiftFormat/PrettyPrint/TokenStreamCreator.swift

@@ -1894,11 +1894,31 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
   }
 
   override func visit(_ node: KeyPathSubscriptComponentSyntax) -> SyntaxVisitorContinueKind {
-    after(node.leftSquare, tokens: .break(.open, size: 0), .open)
-    before(node.rightSquare, tokens: .break(.close, size: 0), .close)
+    var breakBeforeRightParen = !isCompactSingleFunctionCallArgument(node.arguments)
+    if let component = node.parent?.as(KeyPathComponentSyntax.self) {
+      breakBeforeRightParen = !isLastKeyPathComponent(component)
+    }
+
+    arrangeFunctionCallArgumentList(
+      node.arguments,
+      leftDelimiter: node.leftSquare,
+      rightDelimiter: node.rightSquare,
+      forcesBreakBeforeRightDelimiter: breakBeforeRightParen)
     return .visitChildren
   }
 
+  /// Returns a value indicating whether the given key path component was the last component in the
+  /// list containing it.
+  private func isLastKeyPathComponent(_ component: KeyPathComponentSyntax) -> Bool {
+    guard
+      let componentList = component.parent?.as(KeyPathComponentListSyntax.self),
+      let lastComponent = componentList.last
+    else {
+      return false
+    }
+    return component == lastComponent
+  }
+
   override func visit(_ node: TernaryExprSyntax) -> SyntaxVisitorContinueKind {
     // The order of the .open/.close tokens here is intentional. They are normally paired with the
     // corresponding breaks, but in this case, we want to prioritize keeping the entire `? a : b`

Sources/SwiftFormat/Rules/NoEmptyTrailingClosureParentheses.swift

@@ -27,7 +27,10 @@ public final class NoEmptyTrailingClosureParentheses: SyntaxFormatRule {
     guard
       let trailingClosure = node.trailingClosure,
       let leftParen = node.leftParen,
-      node.arguments.isEmpty
+      let rightParen = node.rightParen,
+      node.arguments.isEmpty,
+      !leftParen.trailingTrivia.hasAnyComments,
+      !rightParen.leadingTrivia.hasAnyComments
     else {
       return super.visit(node)
     }

Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift

@@ -238,11 +238,12 @@ public final class UseShorthandTypeNames: SyntaxFormatRule {
   ) -> TypeSyntax {
     var wrappedType = wrappedType
 
-    // Function types and some-or-any types must be wrapped in parentheses before using shorthand
-    // optional syntax, otherwise the "?" will bind incorrectly (in the function case it binds to
-    // only the result, and in the some-or-any case it only binds to the child protocol). Attach the
-    // leading trivia to the left-paren that we're adding in these cases.
+    // Certain types must be wrapped in parentheses before using shorthand optional syntax to avoid
+    // the "?" from binding incorrectly when re-parsed. Attach the leading trivia to the left-paren
+    // that we're adding in these cases.
     switch Syntax(wrappedType).as(SyntaxEnum.self) {
+    case .attributedType(let attributedType):
+      wrappedType = parenthesizedType(attributedType, leadingTrivia: leadingTrivia)
     case .functionType(let functionType):
       wrappedType = parenthesizedType(functionType, leadingTrivia: leadingTrivia)
     case .someOrAnyType(let someOrAnyType):
@@ -319,12 +320,11 @@ public final class UseShorthandTypeNames: SyntaxFormatRule {
   ) -> OptionalChainingExprSyntax? {
     guard var wrappedTypeExpr = expressionRepresentation(of: wrappedType) else { return nil }
 
-    // Function types and some-or-any types must be wrapped in parentheses before using shorthand
-    // optional syntax, otherwise the "?" will bind incorrectly (in the function case it binds to
-    // only the result, and in the some-or-any case it only binds to the child protocol). Attach the
-    // leading trivia to the left-paren that we're adding in these cases.
+    // Certain types must be wrapped in parentheses before using shorthand optional syntax to avoid
+    // the "?" from binding incorrectly when re-parsed. Attach the leading trivia to the left-paren
+    // that we're adding in these cases.
     switch Syntax(wrappedType).as(SyntaxEnum.self) {
-    case .functionType, .someOrAnyType:
+    case .attributedType, .functionType, .someOrAnyType:
       wrappedTypeExpr = parenthesizedExpr(wrappedTypeExpr, leadingTrivia: leadingTrivia)
     default:
       // Otherwise, the argument type can safely become an optional by simply appending a "?". If
@@ -448,6 +448,9 @@ public final class UseShorthandTypeNames: SyntaxFormatRule {
     case .someOrAnyType(let someOrAnyType):
       return ExprSyntax(TypeExprSyntax(type: someOrAnyType))
 
+    case .attributedType(let attributedType):
+      return ExprSyntax(TypeExprSyntax(type: attributedType))
+
     default:
       return nil
     }

Tests/SwiftFormatTests/PrettyPrint/KeyPathExprTests.swift

@@ -131,7 +131,9 @@ final class KeyPathExprTests: PrettyPrintTestCase {
       #"""
       let x = \ReallyLongType.reallyLongProperty.anotherLongProperty
       let x = \.reeeeallyLongProperty.anotherLongProperty
+      let x = \.longProperty.a.b.c[really + long + expression]
       let x = \.longProperty.a.b.c[really + long + expression].anotherLongProperty
+      let x = \.longProperty.a.b.c[label:really + long + expression].anotherLongProperty
       """#
 
     let expected =
@@ -146,6 +148,16 @@ final class KeyPathExprTests: PrettyPrintTestCase {
       let x =
         \.longProperty.a.b.c[
           really + long
+            + expression]
+      let x =
+        \.longProperty.a.b.c[
+          really + long
+            + expression
+        ].anotherLongProperty
+      let x =
+        \.longProperty.a.b.c[
+          label: really
+            + long
             + expression
         ].anotherLongProperty
 

Tests/SwiftFormatTests/Rules/NoEmptyTrailingClosureParenthesesTests.swift

@@ -83,4 +83,29 @@ final class NoEmptyTrailingClosureParenthesesTests: LintOrFormatRuleTestCase {
       ]
     )
   }
+
+  func testDoNotRemoveParensContainingOnlyComments() {
+    assertFormatting(
+      NoEmptyTrailingClosureParentheses.self,
+      input: """
+        greetEnthusiastically(/*oldArg: x*/) { "John" }
+        greetEnthusiastically(
+          /*oldArg: x*/
+        ) { "John" }
+        greetEnthusiastically(
+          // oldArg: x
+        ) { "John" }
+        """,
+      expected: """
+        greetEnthusiastically(/*oldArg: x*/) { "John" }
+        greetEnthusiastically(
+          /*oldArg: x*/
+        ) { "John" }
+        greetEnthusiastically(
+          // oldArg: x
+        ) { "John" }
+        """,
+      findings: []
+    )
+  }
 }

Tests/SwiftFormatTests/Rules/UseShorthandTypeNamesTests.swift

@@ -692,4 +692,44 @@ final class UseShorthandTypeNamesTests: LintOrFormatRuleTestCase {
       ]
     )
   }
+
+  func testAttributedTypesInOptionalsAreParenthesized() {
+    // If we need to insert parentheses, verify that we do, but also verify that we don't insert
+    // them unnecessarily.
+    assertFormatting(
+      UseShorthandTypeNames.self,
+      input: """
+        var x: 1️⃣Optional<consuming P> = S()
+        var y: 2️⃣Optional<@Sendable (Int) -> Void> = S()
+        var z = [3️⃣Optional<consuming P>]([S()])
+        var a = [4️⃣Optional<@Sendable (Int) -> Void>]([S()])
+
+        var x: 5️⃣Optional<(consuming P)> = S()
+        var y: 6️⃣Optional<(@Sendable (Int) -> Void)> = S()
+        var z = [7️⃣Optional<(consuming P)>]([S()])
+        var a = [8️⃣Optional<(@Sendable (Int) -> Void)>]([S()])
+        """,
+      expected: """
+        var x: (consuming P)? = S()
+        var y: (@Sendable (Int) -> Void)? = S()
+        var z = [(consuming P)?]([S()])
+        var a = [(@Sendable (Int) -> Void)?]([S()])
+
+        var x: (consuming P)? = S()
+        var y: (@Sendable (Int) -> Void)? = S()
+        var z = [(consuming P)?]([S()])
+        var a = [(@Sendable (Int) -> Void)?]([S()])
+        """,
+      findings: [
+        FindingSpec("1️⃣", message: "use shorthand syntax for this 'Optional' type"),
+        FindingSpec("2️⃣", message: "use shorthand syntax for this 'Optional' type"),
+        FindingSpec("3️⃣", message: "use shorthand syntax for this 'Optional' type"),
+        FindingSpec("4️⃣", message: "use shorthand syntax for this 'Optional' type"),
+        FindingSpec("5️⃣", message: "use shorthand syntax for this 'Optional' type"),
+        FindingSpec("6️⃣", message: "use shorthand syntax for this 'Optional' type"),
+        FindingSpec("7️⃣", message: "use shorthand syntax for this 'Optional' type"),
+        FindingSpec("8️⃣", message: "use shorthand syntax for this 'Optional' type"),
+      ]
+    )
+  }
 }