Recursively apply NoEmptyTrailingClosureParentheses rule.

This rule wasn't being applied recursively, which meant it wouldn't be applied to the trailing closure of a function call where parens where removed.
swiftlang · Apr 30, 2020 · 5cca489 · 5cca489
1 parent 20162cd
commit 5cca489
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 9 deletions.
diff --git a/Sources/SwiftFormatRules/NoEmptyTrailingClosureParentheses.swift b/Sources/SwiftFormatRules/NoEmptyTrailingClosureParentheses.swift
@@ -22,22 +22,31 @@ import SwiftSyntax
 public final class NoEmptyTrailingClosureParentheses: SyntaxFormatRule {
 
   public override func visit(_ node: FunctionCallExprSyntax) -> ExprSyntax {
-    guard node.argumentList.count == 0 else { return ExprSyntax(node) }
+    guard node.argumentList.count == 0 else { return super.visit(node) }
 
-    guard node.trailingClosure != nil && node.argumentList.isEmpty && node.leftParen != nil else {
-      return ExprSyntax(node)
+    guard let trailingClosure = node.trailingClosure,
+      node.argumentList.isEmpty && node.leftParen != nil else
+    {
+      return super.visit(node)
     }
     guard let name = node.calledExpression.lastToken?.withoutTrivia() else {
-      return ExprSyntax(node)
+      return super.visit(node)
     }
 
     diagnose(.removeEmptyTrailingParentheses(name: "\(name)"), on: node)
 
+    // Need to visit `calledExpression` before creating a new node so that the location data (column
+    // and line numbers) is available.
+    guard let rewrittenCalledExpr = ExprSyntax(visit(Syntax(node.calledExpression))) else {
+      return super.visit(node)
+    }
     let formattedExp = replaceTrivia(
-      on: node.calledExpression,
-      token: node.calledExpression.lastToken,
+      on: rewrittenCalledExpr,
+      token: rewrittenCalledExpr.lastToken,
       trailingTrivia: .spaces(1))
+    let formattedClosure = visit(trailingClosure).as(ClosureExprSyntax.self)
     let result = node.withLeftParen(nil).withRightParen(nil).withCalledExpression(formattedExp)
+      .withTrailingClosure(formattedClosure)
     return ExprSyntax(result)
   }
 }

diff --git a/Tests/SwiftFormatRulesTests/NoEmptyTrailingClosureParenthesesTests.swift b/Tests/SwiftFormatRulesTests/NoEmptyTrailingClosureParenthesesTests.swift
@@ -19,6 +19,21 @@ final class NoEmptyTrailingClosureParenthesesTests: LintOrFormatRuleTestCase {
              func myfunc(cls: MyClass) {
                cls.myBadClosure() { $0 }
              }
+             DispatchQueue.main.async() {
+               greetEnthusiastically() { "John" }
+               DispatchQueue.main.async() {
+                 greetEnthusiastically() { "Willis" }
+               }
+             }
+             DispatchQueue.global.async(inGroup: blah) {
+               DispatchQueue.main.async() {
+                 greetEnthusiastically() { "Willis" }
+               }
+               DispatchQueue.main.async {
+                 greetEnthusiastically() { "Willis" }
+               }
+             }
+             foo(bar() { baz })() { blah }
              """,
       expected: """
                 func greetEnthusiastically(_ nameProvider: () -> String) {
@@ -35,9 +50,39 @@ final class NoEmptyTrailingClosureParenthesesTests: LintOrFormatRuleTestCase {
                 func myfunc(cls: MyClass) {
                   cls.myBadClosure { $0 }
                 }
-                """)
-    XCTAssertDiagnosed(.removeEmptyTrailingParentheses(name: "greetEnthusiastically"))
-    XCTAssertDiagnosed(.removeEmptyTrailingParentheses(name: "myBadClosure"))
+                DispatchQueue.main.async {
+                  greetEnthusiastically { "John" }
+                  DispatchQueue.main.async {
+                    greetEnthusiastically { "Willis" }
+                  }
+                }
+                DispatchQueue.global.async(inGroup: blah) {
+                  DispatchQueue.main.async {
+                    greetEnthusiastically { "Willis" }
+                  }
+                  DispatchQueue.main.async {
+                    greetEnthusiastically { "Willis" }
+                  }
+                }
+                foo(bar { baz }) { blah }
+                """,
+      checkForUnassertedDiagnostics: true)
+    XCTAssertDiagnosed(
+      .removeEmptyTrailingParentheses(name: "greetEnthusiastically"), line: 7, column: 1)
+    XCTAssertDiagnosed(.removeEmptyTrailingParentheses(name: "myBadClosure"), line: 13, column: 3)
     XCTAssertNotDiagnosed(.removeEmptyTrailingParentheses(name: "myClosure"))
+    XCTAssertDiagnosed(.removeEmptyTrailingParentheses(name: "async"), line: 15, column: 1)
+    XCTAssertDiagnosed(
+      .removeEmptyTrailingParentheses(name: "greetEnthusiastically"), line: 16, column: 3)
+    XCTAssertDiagnosed(.removeEmptyTrailingParentheses(name: "async"), line: 17, column: 3)
+    XCTAssertDiagnosed(
+      .removeEmptyTrailingParentheses(name: "greetEnthusiastically"), line: 18, column: 5)
+    XCTAssertDiagnosed(.removeEmptyTrailingParentheses(name: "async"), line: 22, column: 3)
+    XCTAssertDiagnosed(
+      .removeEmptyTrailingParentheses(name: "greetEnthusiastically"), line: 23, column: 5)
+    XCTAssertDiagnosed(
+      .removeEmptyTrailingParentheses(name: "greetEnthusiastically"), line: 26, column: 5)
+    XCTAssertDiagnosed(.removeEmptyTrailingParentheses(name: ")"), line: 29, column: 1)
+    XCTAssertDiagnosed(.removeEmptyTrailingParentheses(name: "bar"), line: 29, column: 5)
   }
 }