Skip to content

Commit ff7ac9a

Browse files
allevatoallevato
committed
Various fixes to ReturnVoidInsteadOfEmptyTuple.
- Handle `()` in closure signature return types. - Handle `()` when nested in the argument lists of function types and closures. - Ensure leading/trailing trivia of the original `()` is preserved. - Don't replace `()` if it has internal comments, like `(/*foo*/)`.
1 parent a50d4fe commit ff7ac9a

File tree

3 files changed

+232
-19
lines changed

3 files changed

+232
-19
lines changed

‎Sources/SwiftFormatRules/ReturnVoidInsteadOfEmptyTuple.swift

+73-5
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,82 @@ public final class ReturnVoidInsteadOfEmptyTuple: SyntaxFormatRule {
2525
public override func visit(_ node: FunctionTypeSyntax) -> TypeSyntax {
2626
guard let returnType = node.returnType.as(TupleTypeSyntax.self),
2727
returnType.elements.count == 0
28-
else { return TypeSyntax(node) }
29-
diagnose(.returnVoid, on: node.returnType)
30-
let voidKeyword = SyntaxFactory.makeSimpleTypeIdentifier(
28+
else {
29+
return super.visit(node)
30+
}
31+
32+
diagnose(.returnVoid, on: returnType)
33+
34+
// If the user has put non-whitespace trivia inside the empty tuple, like a comment, then we
35+
// still diagnose it as a lint error but we don't replace it because it's not obvious where the
36+
// comment should go.
37+
if hasNonWhitespaceLeadingTrivia(returnType.rightParen) {
38+
return super.visit(node)
39+
}
40+
41+
// Make sure that function types nested in the argument list are also rewritten (for example,
42+
// `(Int -> ()) -> ()` should become `(Int -> Void) -> Void`).
43+
let arguments = visit(node.arguments).as(TupleTypeElementListSyntax.self)!
44+
let voidKeyword = makeVoidIdentifierType(toReplace: returnType)
45+
return TypeSyntax(node.withArguments(arguments).withReturnType(TypeSyntax(voidKeyword)))
46+
}
47+
48+
public override func visit(_ node: ClosureSignatureSyntax) -> Syntax {
49+
guard let output = node.output,
50+
let returnType = output.returnType.as(TupleTypeSyntax.self),
51+
returnType.elements.count == 0
52+
else {
53+
return super.visit(node)
54+
}
55+
56+
diagnose(.returnVoid, on: returnType)
57+
58+
// If the user has put non-whitespace trivia inside the empty tuple, like a comment, then we
59+
// still diagnose it as a lint error but we don't replace it because it's not obvious where the
60+
// comment should go.
61+
if hasNonWhitespaceLeadingTrivia(returnType.rightParen) {
62+
return super.visit(node)
63+
}
64+
65+
let input: Syntax?
66+
if let parameterClause = node.input?.as(ParameterClauseSyntax.self) {
67+
// If the closure input is a complete parameter clause (variables and types), make sure that
68+
// nested function types are also rewritten (for example, `label: (Int -> ()) -> ()` should
69+
// become `label: (Int -> Void) -> Void`).
70+
input = visit(parameterClause)
71+
} else {
72+
// Otherwise, it's a simple signature (just variable names, no types), so there is nothing to
73+
// rewrite.
74+
input = node.input
75+
}
76+
let voidKeyword = makeVoidIdentifierType(toReplace: returnType)
77+
return Syntax(node.withInput(input).withOutput(output.withReturnType(TypeSyntax(voidKeyword))))
78+
}
79+
80+
/// Returns a value indicating whether the leading trivia of the given token contained any
81+
/// non-whitespace pieces.
82+
private func hasNonWhitespaceLeadingTrivia(_ token: TokenSyntax) -> Bool {
83+
for piece in token.leadingTrivia {
84+
switch piece {
85+
case .blockComment, .docBlockComment, .docLineComment, .garbageText, .lineComment:
86+
return true
87+
default:
88+
break
89+
}
90+
}
91+
return false
92+
}
93+
94+
/// Returns a type syntax node with the identifier `Void` whose leading and trailing trivia have
95+
/// been copied from the tuple type syntax node it is replacing.
96+
private func makeVoidIdentifierType(toReplace node: TupleTypeSyntax) -> SimpleTypeIdentifierSyntax
97+
{
98+
return SyntaxFactory.makeSimpleTypeIdentifier(
3199
name: SyntaxFactory.makeIdentifier(
32100
"Void",
33-
trailingTrivia: returnType.rightParen.trailingTrivia),
101+
leadingTrivia: node.firstToken?.leadingTrivia ?? [],
102+
trailingTrivia: node.lastToken?.trailingTrivia ?? []),
34103
genericArgumentClause: nil)
35-
return TypeSyntax(node.withReturnType(TypeSyntax(voidKeyword)))
36104
}
37105
}
38106

