Skip to content

Commit c52e0d2

Browse files
dylansturgallevato
authored andcommitted
Add a consistent group around if-stmts.
The consistent group forces the bodies of if-stmts with else clauses to have breaks around all braces when the stmt spans multiple lines. Previously. the breaks around the braces only fired if the body didn't fit on the line. That behavior lead to hard to read if-stmts because if was unclear where conditions stopped and the body started.
1 parent 8fbc3f1 commit c52e0d2

File tree

3 files changed

+64
-1
lines changed

3 files changed

+64
-1
lines changed

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

+13
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,12 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
444444
// MARK: - Control flow statement nodes
445445

446446
override func visit(_ node: IfStmtSyntax) -> SyntaxVisitorContinueKind {
447+
// There may be a consistent breaking group around this node, see `CodeBlockItemSyntax`. This
448+
// group is necessary so that breaks around and inside of the conditions aren't forced to break
449+
// when the if-stmt spans multiple lines.
450+
before(node.conditions.firstToken, tokens: .open)
451+
after(node.conditions.lastToken, tokens: .close)
452+
447453
after(node.labelColon, tokens: .space)
448454
after(node.ifKeyword, tokens: .space)
449455

@@ -1287,6 +1293,13 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
12871293
appendFormatterIgnored(node: Syntax(node))
12881294
return .skipChildren
12891295
}
1296+
1297+
// This group applies to a top-level if-stmt so that all of the bodies will have the same
1298+
// breaking behavior.
1299+
if let ifStmt = node.item.as(IfStmtSyntax.self) {
1300+
before(ifStmt.firstToken, tokens: .open(.consistent))
1301+
after(ifStmt.lastToken, tokens: .close)
1302+
}
12901303
return .visitChildren
12911304
}
12921305

Tests/SwiftFormatPrettyPrintTests/IfStmtTests.swift

+50-1
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,9 @@ final class IfStmtTests: PrettyPrintTestCase {
381381
SomeVeryLongTypeNameThatDefinitelyBreaks,
382382
baz: Baz
383383
) = foo(a, b, c, d)
384-
{ return nil }
384+
{
385+
return nil
386+
}
385387
386388
"""
387389

@@ -515,4 +517,51 @@ final class IfStmtTests: PrettyPrintTestCase {
515517

516518
assertPrettyPrintEqual(input: input, expected: expected, linelength: 50)
517519
}
520+
521+
func testMultipleIfStmts() {
522+
let input =
523+
"""
524+
if foo && bar { baz() } else if bar { baz() } else if foo { baz() } else { blargh() }
525+
if foo && bar && quxxe { baz() } else if bar { baz() } else if foo { baz() } else if quxxe { baz() } else { blargh() }
526+
if let foo = getmyfoo(), let bar = getmybar(), foo.baz && bar.baz { foo() } else { bar() }
527+
if let foo = getmyfoo(), let bar = getmybar(), foo.baz && bar.baz && someOtherCondition { foo() } else { bar() }
528+
if let foo = getmyfoo(), let bar = getmybar(), foo.baz && bar.baz && someOtherCondition { foo() }
529+
"""
530+
531+
let expected =
532+
"""
533+
if foo && bar { baz() } else if bar { baz() } else if foo { baz() } else { blargh() }
534+
if foo && bar && quxxe {
535+
baz()
536+
} else if bar {
537+
baz()
538+
} else if foo {
539+
baz()
540+
} else if quxxe {
541+
baz()
542+
} else {
543+
blargh()
544+
}
545+
if let foo = getmyfoo(), let bar = getmybar(), foo.baz && bar.baz {
546+
foo()
547+
} else {
548+
bar()
549+
}
550+
if let foo = getmyfoo(), let bar = getmybar(),
551+
foo.baz && bar.baz && someOtherCondition
552+
{
553+
foo()
554+
} else {
555+
bar()
556+
}
557+
if let foo = getmyfoo(), let bar = getmybar(),
558+
foo.baz && bar.baz && someOtherCondition
559+
{
560+
foo()
561+
}
562+
563+
"""
564+
565+
assertPrettyPrintEqual(input: input, expected: expected, linelength: 85)
566+
}
518567
}

Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift

+1
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,7 @@ extension IfStmtTests {
410410
("testIfStatement", testIfStatement),
411411
("testLabeledIfStmt", testLabeledIfStmt),
412412
("testMatchingPatternConditions", testMatchingPatternConditions),
413+
("testMultipleIfStmts", testMultipleIfStmts),
413414
("testOptionalBindingConditions", testOptionalBindingConditions),
414415
("testParenthesizedClauses", testParenthesizedClauses),
415416
]

0 commit comments

Comments
 (0)