Repository: apple/swift-syntax (swift)

Issue:
SwiftParser allows closures in condition lists where the compiler doesn't
### Issue Kind

Bad Diagnostic Produced

### Source Code

```swift
if {x}() {}
if x.map{$0} {}
```


### Description

While I would _love_ to have these examples permitted, Swift forbids them but they currently succeed in the new Swift parser without diagnostics and produce the following valid parse tree:

```
SourceFileSyntax children=2
  0: CodeBlockItemListSyntax children=2        // if {x}() {}
    0: CodeBlockItemSyntax children=1
      0: IfStmtSyntax children=3
        0: ifKeyword
        1: ConditionElementListSyntax children=1
          0: ConditionElementSyntax children=1
            0: FunctionCallExprSyntax children=4
              0: ClosureExprSyntax children=3
                0: leftBrace
                1: CodeBlockItemListSyntax children=1
                  0: CodeBlockItemSyntax children=1
                    0: IdentifierExprSyntax children=1
                      0: identifier("x")
                2: rightBrace
              1: leftParen
              2: TupleExprElementListSyntax
              3: rightParen
        2: CodeBlockSyntax children=3
          0: leftBrace
          1: CodeBlockItemListSyntax
          2: rightBrace
    1: CodeBlockItemSyntax children=1        // if x.map{$0} {}
      0: IfStmtSyntax children=3
        0: ifKeyword
        1: ConditionElementListSyntax children=1
          0: ConditionElementSyntax children=1
            0: FunctionCallExprSyntax children=3
              0: MemberAccessExprSyntax children=3
                0: IdentifierExprSyntax children=1
                  0: identifier("x")
                1: period
                2: identifier("map")
              1: TupleExprElementListSyntax
              2: ClosureExprSyntax children=3
                0: leftBrace
                1: CodeBlockItemListSyntax children=1
                  0: CodeBlockItemSyntax children=1
                    0: IdentifierExprSyntax children=1
                      0: dollarIdentifier("$0")
                2: rightBrace
        2: CodeBlockSyntax children=3
          0: leftBrace
          1: CodeBlockItemListSyntax
          2: rightBrace
  1: eof
```

Pull request:
Add a bunch of test cases that verify that SwiftParser accepts closures inside `if` conditions if and only if the C++ parser does.

The only difference is that the C++ parser emits a warning in cases like `if test { $0 } {}`: `Trailing closure in this context is confusable with the body of the statement; pass as a parenthesized argument to silence this warning`. We currently don’t have any infrastructure to emit warnings from SwiftParser and I’m not sure if this warning is really necessary. NFC on whether we need it.

While writing the test cases, I found an issue in `BasicFormat` that adds a newline after the opening `{` of the closure inside these conditions, which causes the closure to get parsed as the statement’s body. Fixed that as well.

Fixes #795
rdar://99948919

Patch:
diff --git a/Sources/SwiftBasicFormat/BasicFormat.swift b/Sources/SwiftBasicFormat/BasicFormat.swift
index 1d7f12fd5c2..a433d133b52 100644
--- a/Sources/SwiftBasicFormat/BasicFormat.swift
+++ b/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
+  }
+}


Test patch:
diff --git a/Tests/SwiftParserTest/Assertions.swift b/Tests/SwiftParserTest/Assertions.swift
index c2d04e90e88..411d05b1dd2 100644
--- a/Tests/SwiftParserTest/Assertions.swift
+++ b/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)
     }
 
diff --git a/Tests/SwiftParserTest/StatementTests.swift b/Tests/SwiftParserTest/StatementTests.swift
index 6b8f16cd5b0..a01f70ed7be 100644
--- a/Tests/SwiftParserTest/StatementTests.swift
+++ b/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 {}
+        """
+    )
+  }
 }