Skip to content

Commit

Permalink
Disallow discretionary newlines before trailing closures.
Browse files Browse the repository at this point in the history
The behavior was inconsistent between subscripts and function calls. They both use a same-break that ignores discretionary newlines now.

The break ignores discretionary because breaking before a trailing closure's left brace uses excessive vertical whitespace without improving readability. Now the brace is usually glued to the right paren/right square bracket, except when it doesn't fit due to line length.
  • Loading branch information
dylansturg authored and allevato committed Apr 30, 2020
1 parent 66cac37 commit a4100c3
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 2 deletions.
8 changes: 6 additions & 2 deletions Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,9 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
(node.trailingClosure != nil && !isCompactSingleFunctionCallArgument(arguments))
|| mustBreakBeforeClosingDelimiter(of: node, argumentListPath: \.argumentList)

before(node.trailingClosure?.leftBrace, tokens: .break(.same))
before(
node.trailingClosure?.leftBrace,
tokens: .break(.same, newlines: .elective(ignoresDiscretionary: true)))

arrangeFunctionCallArgumentList(
arguments,
Expand Down Expand Up @@ -1048,7 +1050,9 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
node.trailingClosure != nil
|| mustBreakBeforeClosingDelimiter(of: node, argumentListPath: \.argumentList)

before(node.trailingClosure?.leftBrace, tokens: .space)
before(
node.trailingClosure?.leftBrace,
tokens: .break(.same, newlines: .elective(ignoresDiscretionary: true)))

arrangeFunctionCallArgumentList(
arguments,
Expand Down
49 changes: 49 additions & 0 deletions Tests/SwiftFormatPrettyPrintTests/FunctionCallTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,55 @@ final class FunctionCallTests: PrettyPrintTestCase {
assertPrettyPrintEqual(input: input, expected: expected, linelength: 20)
}

func testDiscretionaryLineBreakBeforeTrailingClosure() {
let input =
"""
foo(a, b, c)
{
blah()
}
foo(
a, b, c
)
{
blah()
}
foo(arg1, arg2, arg3, arg4, arg5, arg6, arg7)
{
blah()
}
foo(ab, arg1, arg2) {
blah()
}
"""

let expected =
"""
foo(a, b, c) {
blah()
}
foo(
a, b, c
) {
blah()
}
foo(
arg1, arg2, arg3,
arg4, arg5, arg6,
arg7
) {
blah()
}
foo(ab, arg1, arg2)
{
blah()
}
"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 20)
}

func testGroupsTrailingComma() {
let input =
"""
Expand Down
49 changes: 49 additions & 0 deletions Tests/SwiftFormatPrettyPrintTests/SubscriptExprTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,53 @@ final class SubscriptExprTests: PrettyPrintTestCase {

assertPrettyPrintEqual(input: input, expected: expected, linelength: 70)
}

func testDiscretionaryLineBreakBeforeTrailingClosure() {
let input =
"""
foo[a, b, c]
{
blah()
}
foo[
a, b, c
]
{
blah()
}
foo[arg1, arg2, arg3, arg4, arg5, arg6, arg7]
{
blah()
}
foo[ab, arg1, arg2] {
blah()
}
"""

let expected =
"""
foo[a, b, c] {
blah()
}
foo[
a, b, c
] {
blah()
}
foo[
arg1, arg2, arg3,
arg4, arg5, arg6,
arg7
] {
blah()
}
foo[ab, arg1, arg2]
{
blah()
}
"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 20)
}
}
2 changes: 2 additions & 0 deletions Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ extension FunctionCallTests {
("testBasicFunctionCalls_packArguments", testBasicFunctionCalls_packArguments),
("testDiscretionaryLineBreakAfterColon", testDiscretionaryLineBreakAfterColon),
("testDiscretionaryLineBreakBeforeClosingParenthesis", testDiscretionaryLineBreakBeforeClosingParenthesis),
("testDiscretionaryLineBreakBeforeTrailingClosure", testDiscretionaryLineBreakBeforeTrailingClosure),
("testDiscretionaryLineBreaksAreSelfCorrecting", testDiscretionaryLineBreaksAreSelfCorrecting),
("testGroupsTrailingComma", testGroupsTrailingComma),
("testNestedFunctionCallExprSequences", testNestedFunctionCallExprSequences),
Expand Down Expand Up @@ -697,6 +698,7 @@ extension SubscriptExprTests {
static let __allTests__SubscriptExprTests = [
("testBasicSubscriptGetters", testBasicSubscriptGetters),
("testBasicSubscriptSetters", testBasicSubscriptSetters),
("testDiscretionaryLineBreakBeforeTrailingClosure", testDiscretionaryLineBreakBeforeTrailingClosure),
("testGroupsTrailingComma", testGroupsTrailingComma),
("testSubscriptGettersWithTrailingClosures", testSubscriptGettersWithTrailingClosures),
("testSubscriptSettersWithTrailingClosures", testSubscriptSettersWithTrailingClosures),
Expand Down

0 comments on commit a4100c3

Please sign in to comment.