Skip to content

Commit 3a5647b

Browse files
committed
Add test cases for parsing closures inside if statement conditions
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
1 parent 43b7a9f commit 3a5647b

File tree

3 files changed

+210
-9
lines changed

3 files changed

+210
-9
lines changed

Sources/SwiftBasicFormat/BasicFormat.swift

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -123,14 +123,7 @@ open class BasicFormat: SyntaxRewriter {
123123
// MARK: - Helper functions
124124

125125
private func isInsideStringInterpolation(_ token: TokenSyntax) -> Bool {
126-
var ancestor: Syntax = Syntax(token)
127-
while let parent = ancestor.parent {
128-
ancestor = parent
129-
if ancestor.is(ExpressionSegmentSyntax.self) {
130-
return true
131-
}
132-
}
133-
return false
126+
return token.ancestorOrSelf { $0.as(ExpressionSegmentSyntax.self) } != nil
134127
}
135128

136129
private func childrenSeparatedByNewline(_ node: Syntax) -> Bool {
@@ -220,6 +213,31 @@ open class BasicFormat: SyntaxRewriter {
220213
}
221214
}
222215

216+
/// Returns `true` if `token` is the opening brace of a closure that is being
217+
/// parsed in an expression with `ExprFlavor.stmtCondition`.
218+
///
219+
/// In these cases, adding a newline changes whether the closure gets parsed
220+
/// as a closure or if it gets interpreted as the statements body. We should
221+
/// thus be conservative and not add a newline after the `{` in `BasicFormat`.
222+
private func isLeftBraceOfClosureInStmtConditionExpr(_ token: TokenSyntax?) -> Bool {
223+
guard let token, token.keyPathInParent == \ClosureExprSyntax.leftBrace else {
224+
return false
225+
}
226+
return token.ancestorOrSelf(mapping: {
227+
switch $0.keyPathInParent {
228+
case \CatchItemSyntax.pattern,
229+
\ConditionElementSyntax.condition,
230+
\ExpressionPatternSyntax.expression,
231+
\ForStmtSyntax.sequence,
232+
\ForStmtSyntax.whereClause,
233+
\SwitchExprSyntax.subject:
234+
return $0
235+
default:
236+
return nil
237+
}
238+
}) != nil
239+
}
240+
223241
open func requiresNewline(between first: TokenSyntax?, and second: TokenSyntax?) -> Bool {
224242
// We don't want to add newlines inside string interpolation.
225243
// When first or second ``TokenSyntax`` is a multiline quote we want special handling
@@ -231,6 +249,8 @@ open class BasicFormat: SyntaxRewriter {
231249
second?.tokenKind != .multilineStringQuote
232250
{
233251
return false
252+
} else if isLeftBraceOfClosureInStmtConditionExpr(first) {
253+
return false
234254
} else if let second {
235255
var ancestor: Syntax = Syntax(second)
236256
while let parent = ancestor.parent {
@@ -607,3 +627,17 @@ fileprivate extension TokenSyntax {
607627
}
608628
}
609629
}
630+
631+
fileprivate extension SyntaxProtocol {
632+
/// Returns this node or the first ancestor that satisfies `condition`.
633+
func ancestorOrSelf<T>(mapping map: (Syntax) -> T?) -> T? {
634+
var walk: Syntax? = Syntax(self)
635+
while let unwrappedParent = walk {
636+
if let mapped = map(unwrappedParent) {
637+
return mapped
638+
}
639+
walk = unwrappedParent.parent
640+
}
641+
return nil
642+
}
643+
}

Tests/SwiftParserTest/Assertions.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,7 @@ extension ParserTestCase {
685685
XCTFail("A fixed source was provided but the test case produces no diagnostics with Fix-Its", file: file, line: line)
686686
}
687687

688-
if expectedDiagnostics.isEmpty {
688+
if expectedDiagnostics.isEmpty && diags.isEmpty {
689689
assertBasicFormat(source: source, parse: parse, experimentalFeatures: experimentalFeatures, file: file, line: line)
690690
}
691691

Tests/SwiftParserTest/StatementTests.swift

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,4 +728,171 @@ final class StatementTests: ParserTestCase {
728728
"""
729729
)
730730
}
731+
732+
func testTrailingClosureInIfCondition() {
733+
assertParse("if test { $0 } {}")
734+
735+
assertParse(
736+
"""
737+
if test {
738+
$0
739+
}1️⃣ {}
740+
""",
741+
diagnostics: [
742+
DiagnosticSpec(message: "consecutive statements on a line must be separated by newline or ';'", fixIts: ["insert newline", "insert ';'"])
743+
],
744+
fixedSource: """
745+
if test {
746+
$0
747+
}
748+
{}
749+
"""
750+
)
751+
752+
assertParse(
753+
"""
754+
if test { $0
755+
} {}
756+
"""
757+
)
758+
759+
assertParse(
760+
"""
761+
if test { x in
762+
x
763+
} {}
764+
"""
765+
)
766+
}
767+
768+
func testClosureAtStartOfIfCondition() {
769+
assertParse(
770+
"if 1️⃣{x}() {}",
771+
diagnostics: [
772+
DiagnosticSpec(message: "missing condition in 'if' statement")
773+
]
774+
)
775+
776+
assertParse(
777+
"""
778+
if 1️⃣{
779+
x
780+
}() {}
781+
""",
782+
diagnostics: [
783+
DiagnosticSpec(message: "missing condition in 'if' statement")
784+
]
785+
)
786+
787+
assertParse(
788+
"""
789+
if 1️⃣{ x
790+
}() {}
791+
""",
792+
diagnostics: [
793+
DiagnosticSpec(message: "missing condition in 'if' statement")
794+
]
795+
)
796+
797+
assertParse(
798+
"""
799+
if 1️⃣{ a 2️⃣in
800+
x + a
801+
}(1) {}
802+
""",
803+
diagnostics: [
804+
DiagnosticSpec(locationMarker: "1️⃣", message: "missing condition in 'if' statement"),
805+
DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code in 'if' statement"),
806+
]
807+
)
808+
}
809+
810+
func testClosureInsideIfCondition() {
811+
assertParse("if true, {x}() {}")
812+
813+
assertParse(
814+
"""
815+
if true, {
816+
x
817+
}() {}
818+
"""
819+
)
820+
821+
assertParse(
822+
"""
823+
if true, { x
824+
}() {}
825+
"""
826+
)
827+
828+
assertParse(
829+
"""
830+
if true, { a in
831+
x + a
832+
}(1) {}
833+
"""
834+
)
835+
}
836+
837+
func testTrailingClosureInGuard() {
838+
assertParse(
839+
"guard test 1️⃣{ $0 } 2️⃣else {}",
840+
diagnostics: [
841+
DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'else' in 'guard' statement", fixIts: ["insert 'else'"]),
842+
DiagnosticSpec(locationMarker: "2️⃣", message: "extraneous code 'else {}' at top level"),
843+
],
844+
fixedSource: "guard test else { $0 } else {}"
845+
)
846+
847+
assertParse(
848+
"""
849+
guard test 1️⃣{
850+
$0
851+
} 2️⃣else {}
852+
""",
853+
diagnostics: [
854+
DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'else' in 'guard' statement", fixIts: ["insert 'else'"]),
855+
DiagnosticSpec(locationMarker: "2️⃣", message: "extraneous code 'else {}' at top level"),
856+
],
857+
fixedSource:
858+
"""
859+
guard test else {
860+
$0
861+
} else {}
862+
"""
863+
)
864+
865+
assertParse(
866+
"""
867+
guard test 1️⃣{ $0
868+
} 2️⃣else {}
869+
""",
870+
diagnostics: [
871+
DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'else' in 'guard' statement", fixIts: ["insert 'else'"]),
872+
DiagnosticSpec(locationMarker: "2️⃣", message: "extraneous code 'else {}' at top level"),
873+
],
874+
fixedSource: """
875+
guard test else { $0
876+
} else {}
877+
"""
878+
)
879+
880+
assertParse(
881+
"""
882+
guard test 1️⃣{ x 2️⃣in
883+
x
884+
} 3️⃣else {}
885+
""",
886+
diagnostics: [
887+
DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'else' in 'guard' statement", fixIts: ["insert 'else'"]),
888+
DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code in 'guard' statement"),
889+
DiagnosticSpec(locationMarker: "3️⃣", message: "extraneous code 'else {}' at top level"),
890+
],
891+
fixedSource: """
892+
guard test else { x in
893+
x
894+
} else {}
895+
"""
896+
)
897+
}
731898
}

0 commit comments

Comments
 (0)