‎Tests/SwiftFormatRulesTests/ReturnVoidInsteadOfEmptyTupleTests.swift

+154-13
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,161 @@
11
import SwiftFormatRules
22

33
final class ReturnVoidInsteadOfEmptyTupleTests: LintOrFormatRuleTestCase {
4-
func testEmptyTupleReturns() {
4+
func testBasic() {
55
XCTAssertFormatting(
66
ReturnVoidInsteadOfEmptyTuple.self,
7-
input: """
8-
let callback: () -> ()
9-
typealias x = Int -> ()
10-
func y() -> Int -> () { return }
11-
func z(d: Bool -> ()) {}
12-
""",
13-
expected: """
14-
let callback: () -> Void
15-
typealias x = Int -> Void
16-
func y() -> Int -> Void { return }
17-
func z(d: Bool -> Void) {}
18-
""")
7+
input:
8+
"""
9+
let callback: () -> ()
10+
typealias x = Int -> ()
11+
func y() -> Int -> () { return }
12+
func z(d: Bool -> ()) {}
13+
""",
14+
expected:
15+
"""
16+
let callback: () -> Void
17+
typealias x = Int -> Void
18+
func y() -> Int -> Void { return }
19+
func z(d: Bool -> Void) {}
20+
""",
21+
checkForUnassertedDiagnostics: true
22+
)
23+
XCTAssertDiagnosed(.returnVoid, line: 1, column: 21)
24+
XCTAssertDiagnosed(.returnVoid, line: 2, column: 22)
25+
XCTAssertDiagnosed(.returnVoid, line: 3, column: 20)
26+
XCTAssertDiagnosed(.returnVoid, line: 4, column: 19)
27+
}
28+
29+
func testNestedFunctionTypes() {
30+
XCTAssertFormatting(
31+
ReturnVoidInsteadOfEmptyTuple.self,
32+
input:
33+
"""
34+
typealias Nested1 = (() -> ()) -> Int
35+
typealias Nested2 = (() -> ()) -> ()
36+
typealias Nested3 = Int -> (() -> ())
37+
""",
38+
expected:
39+
"""
40+
typealias Nested1 = (() -> Void) -> Int
41+
typealias Nested2 = (() -> Void) -> Void
42+
typealias Nested3 = Int -> (() -> Void)
43+
""",
44+
checkForUnassertedDiagnostics: true
45+
)
46+
XCTAssertDiagnosed(.returnVoid, line: 1, column: 28)
47+
XCTAssertDiagnosed(.returnVoid, line: 2, column: 28)
48+
XCTAssertDiagnosed(.returnVoid, line: 2, column: 35)
49+
XCTAssertDiagnosed(.returnVoid, line: 3, column: 35)
50+
}
51+
52+
func testClosureSignatures() {
53+
XCTAssertFormatting(
54+
ReturnVoidInsteadOfEmptyTuple.self,
55+
input:
56+
"""
57+
callWithTrailingClosure(arg) { arg -> () in body }
58+
callWithTrailingClosure(arg) { arg -> () in
59+
nestedCallWithTrailingClosure(arg) { arg -> () in
60+
body
61+
}
62+
}
63+
callWithTrailingClosure(arg) { (arg: () -> ()) -> Int in body }
64+
callWithTrailingClosure(arg) { (arg: () -> ()) -> () in body }
65+
""",
66+
expected:
67+
"""
68+
callWithTrailingClosure(arg) { arg -> Void in body }
69+
callWithTrailingClosure(arg) { arg -> Void in
70+
nestedCallWithTrailingClosure(arg) { arg -> Void in
71+
body
72+
}
73+
}
74+
callWithTrailingClosure(arg) { (arg: () -> Void) -> Int in body }
75+
callWithTrailingClosure(arg) { (arg: () -> Void) -> Void in body }
76+
""",
77+
checkForUnassertedDiagnostics: true
78+
)
79+
XCTAssertDiagnosed(.returnVoid, line: 1, column: 39)
80+
XCTAssertDiagnosed(.returnVoid, line: 2, column: 39)
81+
XCTAssertDiagnosed(.returnVoid, line: 3, column: 47)
82+
XCTAssertDiagnosed(.returnVoid, line: 7, column: 44)
83+
XCTAssertDiagnosed(.returnVoid, line: 8, column: 44)
84+
XCTAssertDiagnosed(.returnVoid, line: 8, column: 51)
85+
}
86+
87+
func testTriviaPreservation() {
88+
XCTAssertFormatting(
89+
ReturnVoidInsteadOfEmptyTuple.self,
90+
input:
91+
"""
92+
let callback: () -> /*foo*/()/*bar*/
93+
let callback: (Int -> /*foo*/ () /*bar*/) -> ()
94+
""",
95+
expected:
96+
"""
97+
let callback: () -> /*foo*/Void/*bar*/
98+
let callback: (Int -> /*foo*/ Void /*bar*/) -> Void
99+
""",
100+
checkForUnassertedDiagnostics: true
101+
)
102+
XCTAssertDiagnosed(.returnVoid, line: 1, column: 28)
103+
XCTAssertDiagnosed(.returnVoid, line: 2, column: 35)
104+
XCTAssertDiagnosed(.returnVoid, line: 2, column: 52)
105+
}
106+
107+
func testEmptyTupleWithInternalCommentsIsDiagnosedButNotReplaced() {
108+
XCTAssertFormatting(
109+
ReturnVoidInsteadOfEmptyTuple.self,
110+
input:
111+
"""
112+
let callback: () -> ( )
113+
let callback: () -> (\t)
114+
let callback: () -> (
115+
)
116+
let callback: () -> ( /* please don't change me! */ )
117+
let callback: () -> ( /** please don't change me! */ )
118+
let callback: () -> (
119+
// don't change me either!
120+
)
121+
let callback: () -> (
122+
/// don't change me either!
123+
)
124+
let callback: () -> (\u{feff})
125+
126+
let callback: (() -> ()) -> ( /* please don't change me! */ )
127+
callWithTrailingClosure(arg) { (arg: () -> ()) -> ( /* no change */ ) in body }
128+
""",
129+
expected:
130+
"""
131+
let callback: () -> Void
132+
let callback: () -> Void
133+
let callback: () -> Void
134+
let callback: () -> ( /* please don't change me! */ )
135+
let callback: () -> ( /** please don't change me! */ )
136+
let callback: () -> (
137+
// don't change me either!
138+
)
139+
let callback: () -> (
140+
/// don't change me either!
141+
)
142+
let callback: () -> (\u{feff})
143+
144+
let callback: (() -> Void) -> ( /* please don't change me! */ )
145+
callWithTrailingClosure(arg) { (arg: () -> Void) -> ( /* no change */ ) in body }
146+
""",
147+
checkForUnassertedDiagnostics: true
148+
)
149+
XCTAssertDiagnosed(.returnVoid, line: 1, column: 21)
150+
XCTAssertDiagnosed(.returnVoid, line: 2, column: 21)
151+
XCTAssertDiagnosed(.returnVoid, line: 3, column: 21)
152+
XCTAssertDiagnosed(.returnVoid, line: 5, column: 21)
153+
XCTAssertDiagnosed(.returnVoid, line: 6, column: 21)
154+
XCTAssertDiagnosed(.returnVoid, line: 7, column: 21)
155+
XCTAssertDiagnosed(.returnVoid, line: 10, column: 21)
156+
XCTAssertDiagnosed(.returnVoid, line: 15, column: 22)
157+
XCTAssertDiagnosed(.returnVoid, line: 15, column: 29)
158+
XCTAssertDiagnosed(.returnVoid, line: 16, column: 44)
159+
XCTAssertDiagnosed(.returnVoid, line: 16, column: 51)
19160
}
20161
}

‎Tests/SwiftFormatRulesTests/XCTestManifests.swift

+5-1
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,11 @@ extension ReturnVoidInsteadOfEmptyTupleTests {
290290
// `swift test --generate-linuxmain`
291291
// to regenerate.
292292
static let __allTests__ReturnVoidInsteadOfEmptyTupleTests = [
293-
("testEmptyTupleReturns", testEmptyTupleReturns),
293+
("testBasic", testBasic),
294+
("testClosureSignatures", testClosureSignatures),
295+
("testEmptyTupleWithInternalCommentsIsDiagnosedButNotReplaced", testEmptyTupleWithInternalCommentsIsDiagnosedButNotReplaced),
296+
("testNestedFunctionTypes", testNestedFunctionTypes),
297+
("testTriviaPreservation", testTriviaPreservation),
294298
]
295299
}
296300

0 commit comments

Comments
 (0)