Skip to content

Absorb trailing comma of function call args in grouped exprs. #151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,9 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
before(node.firstToken, tokens: .open)
after(node.colon, tokens: .break(.continue, newlines: .elective(ignoresDiscretionary: true)))
after(node.lastToken, tokens: .close)
if let trailingComma = node.trailingComma {
closingDelimiterTokens.insert(trailingComma)
}
}

override func visit(_ node: ArrayExprSyntax) -> SyntaxVisitorContinueKind {
Expand Down Expand Up @@ -729,6 +732,9 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
override func visit(_ node: ArrayElementSyntax) -> SyntaxVisitorContinueKind {
before(node.firstToken, tokens: .open)
after(node.lastToken, tokens: .close)
if let trailingComma = node.trailingComma {
closingDelimiterTokens.insert(trailingComma)
}
return .visitChildren
}

Expand Down Expand Up @@ -766,6 +772,9 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
before(node.firstToken, tokens: .open)
after(node.colon, tokens: .break(.continue, newlines: .elective(ignoresDiscretionary: true)))
after(node.lastToken, tokens: .close)
if let trailingComma = node.trailingComma {
closingDelimiterTokens.insert(trailingComma)
}
return .visitChildren
}

Expand Down Expand Up @@ -837,6 +846,9 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {

let shouldGroupAroundArgument = !isCompactSingleFunctionCallArgument(arguments)
for argument in arguments {
if let trailingComma = argument.trailingComma {
closingDelimiterTokens.insert(trailingComma)
}
arrangeAsFunctionCallArgument(argument, shouldGroup: shouldGroupAroundArgument)
}
}
Expand Down Expand Up @@ -1626,6 +1638,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
// If this is one of multiple comma-delimited bindings, move any pending close breaks to
// follow the comma so that it doesn't get separated from the tokens before it.
closeAfterToken = trailingComma
closingDelimiterTokens.insert(trailingComma)
}

if closeAfterToken != nil && closesNeeded > 0 {
Expand Down
23 changes: 23 additions & 0 deletions Tests/SwiftFormatPrettyPrintTests/ArrayDeclTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,27 @@ final class ArrayDeclTests: PrettyPrintTestCase {
XCTAssertDiagnosed(.removeTrailingComma, line: 1, column: 17)
XCTAssertDiagnosed(.addTrailingComma, line: 4, column: 26)
}

func testGroupsTrailingComma() {
let input =
"""
let a = [
condition ? firstOption : secondOption,
bar(),
]
"""

let expected =
"""
let a = [
condition
? firstOption
: secondOption,
bar(),
]

"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 32)
}
}
23 changes: 23 additions & 0 deletions Tests/SwiftFormatPrettyPrintTests/DictionaryDeclTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,27 @@ final class DictionaryDeclTests: PrettyPrintTestCase {

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

func testGroupsTrailingComma() {
let input =
"""
let d = [
key: cond ? firstOption : secondOption,
key2: bar(),
]
"""

let expected =
"""
let d = [
key: cond
? firstOption
: secondOption,
key2: bar(),
]

"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 32)
}
}
21 changes: 21 additions & 0 deletions Tests/SwiftFormatPrettyPrintTests/FunctionCallTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -249,4 +249,25 @@ final class FunctionCallTests: PrettyPrintTestCase {

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

func testGroupsTrailingComma() {
let input =
"""
foo(
image: useLongName ? image(named: .longNameImage) : image(named: .veryLongNameImageZ),
bar: bar)
"""

let expected =
"""
foo(
image: useLongName
? image(named: .longNameImage)
: image(named: .veryLongNameImageZ),
bar: bar)

"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 70)
}
}
21 changes: 21 additions & 0 deletions Tests/SwiftFormatPrettyPrintTests/ObjectLiteralExprTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,25 @@ final class ObjectLiteralExprTests: PrettyPrintTestCase {
config.lineBreakBeforeEachArgument = false
assertPrettyPrintEqual(input: input, expected: expected, linelength: 38, configuration: config)
}

func testGroupsTrailingComma() {
let input =
"""
#imageLiteral(
image: useLongName ? image(named: .longNameImage) : image(named: .veryLongNameImageZ),
bar: bar)
"""

let expected =
"""
#imageLiteral(
image: useLongName
? image(named: .longNameImage)
: image(named: .veryLongNameImageZ),
bar: bar)

