Skip to content

Commit

Permalink
Add new direct_return opt-in rule (#4717)
Browse files Browse the repository at this point in the history
  • Loading branch information
SimplyDanny authored Feb 19, 2023
1 parent f3e5557 commit 393318d
Show file tree
Hide file tree
Showing 4 changed files with 245 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@
[Moly](https://github.com/kyounh12)
[#4655](https://github.com/realm/SwiftLint/pull/4655)

* Add new `direct_return` rule that triggers on `return` statements returning
variables that have been declared in the statement before only.
[SimplyDanny](https://github.com/SimplyDanny)

* Add `period_spacing` opt-in rule that checks periods are not followed
by 2 or more spaces in comments.
[Julioacarrettoni](https://github.com/Julioacarrettoni)
Expand Down
3 changes: 2 additions & 1 deletion Source/SwiftLintFramework/Models/PrimaryRuleList.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Generated using Sourcery 1.9.2 — https://github.com/krzysztofzablocki/Sourcery
// Generated using Sourcery 2.0.0 — https://github.com/krzysztofzablocki/Sourcery
// DO NOT EDIT

/// The rule list containing all available rules built into SwiftLint.
Expand Down Expand Up @@ -35,6 +35,7 @@ let builtInRules: [Rule.Type] = [
CustomRules.self,
CyclomaticComplexityRule.self,
DeploymentTargetRule.self,
DirectReturnRule.self,
DiscardedNotificationCenterObserverRule.self,
DiscouragedAssertRule.self,
DiscouragedDirectInitRule.self,
Expand Down
232 changes: 232 additions & 0 deletions Source/SwiftLintFramework/Rules/Style/DirectReturnRule.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
import SwiftSyntax

struct DirectReturnRule: SwiftSyntaxCorrectableRule, ConfigurationProviderRule, OptInRule {
var configuration = SeverityConfiguration(.warning)

init() {}

static var description = RuleDescription(
identifier: "direct_return",
name: "Direct Return",
description: "Directly return the expression instead of assigning it to a variable first",
kind: .style,
nonTriggeringExamples: [
Example("""
func f() -> Int {
let b = 2
let a = 1
return b
}
"""),
Example("""
struct S {
var a: Int {
var b = 1
b = 2
return b
}
}
"""),
Example("""
func f() -> Int {
let b = 2
f()
return b
}
"""),
Example("""
func f() -> Int {
{ i in
let b = 2
return i
}(1)
}
""")
],
triggeringExamples: [
Example("""
func f() -> Int {
let ↓b = 2
return b
}
"""),
Example("""
struct S {
var a: Int {
var ↓b = 1
// comment
return b
}
}
"""),
Example("""
func f() -> Bool {
let a = 1, ↓b = true
return b
}
"""),
Example("""
func f() -> Int {
{ _ in
let ↓b = 2
return b
}(1)
}
"""),
Example("""
func f(i: Int) -> Int {
if i > 1 {
let ↓a = 2
return a
} else {
let ↓b = 2, a = 1
return b
}
}
""")
],
corrections: [
Example("""
func f() -> Int {
let b = 2
return b
}
"""): Example("""
func f() -> Int {
return 2
}
"""),
Example("""
struct S {
var a: Int {
var b = 2 > 1
? f()
: 1_000
// comment
return b
}
func f() -> Int { 1 }
}
"""): Example("""
struct S {
var a: Int {
// comment
return 2 > 1
? f()
: 1_000
}
func f() -> Int { 1 }
}
"""),
Example("""
func f() -> Bool {
let a = 1, b = true
return b
}
"""): Example("""
func f() -> Bool {
let a = 1
return true
}
"""),
Example("""
func f() -> Int {
{ _ in
let b = 2
return b
}(1)
}
"""): Example("""
func f() -> Int {
{ _ in
return 2
}(1)
}
""")
]
)

func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor {
Visitor(viewMode: .sourceAccurate)
}

func makeRewriter(file: SwiftLintFile) -> ViolationsSyntaxRewriter? {
Rewriter(
locationConverter: file.locationConverter,
disabledRegions: disabledRegions(file: file)
)
}
}

private class Visitor: ViolationsSyntaxVisitor {
override var skippableDeclarations: [DeclSyntaxProtocol.Type] { [ProtocolDeclSyntax.self] }

override func visitPost(_ statements: CodeBlockItemListSyntax) {
if let (binding, _) = statements.violation {
violations.append(binding.positionAfterSkippingLeadingTrivia)
}
}
}

private extension CodeBlockItemListSyntax {
var violation: (PatternBindingSyntax, ReturnStmtSyntax)? {
guard count >= 2, let last = last?.item,
let returnStmt = last.as(ReturnStmtSyntax.self),
let identifier = returnStmt.expression?.as(IdentifierExprSyntax.self)?.identifier.text,
let varDecl = dropLast().last?.item.as(VariableDeclSyntax.self) else {
return nil
}
let binding = varDecl.bindings.first {
$0.pattern.as(IdentifierPatternSyntax.self)?.identifier.text == identifier
}
if let binding {
return (binding, returnStmt)
}
return nil
}
}
private class Rewriter: SyntaxRewriter, ViolationsSyntaxRewriter {
private(set) var correctionPositions: [AbsolutePosition] = []
let locationConverter: SourceLocationConverter
let disabledRegions: [SourceRange]

init(locationConverter: SourceLocationConverter, disabledRegions: [SourceRange]) {
self.locationConverter = locationConverter
self.disabledRegions = disabledRegions
}

override func visit(_ statements: CodeBlockItemListSyntax) -> CodeBlockItemListSyntax {
guard let (binding, returnStmt) = statements.violation,
!returnStmt.isContainedIn(regions: disabledRegions, locationConverter: locationConverter),
let bindingList = binding.parent?.as(PatternBindingListSyntax.self),
let varDecl = bindingList.parent?.as(VariableDeclSyntax.self),
let initExpression = binding.initializer?.value else {
return super.visit(statements)
}
correctionPositions.append(binding.positionAfterSkippingLeadingTrivia)
var newStmtList = Array(statements.dropLast(2))
let newBindingList = bindingList
.filter { $0 != binding }
.enumerated()
.map { index, item in
if index == bindingList.count - 2 {
return item.withTrailingComma(false)
}
return item
}
if newBindingList.isNotEmpty {
newStmtList.append(CodeBlockItemSyntax(
item: .decl(DeclSyntax(varDecl.withBindings(PatternBindingListSyntax(newBindingList))))
))
newStmtList.append(CodeBlockItemSyntax(
item: .stmt(StmtSyntax(returnStmt.withExpression(initExpression)))
))
} else {
let leadingTrivia = (binding.trailingTrivia ?? .zero) + (returnStmt.leadingTrivia ?? .zero)
newStmtList.append(CodeBlockItemSyntax(
item: .stmt(StmtSyntax(returnStmt.withExpression(initExpression).withLeadingTrivia(leadingTrivia)))
))
}
return super.visit(CodeBlockItemListSyntax(newStmtList))
}
}
8 changes: 7 additions & 1 deletion Tests/GeneratedTests/GeneratedTests.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Generated using Sourcery 1.9.2 — https://github.com/krzysztofzablocki/Sourcery
// Generated using Sourcery 2.0.0 — https://github.com/krzysztofzablocki/Sourcery
// DO NOT EDIT
@_spi(TestHelper)
@testable import SwiftLintFramework
Expand Down Expand Up @@ -193,6 +193,12 @@ class DeploymentTargetRuleGeneratedTests: XCTestCase {
}
}

class DirectReturnRuleGeneratedTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(DirectReturnRule.description)
}
}

class DiscardedNotificationCenterObserverRuleGeneratedTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(DiscardedNotificationCenterObserverRule.description)
Expand Down

0 comments on commit 393318d

Please sign in to comment.