Skip to content

Commit 5df9eb4

Browse files
dylansturgallevato
authored andcommitted
Move open group token past ifstmt's if token to avoid extra newlines.
The implementation of consistent groups forces breaking inside of the group if the group starts at the beginning of a line (i.e. was immediately preceded by a break that fired). That isn't exactly what we want for if-stmt because the stmt generally starts at the beginning of a line and we want the breaks to fire only if the group is longer than the rest of the line. Moving the open token past the if `syntax` token resolves 2 known complexities with consistent groups: 1. Whether an immediately preceding break fired influences 2. The `spaceRemaining` value is only updated after printing the first `syntax` token on a new line It's a little odd to group in this way, since it logically makes more sense to group around the entire if-stmt but there aren't any breaks between the if token and the first condition so moving the open token doesn't change the length of any breaks throughout the statement.
1 parent 0607843 commit 5df9eb4

File tree

2 files changed

+39
-35
lines changed

2 files changed

+39
-35
lines changed

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1310,7 +1310,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
13101310
// This group applies to a top-level if-stmt so that all of the bodies will have the same
13111311
// breaking behavior.
13121312
if let ifStmt = node.item.as(IfStmtSyntax.self) {
1313-
before(ifStmt.firstToken, tokens: .open(.consistent))
1313+
before(ifStmt.conditions.firstToken, tokens: .open(.consistent))
13141314
after(ifStmt.lastToken, tokens: .close)
13151315
}
13161316
return .visitChildren

Tests/SwiftFormatPrettyPrintTests/IfStmtTests.swift

Lines changed: 38 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -521,47 +521,51 @@ final class IfStmtTests: PrettyPrintTestCase {
521521
func testMultipleIfStmts() {
522522
let input =
523523
"""
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() }
524+
func foo() {
525+
if foo && bar { baz() } else if bar { baz() } else if foo { baz() } else { blargh() }
526+
if foo && bar && quxxe { baz() } else if bar { baz() } else if foo { baz() } else if quxxe { baz() } else { blargh() }
527+
if let foo = getmyfoo(), let bar = getmybar(), foo.baz && bar.baz { foo() } else { bar() }
528+
if let foo = getmyfoo(), let bar = getmybar(), foo.baz && bar.baz && someOtherCondition { foo() } else { bar() }
529+
if let foo = getmyfoo(), let bar = getmybar(), foo.baz && bar.baz && someOtherCondition { foo() }
530+
}
529531
"""
530532

531533
let expected =
532534
"""
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()
535+
func foo() {
536+
if foo && bar { baz() } else if bar { baz() } else if foo { baz() } else { blargh() }
537+
if foo && bar && quxxe {
538+
baz()
539+
} else if bar {
540+
baz()
541+
} else if foo {
542+
baz()
543+
} else if quxxe {
544+
baz()
545+
} else {
546+
blargh()
547+
}
548+
if let foo = getmyfoo(), let bar = getmybar(), foo.baz && bar.baz {
549+
foo()
550+
} else {
551+
bar()
552+
}
553+
if let foo = getmyfoo(), let bar = getmybar(),
554+
foo.baz && bar.baz && someOtherCondition
555+
{
556+
foo()
557+
} else {
558+
bar()
559+
}
560+
if let foo = getmyfoo(), let bar = getmybar(),
561+
foo.baz && bar.baz && someOtherCondition
562+
{
563+
foo()
564+
}
561565
}
562566
563567
"""
564568

565-
assertPrettyPrintEqual(input: input, expected: expected, linelength: 85)
569+
assertPrettyPrintEqual(input: input, expected: expected, linelength: 87)
566570
}
567571
}

0 commit comments

Comments
 (0)