"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 70)
}
}
23 changes: 23 additions & 0 deletions Tests/SwiftFormatPrettyPrintTests/PatternBindingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,27 @@ final class PatternBindingTests: PrettyPrintTestCase {

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

func testGroupingIncludesTrailingComma() {
let input =
"""
let foo = veryLongCondition
? firstOption
: secondOption,
bar = bar()
"""

let expected =
"""
let
foo =
veryLongCondition
? firstOption
: secondOption,
bar = bar()

"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 18)
}
}
21 changes: 21 additions & 0 deletions Tests/SwiftFormatPrettyPrintTests/SubscriptExprTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,25 @@ final class SubscriptExprTests: PrettyPrintTestCase {

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

func testGroupsTrailingComma() {
let input =
"""
myCollection[
image: useLongName ? image(named: .longNameImage) : image(named: .veryLongNameImageZ),
bar: bar]
"""

let expected =
"""
myCollection[
image: useLongName
? image(named: .longNameImage)
: image(named: .veryLongNameImageZ),
bar: bar]

"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 70)
}
}
23 changes: 23 additions & 0 deletions Tests/SwiftFormatPrettyPrintTests/TupleDeclTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,27 @@ final class TupleDeclTests: PrettyPrintTestCase {

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

func testGroupsTrailingComma() {
let input =
"""
let t = (
condition ? firstOption : secondOption,
bar()
)
"""

let expected =
"""
let t = (
condition
? firstOption
: secondOption,
bar()
)
"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 32)
}
}
7 changes: 7 additions & 0 deletions Tests/SwiftFormatPrettyPrintTests/XCTestManifests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ extension ArrayDeclTests {
static let __allTests__ArrayDeclTests = [
("testArrayOfFunctions", testArrayOfFunctions),
("testBasicArrays", testBasicArrays),
("testGroupsTrailingComma", testGroupsTrailingComma),
("testNoTrailingCommasInTypes", testNoTrailingCommasInTypes),
("testTrailingCommaDiagnostics", testTrailingCommaDiagnostics),
("testWhitespaceOnlyDoesNotChangeTrailingComma", testWhitespaceOnlyDoesNotChangeTrailingComma),
Expand Down Expand Up @@ -181,6 +182,7 @@ extension DictionaryDeclTests {
// to regenerate.
static let __allTests__DictionaryDeclTests = [
("testBasicDictionaries", testBasicDictionaries),
("testGroupsTrailingComma", testGroupsTrailingComma),
("testIgnoresDiscretionaryNewlineAfterColon", testIgnoresDiscretionaryNewlineAfterColon),
("testNoTrailingCommasInTypes", testNoTrailingCommasInTypes),
("testTrailingCommaDiagnostics", testTrailingCommaDiagnostics),
Expand Down Expand Up @@ -270,6 +272,7 @@ extension FunctionCallTests {
("testBasicFunctionCalls_packArguments", testBasicFunctionCalls_packArguments),
("testDiscretionaryLineBreakBeforeClosingParenthesis", testDiscretionaryLineBreakBeforeClosingParenthesis),
("testDiscretionaryLineBreaksAreSelfCorrecting", testDiscretionaryLineBreaksAreSelfCorrecting),
("testGroupsTrailingComma", testGroupsTrailingComma),
("testIgnoresDiscretionaryLineBreakAfterColon", testIgnoresDiscretionaryLineBreakAfterColon),
("testNestedFunctionCallExprSequences", testNestedFunctionCallExprSequences),
("testSingleUnlabeledArgumentWithDelimiters", testSingleUnlabeledArgumentWithDelimiters),
Expand Down Expand Up @@ -473,6 +476,7 @@ extension ObjectLiteralExprTests {
static let __allTests__ObjectLiteralExprTests = [
("testColorLiteral_noPackArguments", testColorLiteral_noPackArguments),
("testColorLiteral_packArguments", testColorLiteral_packArguments),
("testGroupsTrailingComma", testGroupsTrailingComma),
("testImageLiteral_noPackArguments", testImageLiteral_noPackArguments),
("testImageLiteral_packArguments", testImageLiteral_packArguments),
]
Expand Down Expand Up @@ -508,6 +512,7 @@ extension PatternBindingTests {
// to regenerate.
static let __allTests__PatternBindingTests = [
("testBindingIncludingTypeAnnotation", testBindingIncludingTypeAnnotation),
("testGroupingIncludesTrailingComma", testGroupingIncludesTrailingComma),
("testIgnoresDiscretionaryNewlineAfterColon", testIgnoresDiscretionaryNewlineAfterColon),
]
}
Expand Down Expand Up @@ -660,6 +665,7 @@ extension SubscriptExprTests {
static let __allTests__SubscriptExprTests = [
("testBasicSubscriptGetters", testBasicSubscriptGetters),
("testBasicSubscriptSetters", testBasicSubscriptSetters),
("testGroupsTrailingComma", testGroupsTrailingComma),
("testSubscriptGettersWithTrailingClosures", testSubscriptGettersWithTrailingClosures),
("testSubscriptSettersWithTrailingClosures", testSubscriptSettersWithTrailingClosures),
]
Expand Down Expand Up @@ -715,6 +721,7 @@ extension TupleDeclTests {
// to regenerate.
static let __allTests__TupleDeclTests = [
("testBasicTuples", testBasicTuples),
("testGroupsTrailingComma", testGroupsTrailingComma),
("testIgnoresDiscretionaryNewlineAfterColon", testIgnoresDiscretionaryNewlineAfterColon),
("testLabeledTuples", testLabeledTuples),
]
Expand Down