Default to requiring whitespace before (

There are only a few cases where `(` should be preceded by a space: closure parameters, function types and tuples.

Instead of defaulting to add whitespace before `(` check for those cases and if we are not in one of them, don’t require whitespace.

rdar://124569733

Author: ahoppen

Sources/SwiftBasicFormat/BasicFormat.swift

@@ -310,27 +310,18 @@ open class BasicFormat: SyntaxRewriter {
       (.backtick, _),
       (.dollarIdentifier, .period),  // a.b
       (.endOfFile, _),
-      (.exclamationMark, .leftParen),  // myOptionalClosure!()
       (.exclamationMark, .period),  // myOptionalBar!.foo()
-      (.regexPoundDelimiter, .leftParen),  // opening extended regex delimiter should never be separate by a space
       (.regexPoundDelimiter, .regexSlash),  // opening extended regex delimiter should never be separate by a space
       (.identifier, .leftAngle),  // MyType<Int>
-      (.identifier, .leftParen),  // foo()
       (.identifier, .leftSquare),  // myArray[1]
       (.identifier, .period),  // a.b
       (.integerLiteral, .period),  // macOS 11.2.1
       (.keyword(.Any), .period),  // Any.Type
       (.keyword(.`init`), .leftAngle),  // init<T>()
-      (.keyword(.`init`), .leftParen),  // init()
-      (.keyword(.private), .leftParen),  // private(set)
       (.keyword(.self), .period),  // self.someProperty
-      (.keyword(.self), .leftParen),  // self()
       (.keyword(.self), .leftSquare),  // self[]
       (.keyword(.Self), .period),  // Self.someProperty
-      (.keyword(.Self), .leftParen),  // Self()
       (.keyword(.Self), .leftSquare),  // Self[]
-      (.keyword(.set), .leftParen),  // var mVar: Int { set(value) {} }
-      (.keyword(.subscript), .leftParen),  // subscript(x: Int)
       (.keyword(.super), .period),  // super.someProperty
       (.leftBrace, .rightBrace),  // {}
       (.leftParen, _),
@@ -339,13 +330,10 @@ open class BasicFormat: SyntaxRewriter {
       (.multilineStringQuote, .rawStringPoundDelimiter),
       (.period, _),
       (.postfixQuestionMark, .leftAngle),  // init?<T>()
-      (.postfixQuestionMark, .leftParen),  // init?() or myOptionalClosure?()
       (.postfixQuestionMark, .period),  // someOptional?.someProperty
       (.pound, _),
-      (.poundUnavailable, .leftParen),  // #unavailable(...)
       (.prefixAmpersand, _),
       (.prefixOperator, _),
-      (.rawStringPoundDelimiter, .leftParen),  // opening raw string delimiter should never be separate by a space
       // opening raw string delimiter should never be separate by a space
       (.rawStringPoundDelimiter, .multilineStringQuote),
       (.rawStringPoundDelimiter, .singleQuote),  // opening raw string delimiter should never be separate by a space
@@ -355,10 +343,7 @@ open class BasicFormat: SyntaxRewriter {
       (.regexPoundDelimiter, .period),  // #/myRegex/#.someMember
       (.regexSlash, .regexPoundDelimiter),  // closing extended regex delimiter should never be separate by a space
       (.regexSlash, .period),  // /myRegex/.someMember
-      (.rightAngle, .leftParen),  // func foo<T>(x: T)
       (.rightAngle, .period),  // Foo<T>.bar
-      (.rightBrace, .leftParen),  // { return 1 }()
-      (.rightParen, .leftParen),  // returnsClosure()()
       (.rightParen, .period),  // foo().bar
       (.rightSquare, .period),  // myArray[1].someProperty
       (.singleQuote, .rawStringPoundDelimiter),  // closing raw string delimiter should never be separate by a space
@@ -378,9 +363,11 @@ open class BasicFormat: SyntaxRewriter {
       (nil, _):
       return false
     case (_, .colon):
-      if second?.keyPathInParent != \TernaryExprSyntax.colon
-        && second?.keyPathInParent != \UnresolvedTernaryExprSyntax.colon
-      {
+      switch second?.keyPathInParent {
+      case \TernaryExprSyntax.colon,
+        \UnresolvedTernaryExprSyntax.colon:
+        break
+      default:
         return false
       }
     case (.leftAngle, _) where second?.tokenKind != .rightAngle:
@@ -389,6 +376,18 @@ open class BasicFormat: SyntaxRewriter {
     case (_, .rightAngle) where first?.tokenKind != .leftAngle:
       // `<` and `>` need to be separated by a space because otherwise they become an operator
       return false
+    case (_, .leftParen):
+      switch second?.keyPathInParent {
+      case \ClosureParameterClauseSyntax.leftParen,
+        \FunctionTypeSyntax.leftParen,
+        \TupleExprSyntax.leftParen,
+        \TuplePatternSyntax.leftParen,
+        \TupleTypeSyntax.leftParen:
+        break
+      default:
+        return false
+      }
+
     default:
       break
     }

Tests/SwiftBasicFormatTest/BasicFormatTests.swift

@@ -43,6 +43,15 @@ fileprivate func assertFormatted(
   )
 }
 
+fileprivate func assertFormattingRoundTrips(
+  _ source: String,
+  using format: BasicFormat = BasicFormat(indentationWidth: .spaces(4)),
+  file: StaticString = #filePath,
+  line: UInt = #line
+) {
+  assertFormatted(source: source, expected: source, using: format, file: file, line: line)
+}
+
 final class BasicFormatTest: XCTestCase {
   func testNotIndented() {
     assertFormatted(
@@ -100,26 +109,26 @@ final class BasicFormatTest: XCTestCase {
   }
 
   func testAlreadyIndented() {
-    let source = """
+    assertFormattingRoundTrips(
+      """
       func foo() {
         someFunc(a: 1,
                  b: 1)
       }
       """
-
-    assertFormatted(source: source, expected: source)
+    )
   }
 
   func testAlreadyIndentedWithComment() {
-    let source = """
+    assertFormattingRoundTrips(
+      """
       func foo() {
         // ABC
         someFunc(a: 1,
                  b: 1)
       }
       """
-
-    assertFormatted(source: source, expected: source)
+    )
   }
 
   func testAlreadyIndentedWithComment2() {
@@ -278,7 +287,8 @@ final class BasicFormatTest: XCTestCase {
   }
 
   func testMultilineStringLiteralWithBlankLines() {
-    let source = #"""
+    assertFormattingRoundTrips(
+      #"""
       assertionFailure("""
 
         First line
@@ -287,29 +297,30 @@ final class BasicFormatTest: XCTestCase {
 
         """)
       """#
-    assertFormatted(source: source, expected: source)
+    )
   }
 
   func testMultilineStringLiteralWithFirstLineBlank() {
-    let source = #"""
+    assertFormattingRoundTrips(
+      #"""
       assertionFailure("""
 
         """)
       """#
-    assertFormatted(source: source, expected: source)
+    )
   }
 
   func testNestedMultilineStringLiterals() {
-    let source = #"""
+    assertFormattingRoundTrips(
+      #"""
       assertionFailure("""
 
         \("""
           First Line
         """)
       """)
       """#
-
-    assertFormatted(source: source, expected: source)
+    )
   }
 
   func testIndentNestedMultilineStringLiterals() throws {
@@ -375,7 +386,8 @@ final class BasicFormatTest: XCTestCase {
   }
 
   func testClosureInStringInterpolation() {
-    let source = #"""
+    assertFormattingRoundTrips(
+      #"""
       """
       \(gen { (x) in
           return """
@@ -384,8 +396,7 @@ final class BasicFormatTest: XCTestCase {
       })
       """
       """#
-
-    assertFormatted(source: source, expected: source)
+    )
   }
 
   func testNestedUserDefinedIndentation() {
@@ -540,31 +551,27 @@ final class BasicFormatTest: XCTestCase {
   }
 
   func testPeriodAfterStringLiteral() {
-    let source = """
+    assertFormattingRoundTrips(
+      """
       "test".lowercased()
       """
-    assertFormatted(source: source, expected: source)
+    )
   }
 
   func testPeriodAfterRawStringLiteral() {
-    let source = """
+    assertFormattingRoundTrips(
+      """
       #"test"#.lowercased()
       """
-    assertFormatted(source: source, expected: source)
+    )
   }
 
   func testPeriodAfterRegexLiteral() {
-    let source = """
-      /test/.something
-      """
-    assertFormatted(source: source, expected: source)
+    assertFormattingRoundTrips("/test/.something")
   }
 
   func testPeriodAfterRawRegexLiteral() {
-    let source = """
-      /test/.something
-      """
-    assertFormatted(source: source, expected: source)
+    assertFormattingRoundTrips("/test/.something")
   }
 
   func testNewlineInTrailingTriviaAtEndOfIndentationScope() throws {
@@ -627,30 +634,65 @@ final class BasicFormatTest: XCTestCase {
   }
 
   func testIndentedStandaloneClosureRoundTrips() throws {
-    let source = """
+    assertFormattingRoundTrips(
+      """
           foo {
               "abc"
           }
       """
-    assertFormatted(source: source, expected: source)
+    )
   }
 
   func testIndentedStandaloneClosureRoundTrips2() throws {
-    let source = """
+    assertFormattingRoundTrips(
+      """
           foo {
               if true {
                   print("test")
               }
           }
       """
-    assertFormatted(source: source, expected: source)
+    )
   }
 
   func testPrivateSetVar() {
-    let source = """
-      private(set) var x = 1
+    assertFormattingRoundTrips("private(set) var x = 1")
+
+    assertFormattingRoundTrips("internal(set) var x = 1")
+  }
+
+  func testSpiAttribute() {
+    assertFormattingRoundTrips(
+      """
+      @_spi(MySPI) struct Foo {
+      }
       """
+    )
+  }
 
-    assertFormatted(source: source, expected: source)
+  func testTypedThrows() {
+    assertFormattingRoundTrips(
+      """
+      func foo() throws(MyError) {
+      }
+      """
+    )
+  }
+
+  func testClosureParameterClause() {
+    assertFormatted(
+      source: "{(x: Int) in}",
+      expected: """
+        { (x: Int) in
+        }
+        """
+    )
+  }
+
+  func testFunctionType() {
+    assertFormatted(
+      source: "let x:(Int)->Void",
+      expected: "let x: (Int) -> Void"
+    )
   }
 }