Add discouraged_void_return rule (realm#5400)

CHANGELOG.md

@@ -34,7 +34,11 @@
 
 * Add new `final_test_case` rule that triggers on non-final test classes.  
   [SimplyDanny](https://github.com/SimplyDanny)
-
+
+* Add new `discouraged_void_return` rule that triggers on return statements
+  in `Void` functions if the statement returns an expression.  
+  [SimplyDanny](https://github.com/SimplyDanny)
+
 * Allow to configure more operators in `identifier_name` rule. The new option
   is named `additional_operators`. Use it to add more operators to the list
   of default operators known to the rule.  

Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift

@@ -43,6 +43,7 @@ public let builtInRules: [any Rule.Type] = [
     DiscouragedObjectLiteralRule.self,
     DiscouragedOptionalBooleanRule.self,
     DiscouragedOptionalCollectionRule.self,
+    DiscouragedVoidReturnRule.self,
     DuplicateConditionsRule.self,
     DuplicateEnumCasesRule.self,
     DuplicateImportsRule.self,

Source/SwiftLintBuiltInRules/Rules/Lint/DiscouragedVoidReturnRule.swift

@@ -0,0 +1,179 @@
+import SwiftLintCore
+import SwiftSyntax
+
+@SwiftSyntaxRule
+struct DiscouragedVoidReturnRule: SwiftSyntaxCorrectableRule, OptInRule {
+    var configuration = SeverityConfiguration<Self>(.warning)
+
+    static var description = RuleDescription(
+        identifier: "discouraged_void_return",
+        name: "Discouraged Void Return",
+        description: "Functions without a return type should not return an expression",
+        kind: .style,
+        nonTriggeringExamples: [
+            Example("func f() -> Bool { return true }"),
+            Example("func f() -> Bool { true }"),
+            Example("func f() -> Void { g() }"),
+            Example("func f() -> () { g() }"),
+            Example("func f() { g() }"),
+            Example("func f() { { return g() }() }"),
+            Example("""
+            func f() {
+                func g() -> Int {
+                    return 1
+                }
+            }
+            """),
+            Example("init?() { return nil }"),
+            Example("""
+            func f() {
+                var i: Int { return 1 }
+            }
+            """)
+        ],
+        triggeringExamples: [
+            Example("func f() -> Void { ↓return g() }"),
+            Example("func f() -> () { ↓return g() }"),
+            Example("func f() { ↓return g() }"),
+            Example("""
+            func f(b: Bool) {
+                if b {
+                    ↓return g()
+                }
+            }
+            """)
+        ],
+        corrections: [
+            Example("""
+            func f() -> Void {
+                ↓return g()
+                // some comment
+            }
+            """): Example("""
+                func f() -> Void {
+                    g()
+                    return
+                    // some comment
+                }
+                """),
+            Example("""
+            func f(b: Bool) {
+                if b {
+                    // some comment
+                    ↓return g()
+                }
+            }
+            """): Example("""
+                func f(b: Bool) {
+                    if b {
+                        // some comment
+                        g()
+                        return
+                    }
+                }
+                """)
+        ]
+    )
+
+    func makeRewriter(file: SwiftLintFile) -> (some ViolationsSyntaxRewriter)? {
+       Rewriter(
+           configuration: configuration,
+           file: file,
+           disabledRegions: disabledRegions(file: file)
+       )
+   }
+}
+
+private extension DiscouragedVoidReturnRule {
+    final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
+        private var returnsVoidScope = Stack<Bool>()
+
+        override var skippableDeclarations: [any DeclSyntaxProtocol.Type] { [ProtocolDeclSyntax.self] }
+
+        override func visit(_ node: AccessorBlockSyntax) -> SyntaxVisitorContinueKind {
+            returnsVoidScope.push(false)
+            return .visitChildren
+        }
+
+        override func visitPost(_ node: AccessorBlockSyntax) {
+            returnsVoidScope.pop()
+        }
+
+        override func visit(_ node: ClosureExprSyntax) -> SyntaxVisitorContinueKind {
+            returnsVoidScope.push(false)
+            return .visitChildren
+        }
+
+        override func visitPost(_ node: ClosureExprSyntax) {
+            returnsVoidScope.pop()
+        }
+
+        override func visit(_ node: InitializerDeclSyntax) -> SyntaxVisitorContinueKind {
+            returnsVoidScope.push(false)
+            return .visitChildren
+        }
+
+        override func visitPost(_ node: InitializerDeclSyntax) {
+            returnsVoidScope.pop()
+        }
+
+        override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind {
+            if let type = node.signature.returnClause?.type {
+                if type.as(TupleTypeSyntax.self)?.elements.isEmpty == true ||
+                   type.as(IdentifierTypeSyntax.self)?.name.text == "Void" {
+                    returnsVoidScope.push(true)
+                } else {
+                    returnsVoidScope.push(false)
+                }
+            } else {
+                returnsVoidScope.push(true)
+            }
+            return .visitChildren
+        }
+
+        override func visitPost(_ node: FunctionDeclSyntax) {
+            returnsVoidScope.pop()
+        }
+
+        override func visitPost(_ node: ReturnStmtSyntax) {
+            if returnsVoidScope.peek() == true, node.expression != nil {
+                violations.append(node.positionAfterSkippingLeadingTrivia)
+            }
+        }
+    }
+
+    final class Rewriter: ViolationsSyntaxRewriter {
+        private let violationPositions: [AbsolutePosition]
+
+        init(configuration: ConfigurationType,
+             file: SwiftLintFile,
+             disabledRegions: [SourceRange]) {
+            self.violationPositions = Visitor(configuration: configuration, file: file).walk(file: file) {
+                $0.violations.map(\.position)
+            }
+            super.init(locationConverter: file.locationConverter, disabledRegions: disabledRegions)
+        }
+
+        override func visit(_ statements: CodeBlockItemListSyntax) -> CodeBlockItemListSyntax {
+            guard let returnStmt = statements.last?.item.as(ReturnStmtSyntax.self),
+                  let expr = returnStmt.expression,
+                  violationPositions.contains(returnStmt.positionAfterSkippingLeadingTrivia) else {
+                return super.visit(statements)
+            }
+            correctionPositions.append(returnStmt.positionAfterSkippingLeadingTrivia)
+            let newStmtList = Array(statements.dropLast()) + [
+                CodeBlockItemSyntax(item: .expr(expr))
+                    .with(\.leadingTrivia, returnStmt.leadingTrivia),
+                CodeBlockItemSyntax(item: .stmt(StmtSyntax(
+                    returnStmt
+                        .with(\.expression, nil)
+                        .with(
+                            \.leadingTrivia,
+                            .newline + (returnStmt.leadingTrivia.indentation(isOnNewline: false) ?? []))
+                        .with(\.trailingTrivia, returnStmt.trailingTrivia)
+                )))
+            ]
+            return super.visit(CodeBlockItemListSyntax(newStmtList))
+        }
+    }
+}

Tests/GeneratedTests/GeneratedTests.swift

@@ -248,6 +248,12 @@ class DiscouragedOptionalCollectionRuleGeneratedTests: SwiftLintTestCase {
     }
 }
 
+class DiscouragedVoidReturnRuleGeneratedTests: SwiftLintTestCase {
+    func testWithDefaultConfiguration() {
+        verifyRule(DiscouragedVoidReturnRule.description)
+    }
+}
+
 class DuplicateConditionsRuleGeneratedTests: SwiftLintTestCase {
     func testWithDefaultConfiguration() {
         verifyRule(DuplicateConditionsRule.description)