Merge pull request #2104 from ahoppen/ahoppen/closure-parsing

Add test cases for parsing closures inside `if` statement conditions

Author: ahoppen

Sources/SwiftBasicFormat/BasicFormat.swift

@@ -123,14 +123,7 @@ open class BasicFormat: SyntaxRewriter {
   // MARK: - Helper functions
 
   private func isInsideStringInterpolation(_ token: TokenSyntax) -> Bool {
-    var ancestor: Syntax = Syntax(token)
-    while let parent = ancestor.parent {
-      ancestor = parent
-      if ancestor.is(ExpressionSegmentSyntax.self) {
-        return true
-      }
-    }
-    return false
+    return token.ancestorOrSelf { $0.as(ExpressionSegmentSyntax.self) } != nil
   }
 
   private func childrenSeparatedByNewline(_ node: Syntax) -> Bool {
@@ -220,6 +213,31 @@ open class BasicFormat: SyntaxRewriter {
     }
   }
 
+  /// Returns `true` if `token` is the opening brace of a closure that is being
+  /// parsed in an expression with `ExprFlavor.stmtCondition`.
+  ///
+  /// In these cases, adding a newline changes whether the closure gets parsed
+  /// as a closure or if it gets interpreted as the statements body. We should
+  /// thus be conservative and not add a newline after the `{` in `BasicFormat`.
+  private func isLeftBraceOfClosureInStmtConditionExpr(_ token: TokenSyntax?) -> Bool {
+    guard let token, token.keyPathInParent == \ClosureExprSyntax.leftBrace else {
+      return false
+    }
+    return token.ancestorOrSelf(mapping: {
+      switch $0.keyPathInParent {
+      case \CatchItemSyntax.pattern,
+        \ConditionElementSyntax.condition,
+        \ExpressionPatternSyntax.expression,
+        \ForStmtSyntax.sequence,
+        \ForStmtSyntax.whereClause,
+        \SwitchExprSyntax.subject:
+        return $0
+      default:
+        return nil
+      }
+    }) != nil
+  }
+
   open func requiresNewline(between first: TokenSyntax?, and second: TokenSyntax?) -> Bool {
     // We don't want to add newlines inside string interpolation.
     // When first or second ``TokenSyntax`` is a multiline quote we want special handling
@@ -231,6 +249,8 @@ open class BasicFormat: SyntaxRewriter {
       second?.tokenKind != .multilineStringQuote
     {
       return false
+    } else if isLeftBraceOfClosureInStmtConditionExpr(first) {
+      return false
     } else if let second {
       var ancestor: Syntax = Syntax(second)
       while let parent = ancestor.parent {
@@ -607,3 +627,17 @@ fileprivate extension TokenSyntax {
     }
   }
 }
+
+fileprivate extension SyntaxProtocol {
+  /// Returns this node or the first ancestor that satisfies `condition`.
+  func ancestorOrSelf<T>(mapping map: (Syntax) -> T?) -> T? {
+    var walk: Syntax? = Syntax(self)
+    while let unwrappedParent = walk {
+      if let mapped = map(unwrappedParent) {
+        return mapped
+      }
+      walk = unwrappedParent.parent
+    }
+    return nil
+  }
+}

Tests/SwiftParserTest/Assertions.swift

@@ -685,7 +685,7 @@ extension ParserTestCase {
       XCTFail("A fixed source was provided but the test case produces no diagnostics with Fix-Its", file: file, line: line)
     }
 
-    if expectedDiagnostics.isEmpty {
+    if expectedDiagnostics.isEmpty && diags.isEmpty {
       assertBasicFormat(source: source, parse: parse, experimentalFeatures: experimentalFeatures, file: file, line: line)
     }
 

Tests/SwiftParserTest/StatementTests.swift

@@ -728,4 +728,171 @@ final class StatementTests: ParserTestCase {
       """
     )
   }
+
+  func testTrailingClosureInIfCondition() {
+    assertParse("if test { $0 } {}")
+
+    assertParse(
+      """
+      if test {
+        $0
+      }1️⃣ {}
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "consecutive statements on a line must be separated by newline or ';'", fixIts: ["insert newline", "insert ';'"])
+      ],
+      fixedSource: """
+        if test {
+          $0
+        }
+        {}
+        """
+    )
+
+    assertParse(
+      """
+      if test { $0
+      } {}
+      """
+    )
+
+    assertParse(
+      """
+      if test { x in
+        x
+      } {}
+      """
+    )
+  }
+
+  func testClosureAtStartOfIfCondition() {
+    assertParse(
+      "if 1️⃣{x}() {}",
+      diagnostics: [
+        DiagnosticSpec(message: "missing condition in 'if' statement")
+      ]
+    )
+
+    assertParse(
+      """
+      if 1️⃣{
+        x
+      }() {}
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "missing condition in 'if' statement")
+      ]
+    )
+
+    assertParse(
+      """
+      if 1️⃣{ x
+      }() {}
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "missing condition in 'if' statement")
+      ]
+    )
+
+    assertParse(
+      """
+      if 1️⃣{ a 2️⃣in
+        x + a
+      }(1) {}
+      """,
+      diagnostics: [
+        DiagnosticSpec(locationMarker: "1️⃣", message: "missing condition in 'if' statement"),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code in 'if' statement"),
+      ]
+    )
+  }
+
+  func testClosureInsideIfCondition() {
+    assertParse("if true, {x}() {}")
+
+    assertParse(
+      """
+      if true, {
+        x
+      }() {}
+      """
+    )
+
+    assertParse(
+      """
+      if true, { x
+      }() {}
+      """
+    )
+
+    assertParse(
+      """
+      if true, { a in
+        x + a
+      }(1) {}
+      """
+    )
+  }
+
+  func testTrailingClosureInGuard() {
+    assertParse(
+      "guard test 1️⃣{ $0 } 2️⃣else {}",
+      diagnostics: [
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'else' in 'guard' statement", fixIts: ["insert 'else'"]),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "extraneous code 'else {}' at top level"),
+      ],
+      fixedSource: "guard test else { $0 } else {}"
+    )
+
+    assertParse(
+      """
+      guard test 1️⃣{
+        $0
+      } 2️⃣else {}
+      """,
+      diagnostics: [
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'else' in 'guard' statement", fixIts: ["insert 'else'"]),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "extraneous code 'else {}' at top level"),
+      ],
+      fixedSource:
+        """
+        guard test else {
+          $0
+        } else {}
+        """
+    )
+
+    assertParse(
+      """
+      guard test 1️⃣{ $0
+      } 2️⃣else {}
+      """,
+      diagnostics: [
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'else' in 'guard' statement", fixIts: ["insert 'else'"]),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "extraneous code 'else {}' at top level"),
+      ],
+      fixedSource: """
+        guard test else { $0
+        } else {}
+        """
+    )
+
+    assertParse(
+      """
+      guard test 1️⃣{ x 2️⃣in
+        x
+      } 3️⃣else {}
+      """,
+      diagnostics: [
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'else' in 'guard' statement", fixIts: ["insert 'else'"]),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code in 'guard' statement"),
+        DiagnosticSpec(locationMarker: "3️⃣", message: "extraneous code 'else {}' at top level"),
+      ],
+      fixedSource: """
+        guard test else { x in
+          x
+        } else {}
+        """
+    )
+  }
 }