Skip to content

Commit dd2fb21

Browse files
committed
Add explicit_return opt-in rule: rebase, upgrade
1 parent 3d59e70 commit dd2fb21

18 files changed

+589
-388
lines changed

.swiftlint.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ disabled_rules:
2525
- discouraged_optional_collection
2626
- explicit_acl
2727
- explicit_enum_raw_value
28+
- explicit_return
2829
- explicit_top_level_acl
2930
- explicit_type_interface
3031
- file_types_order

CHANGELOG.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@
3333
built-in rules in accordance with the SARIF specification.
3434
[ahmadalfy](https://github.com/ahmadalfy)
3535
[#6499](https://github.com/realm/SwiftLint/issues/6499)
36+
37+
* Add `explicit_return` opt-in rule that warns against omitting the `return`
38+
keyword inside closures, functions and getters.
39+
[m-chojnacki](https://github.com/m-chojnacki)
40+
[talanov](https://github.com/talanov)
41+
[#3632](https://github.com/realm/SwiftLint/issues/3632)
42+
[#3859](https://github.com/realm/SwiftLint/issues/3859)
3643

3744
* Add `allow_underscore_prefixed_names` option to `unused_parameter` so
3845
underscore-prefixed parameter names can be treated as intentionally
@@ -3033,10 +3040,6 @@ accordingly._
30333040
[JP Simard](https://github.com/jpsim)
30343041

30353042
### Bug Fixes
3036-
* Add `explicit_return` opt-in rule that warns against omitting the `return`
3037-
keyword inside closures, functions and getters.
3038-
3039-
#### Bug Fixes
30403043

30413044
* Fix false positive in `self_in_property_initialization` rule when using
30423045
closures inside `didSet` and other accessors.
Lines changed: 19 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,32 @@
1-
public struct ExplicitReturnConfiguration: RuleConfiguration, Equatable {
2-
public enum ReturnKind: String, CaseIterable {
1+
import SwiftLintCore
2+
3+
@AutoConfigParser
4+
struct ExplicitReturnConfiguration: SeverityBasedRuleConfiguration {
5+
@AcceptableByConfigurationElement
6+
enum ReturnKind: String, CaseIterable, Comparable {
37
case closure
48
case function
59
case getter
6-
}
10+
case `subscript`
11+
case initializer
712

8-
public static let defaultIncludedKinds = Set(ReturnKind.allCases)
13+
static func < (lhs: Self, rhs: Self) -> Bool {
14+
lhs.rawValue < rhs.rawValue
15+
}
16+
}
917

10-
private(set) var severityConfiguration = SeverityConfiguration(.warning)
18+
static let defaultIncludedKinds: Set<ReturnKind> = [.function, .getter, .subscript, .initializer]
1119

20+
@ConfigurationElement(key: "severity")
21+
private(set) var severityConfiguration = SeverityConfiguration<Parent>(.warning)
22+
@ConfigurationElement(key: "included")
1223
private(set) var includedKinds = Self.defaultIncludedKinds
1324

14-
public var consoleDescription: String {
15-
let includedKinds = self.includedKinds.map { $0.rawValue }
16-
return severityConfiguration.consoleDescription +
17-
", included: [\(includedKinds.sorted().joined(separator: ", "))]"
18-
}
19-
20-
public init(includedKinds: Set<ReturnKind> = Self.defaultIncludedKinds) {
25+
init(includedKinds: Set<ReturnKind> = Self.defaultIncludedKinds) {
2126
self.includedKinds = includedKinds
2227
}
2328

24-
public mutating func apply(configuration: Any) throws {
25-
guard let configuration = configuration as? [String: Any] else {
26-
throw ConfigurationError.unknownConfiguration
27-
}
28-
29-
if let includedKinds = configuration["included"] as? [String] {
30-
self.includedKinds = try Set(includedKinds.map {
31-
guard let kind = ReturnKind(rawValue: $0) else {
32-
throw ConfigurationError.unknownConfiguration
33-
}
34-
35-
return kind
36-
})
37-
}
38-
39-
if let severityString = configuration["severity"] as? String {
40-
try severityConfiguration.apply(configuration: severityString)
41-
}
42-
}
43-
44-
public func isKindIncluded(_ kind: ReturnKind) -> Bool {
45-
return self.includedKinds.contains(kind)
29+
func isKindIncluded(_ kind: ReturnKind) -> Bool {
30+
includedKinds.contains(kind)
4631
}
4732
}
Lines changed: 78 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,122 +1,115 @@
11
import Foundation
2-
import SourceKittenFramework
32
import SwiftSyntax
43

5-
public struct ExplicitReturnRule: ConfigurationProviderRule, SubstitutionCorrectableRule, OptInRule {
6-
public var configuration = ExplicitReturnConfiguration()
4+
@SwiftSyntaxRule(correctable: true, optIn: true)
5+
struct ExplicitReturnRule: Rule {
6+
var configuration = ExplicitReturnConfiguration()
77

8-
public init() {}
9-
10-
public static let description = RuleDescription(
8+
static let description = RuleDescription(
119
identifier: "explicit_return",
1210
name: "Explicit Return",
13-
description: "Prefer explicit returns in closures, functions and getters.",
11+
description: "Prefer explicit returns in closures, functions and getters",
1412
kind: .style,
1513
nonTriggeringExamples: ExplicitReturnRuleExamples.nonTriggeringExamples,
1614
triggeringExamples: ExplicitReturnRuleExamples.triggeringExamples,
1715
corrections: ExplicitReturnRuleExamples.corrections
1816
)
19-
20-
public func validate(file: SwiftLintFile) -> [StyleViolation] {
21-
return violations(file: file).map {
22-
StyleViolation(
23-
ruleDescription: Self.description,
24-
severity: configuration.severityConfiguration.severity,
25-
location: Location(file: file, byteOffset: $0)
26-
)
27-
}
28-
}
29-
30-
public func violationRanges(in file: SwiftLintFile) -> [NSRange] {
31-
return violations(file: file).compactMap {
32-
file.stringView.byteRangeToNSRange(ByteRange(location: $0, length: 0))
33-
}
34-
}
35-
36-
public func substitution(for violationRange: NSRange, in file: SwiftLintFile) -> (NSRange, String)? {
37-
return (violationRange, "return ")
38-
}
39-
40-
private func violations(file: SwiftLintFile) -> [ByteCount] {
41-
guard let tree = file.syntaxTree else { return [] }
42-
43-
let visitor = ExplicitReturnVisitor(includedKinds: configuration.includedKinds)
44-
visitor.walk(tree)
45-
46-
return visitor.positions.map { ByteCount($0.utf8Offset) }
47-
}
4817
}
4918

50-
private final class ExplicitReturnVisitor: SyntaxVisitor {
51-
private let includedKinds: Set<ExplicitReturnConfiguration.ReturnKind>
52-
53-
private(set) var positions: [AbsolutePosition] = []
54-
55-
init(includedKinds: Set<ExplicitReturnConfiguration.ReturnKind>) {
56-
self.includedKinds = includedKinds
57-
}
58-
59-
override func visitPost(_ node: ClosureExprSyntax) {
60-
guard includedKinds.contains(.closure),
61-
let firstItem = node.statements.first?.item,
62-
node.statements.count == 1 else { return }
19+
private extension ExplicitReturnRule {
20+
final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
21+
override var skippableDeclarations: [any DeclSyntaxProtocol.Type] { [ProtocolDeclSyntax.self] }
6322

64-
if firstItem.isImplicitlyReturnable {
65-
positions.append(firstItem.positionAfterSkippingLeadingTrivia)
23+
override func visitPost(_ node: AccessorDeclSyntax) {
24+
if configuration.isKindIncluded(.getter),
25+
node.accessorSpecifier.tokenKind == .keyword(.get),
26+
let body = node.body {
27+
collectViolation(in: body.statements)
28+
}
6629
}
67-
}
6830

69-
override func visitPost(_ node: FunctionDeclSyntax) {
70-
guard includedKinds.contains(.function),
71-
node.signature.allowsImplicitReturns,
72-
let firstItem = node.body?.statements.first?.item,
73-
node.body?.statements.count == 1 else { return }
31+
override func visitPost(_ node: ClosureExprSyntax) {
32+
if configuration.isKindIncluded(.closure) {
33+
collectViolation(in: node.statements, isInsideClosure: true)
34+
}
35+
}
7436

75-
if firstItem.isImplicitlyReturnable {
76-
positions.append(firstItem.positionAfterSkippingLeadingTrivia)
37+
override func visitPost(_ node: FunctionDeclSyntax) {
38+
if configuration.isKindIncluded(.function),
39+
node.signature.allowsImplicitReturns,
40+
let body = node.body {
41+
collectViolation(in: body.statements)
42+
}
7743
}
78-
}
7944

80-
override func visitPost(_ node: VariableDeclSyntax) {
81-
guard includedKinds.contains(.getter) else { return }
45+
override func visitPost(_ node: InitializerDeclSyntax) {
46+
if configuration.isKindIncluded(.initializer),
47+
node.optionalMark != nil,
48+
let body = node.body {
49+
collectViolation(in: body.statements)
50+
}
51+
}
8252

83-
for binding in node.bindings {
84-
if let accessor = binding.accessor?.as(CodeBlockSyntax.self) {
85-
// Shorthand syntax for getters: `var foo: Int { 0 }`
86-
guard let firstItem = accessor.statements.first?.item,
87-
accessor.statements.count == 1 else { continue }
53+
override func visitPost(_ node: PatternBindingSyntax) {
54+
if configuration.isKindIncluded(.getter),
55+
case let .getter(itemList) = node.accessorBlock?.accessors {
56+
collectViolation(in: itemList)
57+
}
58+
}
8859

89-
if firstItem.isImplicitlyReturnable {
90-
positions.append(firstItem.positionAfterSkippingLeadingTrivia)
91-
}
92-
} else if let accessorBlock = binding.accessor?.as(AccessorBlockSyntax.self) {
93-
// Full syntax for getters: `var foo: Int { get { 0 } }`
94-
guard let accessor = accessorBlock.accessors.first(where: { $0.accessorKind.text == "get" }),
95-
let firstItem = accessor.body?.statements.first?.item,
96-
accessor.body?.statements.count == 1 else { continue }
60+
override func visitPost(_ node: SubscriptDeclSyntax) {
61+
if configuration.isKindIncluded(.subscript),
62+
case let .getter(itemList) = node.accessorBlock?.accessors {
63+
collectViolation(in: itemList)
64+
}
65+
}
9766

98-
if firstItem.isImplicitlyReturnable {
99-
positions.append(firstItem.positionAfterSkippingLeadingTrivia)
100-
}
67+
private func collectViolation(in itemList: CodeBlockItemListSyntax, isInsideClosure: Bool = false) {
68+
guard let onlyItem = itemList.onlyElement?.item,
69+
!onlyItem.is(ReturnStmtSyntax.self),
70+
Syntax(onlyItem).isProtocol((any ExprSyntaxProtocol).self) else {
71+
return
10172
}
73+
if isInsideClosure, Syntax(onlyItem).isFunctionCallExpr {
74+
return
75+
}
76+
let position = onlyItem.positionAfterSkippingLeadingTrivia
77+
violations.append(
78+
at: position,
79+
correction: .init(
80+
start: position,
81+
end: position,
82+
replacement: "return "
83+
)
84+
)
10285
}
10386
}
10487
}
10588

10689
private extension Syntax {
107-
var isImplicitlyReturnable: Bool {
108-
isProtocol(ExprSyntaxProtocol.self)
90+
var isFunctionCallExpr: Bool {
91+
if `is`(FunctionCallExprSyntax.self) {
92+
return true
93+
}
94+
if let tryExpr = `as`(TryExprSyntax.self) {
95+
return Syntax(tryExpr.expression).isFunctionCallExpr
96+
}
97+
if let awaitExpr = `as`(AwaitExprSyntax.self) {
98+
return Syntax(awaitExpr.expression).isFunctionCallExpr
99+
}
100+
return false
109101
}
110102
}
111103

112104
private extension FunctionSignatureSyntax {
113105
var allowsImplicitReturns: Bool {
114-
if let simpleType = output?.returnType.as(SimpleTypeIdentifierSyntax.self) {
115-
return simpleType.name.text != "Void" && simpleType.name.text != "Never"
116-
} else if let tupleType = output?.returnType.as(TupleTypeSyntax.self) {
106+
guard let returnClause else { return false }
107+
if let identifierType = returnClause.type.as(IdentifierTypeSyntax.self) {
108+
return identifierType.name.text != "Void" && identifierType.name.text != "Never"
109+
}
110+
if let tupleType = returnClause.type.as(TupleTypeSyntax.self) {
117111
return !tupleType.elements.isEmpty
118-
} else {
119-
return output != nil
120112
}
113+
return true
121114
}
122115
}

0 commit comments

Comments
 (0)