Skip to content

Commit

Permalink
Improve handling of trailing closures in forEach subject
Browse files Browse the repository at this point in the history
  • Loading branch information
calda committed Aug 5, 2023
1 parent 120376c commit 4d92ba2
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
12 changes: 11 additions & 1 deletion Sources/Rules.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8012,7 +8012,8 @@ public struct _FormatRules {
// 1. an identifier like `foo.`
// 2. a function call like `foo(...).`
// 3. a subscript like `foo[...].
// 4. Some other combination of parens / subscript like `(foo).`
// 4. a trailing closure like `map { ... }`
// 5. Some other combination of parens / subscript like `(foo).`
// or even `foo["bar"]()()`.
// And any of these can be preceeded by one of the others
switch formatter.tokens[index] {
Expand Down Expand Up @@ -8045,6 +8046,15 @@ public struct _FormatRules {
// - If the previous token is a newline then this isn't a function call
// and we'd stop parsing. `foo ()` is a function call but `foo\n()` isn't.
return startOfChainComponent(at: previousNonSpaceNonCommentIndex) ?? startOfScopeIndex

case .endOfScope("}"):
// Stop parsing if we reach a trailing closure.
// Converting this to a for loop would result in unusual looking syntax like
// `for string in strings.map { $0.uppercased() } { print(string) }`
// which causes a warning to be emitted: "trailing closure in this context is
// confusable with the body of the statement; pass as a parenthesized argument
// to silence this warning".
return nil

default:
return nil
Expand Down
14 changes: 13 additions & 1 deletion Tests/RulesTests+Syntax.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3705,7 +3705,7 @@ class SyntaxTests: RulesTests {
func testPreservesChainWithClosure() {
let input = """
// Converting this to a for loop would result in unusual looking syntax like
// `for string in strings.map { $0.uppercased() } { print($0) }`
// `for string in strings.map { $0.uppercased() } { print(string) }`
// which causes a warning to be emitted: "trailing closure in this context is
// confusable with the body of the statement; pass as a parenthesized argument
// to silence this warning".
Expand All @@ -3714,6 +3714,18 @@ class SyntaxTests: RulesTests {
testFormatting(for: input, rule: FormatRules.forLoop)
}

Check warning on line 3715 in Tests/RulesTests+Syntax.swift

View check run for this annotation

Codecov / codecov/patch

Tests/RulesTests+Syntax.swift#L3705-L3715

Added lines #L3705 - L3715 were not covered by tests

func testPreservesChainWithClosureInMiddleOfChain() {
let input = """
// Converting this to a for loop would result in unusual looking syntax like
// `for string in strings.map { $0.uppercased() }.values { print(string) }`
// which causes a warning to be emitted: "trailing closure in this context is
// confusable with the body of the statement; pass as a parenthesized argument
// to silence this warning".
strings.map { $0.uppercased() }.values.forEach { print($0) }
"""
testFormatting(for: input, rule: FormatRules.forLoop)
}

Check warning on line 3727 in Tests/RulesTests+Syntax.swift

View check run for this annotation

Codecov / codecov/patch

Tests/RulesTests+Syntax.swift#L3717-L3727

Added lines #L3717 - L3727 were not covered by tests

func testHandlesTryBeforeForEach() {
let input = """
try planets.forEach {
Expand Down

0 comments on commit 4d92ba2

Please sign in to comment.