Skip to content

Commit eb6d75b

Browse files
allevatoallevato
authored
Merge pull request #596 from xedin/upstream-return-omission-rule
[Lint/Format] Add a rule to omit `return` from functions, closures, subscripts, and variables
2 parents e052da5 + eabc443 commit eb6d75b

File tree

5 files changed

+278
-0
lines changed

5 files changed

+278
-0
lines changed

‎Sources/SwiftFormat/Core/Pipelines+Generated.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ class LintPipeline: SyntaxVisitor {
6060
return .visitChildren
6161
}
6262

63+
override func visit(_ node: ClosureExprSyntax) -> SyntaxVisitorContinueKind {
64+
visitIfEnabled(OmitExplicitReturns.visit, for: node)
65+
return .visitChildren
66+
}
67+
6368
override func visit(_ node: ClosureParameterSyntax) -> SyntaxVisitorContinueKind {
6469
visitIfEnabled(NoLeadingUnderscores.visit, for: node)
6570
return .visitChildren
@@ -146,6 +151,7 @@ class LintPipeline: SyntaxVisitor {
146151
visitIfEnabled(AlwaysUseLowerCamelCase.visit, for: node)
147152
visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
148153
visitIfEnabled(NoLeadingUnderscores.visit, for: node)
154+
visitIfEnabled(OmitExplicitReturns.visit, for: node)
149155
visitIfEnabled(UseTripleSlashForDocumentationComments.visit, for: node)
150156
visitIfEnabled(ValidateDocumentationComments.visit, for: node)
151157
return .visitChildren
@@ -226,6 +232,7 @@ class LintPipeline: SyntaxVisitor {
226232
}
227233

228234
override func visit(_ node: PatternBindingSyntax) -> SyntaxVisitorContinueKind {
235+
visitIfEnabled(OmitExplicitReturns.visit, for: node)
229236
visitIfEnabled(UseSingleLinePropertyGetter.visit, for: node)
230237
return .visitChildren
231238
}
@@ -275,6 +282,7 @@ class LintPipeline: SyntaxVisitor {
275282
override func visit(_ node: SubscriptDeclSyntax) -> SyntaxVisitorContinueKind {
276283
visitIfEnabled(AllPublicDeclarationsHaveDocumentation.visit, for: node)
277284
visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
285+
visitIfEnabled(OmitExplicitReturns.visit, for: node)
278286
visitIfEnabled(UseTripleSlashForDocumentationComments.visit, for: node)
279287
return .visitChildren
280288
}
@@ -343,6 +351,7 @@ extension FormatPipeline {
343351
node = NoLabelsInCasePatterns(context: context).rewrite(node)
344352
node = NoParensAroundConditions(context: context).rewrite(node)
345353
node = NoVoidReturnOnFunctionSignature(context: context).rewrite(node)
354+
node = OmitExplicitReturns(context: context).rewrite(node)
346355
node = OneCasePerLine(context: context).rewrite(node)
347356
node = OneVariableDeclarationPerLine(context: context).rewrite(node)
348357
node = OrderedImports(context: context).rewrite(node)

‎Sources/SwiftFormat/Core/RuleNameCache+Generated.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public let ruleNameCache: [ObjectIdentifier: String] = [
3737
ObjectIdentifier(NoLeadingUnderscores.self): "NoLeadingUnderscores",
3838
ObjectIdentifier(NoParensAroundConditions.self): "NoParensAroundConditions",
3939
ObjectIdentifier(NoVoidReturnOnFunctionSignature.self): "NoVoidReturnOnFunctionSignature",
40+
ObjectIdentifier(OmitExplicitReturns.self): "OmitExplicitReturns",
4041
ObjectIdentifier(OneCasePerLine.self): "OneCasePerLine",
4142
ObjectIdentifier(OneVariableDeclarationPerLine.self): "OneVariableDeclarationPerLine",
4243
ObjectIdentifier(OnlyOneTrailingClosureArgument.self): "OnlyOneTrailingClosureArgument",

‎Sources/SwiftFormat/Rules/OmitExplicitReturns.swift

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import SwiftSyntax
14+
15+
/// Single-expression functions, closures, subscripts can omit `return` statement.
16+
///
17+
/// Lint: `func <name>() { return ... }` and similar single expression constructs will yield a lint error.
18+
///
19+
/// Format: `func <name>() { return ... }` constructs will be replaced with
20+
/// equivalent `func <name>() { ... }` constructs.
21+
@_spi(Rules)
22+
public final class OmitExplicitReturns: SyntaxFormatRule {
23+
public override class var isOptIn: Bool { return true }
24+
25+
public override func visit(_ node: FunctionDeclSyntax) -> DeclSyntax {
26+
let decl = super.visit(node)
27+
28+
// func <name>() -> <Type> { return ... }
29+
guard var funcDecl = decl.as(FunctionDeclSyntax.self),
30+
let body = funcDecl.body,
31+
let returnStmt = containsSingleReturn(body.statements) else {
32+
return decl
33+
}
34+
35+
funcDecl.body?.statements = rewrapReturnedExpression(returnStmt)
36+
diagnose(.omitReturnStatement, on: returnStmt, severity: .refactoring)
37+
return DeclSyntax(funcDecl)
38+
}
39+
40+
public override func visit(_ node: SubscriptDeclSyntax) -> DeclSyntax {
41+
let decl = super.visit(node)
42+
43+
guard var subscriptDecl = decl.as(SubscriptDeclSyntax.self),
44+
let accessorBlock = subscriptDecl.accessorBlock,
45+
// We are assuming valid Swift code here where only
46+
// one `get { ... }` is allowed.
47+
let transformed = transformAccessorBlock(accessorBlock) else {
48+
return decl
49+
}
50+
51+
subscriptDecl.accessorBlock = transformed
52+
return DeclSyntax(subscriptDecl)
53+
}
54+
55+
public override func visit(_ node: PatternBindingSyntax) -> PatternBindingSyntax {
56+
var binding = node
57+
58+
guard let accessorBlock = binding.accessorBlock,
59+
let transformed = transformAccessorBlock(accessorBlock) else {
60+
return node
61+
}
62+
63+
binding.accessorBlock = transformed
64+
return binding
65+
}
66+
67+
public override func visit(_ node: ClosureExprSyntax) -> ExprSyntax {
68+
let expr = super.visit(node)
69+
70+
// test { return ... }
71+
guard var closureExpr = expr.as(ClosureExprSyntax.self),
72+
let returnStmt = containsSingleReturn(closureExpr.statements) else {
73+
return expr
74+
}
75+
76+
closureExpr.statements = rewrapReturnedExpression(returnStmt)
77+
diagnose(.omitReturnStatement, on: returnStmt, severity: .refactoring)
78+
return ExprSyntax(closureExpr)
79+
}
80+
81+
private func transformAccessorBlock(_ accessorBlock: AccessorBlockSyntax) -> AccessorBlockSyntax? {
82+
// We are assuming valid Swift code here where only
83+
// one `get { ... }` is allowed.
84+
switch accessorBlock.accessors {
85+
case .accessors(let accessors):
86+
guard var getter = accessors.filter({
87+
$0.accessorSpecifier.tokenKind == .keyword(.get)
88+
}).first else {
89+
return nil
90+
}
91+
92+
guard let body = getter.body,
93+
let returnStmt = containsSingleReturn(body.statements) else {
94+
return nil
95+
}
96+
97+
guard let getterAt = accessors.firstIndex(where: {
98+
$0.accessorSpecifier.tokenKind == .keyword(.get)
99+
}) else {
100+
return nil
101+
}
102+
103+
getter.body?.statements = rewrapReturnedExpression(returnStmt)
104+
105+
diagnose(.omitReturnStatement, on: returnStmt, severity: .refactoring)
106+
107+
var newBlock = accessorBlock
108+
newBlock.accessors = .accessors(accessors.with(\.[getterAt], getter))
109+
return newBlock
110+
111+
case .getter(let getter):
112+
guard let returnStmt = containsSingleReturn(getter) else {
113+
return nil
114+
}
115+
116+
diagnose(.omitReturnStatement, on: returnStmt, severity: .refactoring)
117+
118+
var newBlock = accessorBlock
119+
newBlock.accessors = .getter(rewrapReturnedExpression(returnStmt))
120+
return newBlock
121+
}
122+
}
123+
124+
private func containsSingleReturn(_ body: CodeBlockItemListSyntax) -> ReturnStmtSyntax? {
125+
guard let element = body.firstAndOnly?.as(CodeBlockItemSyntax.self),
126+
let returnStmt = element.item.as(ReturnStmtSyntax.self) else
127+
{
128+
return nil
129+
}
130+
131+
return !returnStmt.children(viewMode: .all).isEmpty && returnStmt.expression != nil ? returnStmt : nil
132+
}
133+
134+
private func rewrapReturnedExpression(_ returnStmt: ReturnStmtSyntax) -> CodeBlockItemListSyntax {
135+
CodeBlockItemListSyntax([
136+
CodeBlockItemSyntax(
137+
leadingTrivia: returnStmt.leadingTrivia,
138+
item: .expr(returnStmt.expression!),
139+
semicolon: nil,
140+
trailingTrivia: returnStmt.trailingTrivia)
141+
])
142+
}
143+
}
144+
145+
extension Finding.Message {
146+
public static let omitReturnStatement: Finding.Message =
147+
"'return' can be omitted because body consists of a single expression"
148+
}

‎Sources/SwiftFormatConfiguration/RuleRegistry+Generated.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ enum RuleRegistry {
3636
"NoLeadingUnderscores": false,
3737
"NoParensAroundConditions": true,
3838
"NoVoidReturnOnFunctionSignature": true,
39+
"OmitExplicitReturns": false,
3940
"OneCasePerLine": true,
4041
"OneVariableDeclarationPerLine": true,
4142
"OnlyOneTrailingClosureArgument": true,

‎Tests/SwiftFormatTests/Rules/OmitReturnsTests.swift

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import _SwiftFormatTestSupport
2+
3+
@_spi(Rules) import SwiftFormat
4+
5+
final class OmitReturnsTests: LintOrFormatRuleTestCase {
6+
func testOmitReturnInFunction() {
7+
assertFormatting(
8+
OmitExplicitReturns.self,
9+
input: """
10+
func test() -> Bool {
11+
1️⃣return false
12+
}
13+
""",
14+
expected: """
15+
func test() -> Bool {
16+
false
17+
}
18+
""",
19+
findings: [
20+
FindingSpec("1️⃣", message: "'return' can be omitted because body consists of a single expression")
21+
])
22+
}
23+
24+
func testOmitReturnInClosure() {
25+
assertFormatting(
26+
OmitExplicitReturns.self,
27+
input: """
28+
vals.filter {
29+
1️⃣return $0.count == 1
30+
}
31+
""",
32+
expected: """
33+
vals.filter {
34+
$0.count == 1
35+
}
36+
""",
37+
findings: [
38+
FindingSpec("1️⃣", message: "'return' can be omitted because body consists of a single expression")
39+
])
40+
}
41+
42+
func testOmitReturnInSubscript() {
43+
assertFormatting(
44+
OmitExplicitReturns.self,
45+
input: """
46+
struct Test {
47+
subscript(x: Int) -> Bool {
48+
1️⃣return false
49+
}
50+
}
51+
52+
struct Test {
53+
subscript(x: Int) -> Bool {
54+
get {
55+
2️⃣return false
56+
}
57+
set { }
58+
}
59+
}
60+
""",
61+
expected: """
62+
struct Test {
63+
subscript(x: Int) -> Bool {
64+
false
65+
}
66+
}
67+
68+
struct Test {
69+
subscript(x: Int) -> Bool {
70+
get {
71+
false
72+
}
73+
set { }
74+
}
75+
}
76+
""",
77+
findings: [
78+
FindingSpec("1️⃣", message: "'return' can be omitted because body consists of a single expression"),
79+
FindingSpec("2️⃣", message: "'return' can be omitted because body consists of a single expression")
80+
])
81+
}
82+
83+
func testOmitReturnInComputedVars() {
84+
assertFormatting(
85+
OmitExplicitReturns.self,
86+
input: """
87+
var x: Int {
88+
1️⃣return 42
89+
}
90+
91+
struct Test {
92+
var x: Int {
93+
get {
94+
2️⃣return 42
95+
}
96+
set { }
97+
}
98+
}
99+
""",
100+
expected: """
101+
var x: Int {
102+
42
103+
}
104+
105+
struct Test {
106+
var x: Int {
107+
get {
108+
42
109+
}
110+
set { }
111+
}
112+
}
113+
""",
114+
findings: [
115+
FindingSpec("1️⃣", message: "'return' can be omitted because body consists of a single expression"),
116+
FindingSpec("2️⃣", message: "'return' can be omitted because body consists of a single expression")
117+
])
118+
}
119+
}

0 commit comments

Comments
 (0)