Skip to content

Commit 303d82d

Browse files
authored
Update trailingClosures rule to support multiple trailing closures (#2190)
1 parent 38f9a0d commit 303d82d

File tree

4 files changed

+259
-26
lines changed

4 files changed

+259
-26
lines changed

Rules.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3403,6 +3403,19 @@ Option | Description
34033403
+ let foo = bar.map { ... }.joined()
34043404
```
34053405

3406+
```diff
3407+
- withAnimation(.spring, {
3408+
- isVisible = true
3409+
- }, completion: {
3410+
- handleCompletion()
3411+
- })
3412+
+ withAnimation(.spring) {
3413+
+ isVisible = true
3414+
+ } completion: {
3415+
+ handleCompletion()
3416+
+ }
3417+
```
3418+
34063419
</details>
34073420
<br/>
34083421

Sources/ParsingHelpers.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2962,6 +2962,12 @@ extension Formatter {
29622962
}
29632963
}
29642964

2965+
// Ensure we have a valid range
2966+
guard valueStart <= valueEnd else {
2967+
currentIndex = endOfCurrentArgument
2968+
continue
2969+
}
2970+
29652971
let valueRange = valueStart ... valueEnd
29662972
argumentLabels.append(FunctionCallArgument(
29672973
label: tokens[argumentLabelIndex].string,
@@ -2983,6 +2989,12 @@ extension Formatter {
29832989
}
29842990
}
29852991

2992+
// Ensure we have a valid range
2993+
guard valueStart <= valueEnd else {
2994+
currentIndex = endOfCurrentArgument
2995+
continue
2996+
}
2997+
29862998
let valueRange = valueStart ... valueEnd
29872999
argumentLabels.append(FunctionCallArgument(
29883000
label: nil,

Sources/Rules/TrailingClosures.swift

Lines changed: 144 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,39 +30,144 @@ public extension FormatRule {
3030
guard !nonTrailing.contains(name), !formatter.isConditionalStatement(at: i) else {
3131
return
3232
}
33-
guard let closingIndex = formatter.index(of: .endOfScope(")"), after: i), let closingBraceIndex =
34-
formatter.index(of: .nonSpaceOrComment, before: closingIndex, if: { $0 == .endOfScope("}") }),
35-
let openingBraceIndex = formatter.index(of: .startOfScope("{"), before: closingBraceIndex),
36-
formatter.index(of: .endOfScope("}"), before: openingBraceIndex) == nil
37-
else {
38-
return
33+
34+
// Parse all arguments to detect multiple trailing closures
35+
let arguments = formatter.parseFunctionCallArguments(startOfScope: i)
36+
let closures = arguments.filter { arg in
37+
let range = arg.valueRange
38+
guard let first = formatter.index(of: .nonSpaceOrCommentOrLinebreak, in: range.lowerBound ..< range.upperBound + 1),
39+
let last = formatter.index(of: .nonSpaceOrCommentOrLinebreak, before: range.upperBound + 1, if: { _ in true }),
40+
formatter.tokens[first] == .startOfScope("{"),
41+
formatter.tokens[last] == .endOfScope("}"),
42+
formatter.index(of: .endOfScope("}"), after: first) == last
43+
else {
44+
return false
45+
}
46+
return true
3947
}
40-
guard formatter.next(.nonSpaceOrCommentOrLinebreak, after: closingIndex) != .startOfScope("{"),
41-
var startIndex = formatter.index(of: .nonSpaceOrLinebreak, before: openingBraceIndex)
42-
else {
43-
return
48+
49+
// Determine if we should apply trailing closure transformation
50+
let shouldTransform: Bool
51+
if closures.count > 1 {
52+
// Multiple closures: first must be unlabeled, subsequent must be labeled
53+
shouldTransform = closures[0].label == nil && closures.dropFirst().allSatisfy { $0.label != nil }
54+
} else if closures.count == 1 {
55+
// Single closure: check if it should be made trailing
56+
let closure = closures[0]
57+
if closure.label == nil {
58+
// Unlabeled single closure
59+
shouldTransform = true
60+
} else {
61+
// Labeled single closure: only if function is in useTrailing list
62+
shouldTransform = useTrailing.contains(name)
63+
}
64+
} else {
65+
shouldTransform = false
4466
}
45-
switch formatter.tokens[startIndex] {
46-
case .delimiter(","), .startOfScope("("):
47-
break
48-
case .delimiter(":"):
49-
guard useTrailing.contains(name) else {
67+
68+
guard shouldTransform else { return }
69+
guard let closingIndex = formatter.index(of: .endOfScope(")"), after: i) else { return }
70+
guard formatter.next(.nonSpaceOrCommentOrLinebreak, after: closingIndex) != .startOfScope("{") else { return }
71+
72+
// Handle a single trailing closure
73+
if closures.count == 1 {
74+
let closure = closures[0]
75+
let range = closure.valueRange
76+
guard let closingBraceIndex = formatter.index(of: .nonSpaceOrComment, before: closingIndex, if: { $0 == .endOfScope("}") }),
77+
let openingBraceIndex = formatter.index(of: .startOfScope("{"), before: closingBraceIndex),
78+
formatter.index(of: .endOfScope("}"), before: openingBraceIndex) == nil,
79+
var startIndex = formatter.index(of: .nonSpaceOrLinebreak, before: openingBraceIndex)
80+
else { return }
81+
82+
switch formatter.tokens[startIndex] {
83+
case .delimiter(","), .startOfScope("("):
84+
break
85+
case .delimiter(":"):
86+
if let commaIndex = formatter.index(of: .delimiter(","), before: openingBraceIndex) {
87+
startIndex = commaIndex
88+
} else if formatter.index(of: .startOfScope("("), before: openingBraceIndex) == i {
89+
startIndex = i
90+
} else {
91+
return
92+
}
93+
default:
5094
return
5195
}
52-
if let commaIndex = formatter.index(of: .delimiter(","), before: openingBraceIndex) {
53-
startIndex = commaIndex
54-
} else if formatter.index(of: .startOfScope("("), before: openingBraceIndex) == i {
55-
startIndex = i
96+
let wasParen = (startIndex == i)
97+
formatter.removeParen(at: closingIndex)
98+
formatter.replaceTokens(in: startIndex ..< openingBraceIndex, with:
99+
wasParen ? [.space(" ")] : [.endOfScope(")"), .space(" ")])
100+
return
101+
}
102+
103+
// Handle multiple trailing closures
104+
var transformations: [(range: Range<Int>, tokens: [Token])] = []
105+
transformations.append((range: closingIndex ..< closingIndex + 1, tokens: []))
106+
107+
for (index, closure) in closures.enumerated() {
108+
let range = closure.valueRange
109+
guard range.lowerBound < formatter.tokens.count,
110+
range.upperBound < formatter.tokens.count,
111+
let openBrace = formatter.index(of: .nonSpaceOrCommentOrLinebreak, in: range.lowerBound ..< range.upperBound + 1),
112+
openBrace < formatter.tokens.count,
113+
formatter.tokens[openBrace] == .startOfScope("{"),
114+
let beforeBrace = formatter.index(of: .nonSpaceOrLinebreak, before: openBrace),
115+
beforeBrace < formatter.tokens.count else { continue }
116+
117+
if closure.label == nil {
118+
// First (unlabeled) closure
119+
if formatter.tokens[beforeBrace] == .delimiter(",") {
120+
let existingTokens = Array(formatter.tokens[(beforeBrace + 1) ..< openBrace])
121+
let hasLineBreak = existingTokens.contains { $0.isLinebreak }
122+
123+
if hasLineBreak {
124+
transformations.append((range: beforeBrace ..< openBrace, tokens: [
125+
.linebreak("\n", 0), .endOfScope(")"), .space(" "),
126+
]))
127+
} else {
128+
transformations.append((range: beforeBrace ..< openBrace, tokens: [
129+
.endOfScope(")"), .space(" "),
130+
]))
131+
}
132+
} else if formatter.tokens[beforeBrace] == .startOfScope("(") {
133+
transformations.append((range: beforeBrace ..< openBrace, tokens: [.space(" ")]))
134+
}
56135
} else {
57-
return
136+
// Labeled closure
137+
if let labelIndex = closure.labelIndex,
138+
let commaIndex = formatter.index(of: .delimiter(","), before: labelIndex),
139+
commaIndex < formatter.tokens.count
140+
{
141+
let hasLineBreakAfterComma = formatter.tokens[(commaIndex + 1) ..< labelIndex].contains { $0.isLinebreak }
142+
143+
if hasLineBreakAfterComma {
144+
transformations.append((range: commaIndex ..< commaIndex + 1, tokens: []))
145+
} else {
146+
let nextTokenIndex = commaIndex + 1
147+
if nextTokenIndex < labelIndex, formatter.tokens[nextTokenIndex].isSpace {
148+
transformations.append((range: commaIndex ..< commaIndex + 1, tokens: []))
149+
} else {
150+
transformations.append((range: commaIndex ..< commaIndex + 1, tokens: [.space(" ")]))
151+
}
152+
}
153+
}
58154
}
59-
default:
60-
return
155+
156+
// Remove trailing comma after last closure
157+
if index == closures.count - 1 {
158+
if let closingBrace = formatter.index(of: .endOfScope("}"), after: range.upperBound - 1),
159+
let commaAfter = formatter.index(of: .delimiter(","), after: closingBrace),
160+
commaAfter < closingIndex
161+
{
162+
transformations.append((range: commaAfter ..< commaAfter + 1, tokens: []))
163+
}
164+
}
165+
}
166+
167+
// Apply transformations from right to left
168+
for transformation in transformations.sorted(by: { $0.range.lowerBound > $1.range.lowerBound }) {
169+
formatter.replaceTokens(in: transformation.range, with: transformation.tokens)
61170
}
62-
let wasParen = (startIndex == i)
63-
formatter.removeParen(at: closingIndex)
64-
formatter.replaceTokens(in: startIndex ..< openingBraceIndex, with:
65-
wasParen ? [.space(" ")] : [.endOfScope(")"), .space(" ")])
66171
}
67172
} examples: {
68173
"""
@@ -75,6 +180,19 @@ public extension FormatRule {
75180
- let foo = bar.map({ ... }).joined()
76181
+ let foo = bar.map { ... }.joined()
77182
```
183+
184+
```diff
185+
- withAnimation(.spring, {
186+
- isVisible = true
187+
- }, completion: {
188+
- handleCompletion()
189+
- })
190+
+ withAnimation(.spring) {
191+
+ isVisible = true
192+
+ } completion: {
193+
+ handleCompletion()
194+
+ }
195+
```
78196
"""
79197
}
80198
}

Tests/Rules/TrailingClosuresTests.swift

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,68 @@ class TrailingClosuresTests: XCTestCase {
386386

387387
// multiple closures
388388

389+
func testMultipleTrailingClosuresWithFirstUnlabeled() {
390+
let input = """
391+
withAnimation(.linear, {
392+
// perform animation
393+
}, completion: {
394+
// handle completion
395+
})
396+
"""
397+
let output = """
398+
withAnimation(.linear) {
399+
// perform animation
400+
} completion: {
401+
// handle completion
402+
}
403+
"""
404+
testFormatting(for: input, output, rule: .trailingClosures)
405+
}
406+
407+
func testMultipleTrailingClosuresWithFirstLabeled() {
408+
let input = """
409+
withAnimation(.linear, animation: {
410+
// perform animation
411+
}, completion: {
412+
// handle completion
413+
})
414+
"""
415+
testFormatting(for: input, rule: .trailingClosures)
416+
}
417+
418+
func testMultipleTrailingClosuresWithThreeClosures() {
419+
let input = """
420+
performTask(param: 1, {
421+
// first closure
422+
}, onSuccess: {
423+
// success handler
424+
}, onFailure: {
425+
// failure handler
426+
})
427+
"""
428+
let output = """
429+
performTask(param: 1) {
430+
// first closure
431+
} onSuccess: {
432+
// success handler
433+
} onFailure: {
434+
// failure handler
435+
}
436+
"""
437+
testFormatting(for: input, output, rule: .trailingClosures)
438+
}
439+
440+
func testMultipleTrailingClosuresNotAppliedWhenFirstIsLabeled() {
441+
let input = """
442+
someFunction(param: 1, first: {
443+
// first closure
444+
}, second: {
445+
// second closure
446+
})
447+
"""
448+
testFormatting(for: input, rule: .trailingClosures)
449+
}
450+
389451
func testMultipleNestedClosures() throws {
390452
let repeatCount = 10
391453
let input = """
@@ -404,4 +466,32 @@ class TrailingClosuresTests: XCTestCase {
404466
"""
405467
testFormatting(for: input, rule: .trailingClosures)
406468
}
469+
470+
func testMultipleTrailingClosuresWithTrailingComma() {
471+
let input = """
472+
withAnimationIfNeeded(
473+
.linear,
474+
{ didAppear = true },
475+
completion: { animateText = true },
476+
)
477+
"""
478+
let output = """
479+
withAnimationIfNeeded(
480+
.linear
481+
) { didAppear = true }
482+
completion: { animateText = true }
483+
484+
"""
485+
testFormatting(for: input, [output], rules: [.trailingClosures, .indent])
486+
}
487+
488+
func testMultipleUnlabeledClosuresNotTransformed() {
489+
let input = """
490+
let foo = bar(
491+
{ baz },
492+
{ quux }
493+
)
494+
"""
495+
testFormatting(for: input, rule: .trailingClosures)
496+
}
407497
}

0 commit comments

Comments
 (0)