Add back return_value_from_void_function rule (#3882)

.swiftlint.yml

@@ -59,6 +59,7 @@ opt_in_rules:
   - reduce_into
   - redundant_nil_coalescing
   - redundant_type_annotation
+  - return_value_from_void_function
   - single_test_class
   - sorted_first_last
   - sorted_imports

CHANGELOG.md

@@ -26,6 +26,10 @@
   [Juozas Valancius](https://github.com/juozasvalancius)
   [#3840](https://github.com/realm/SwiftLint/issues/3840)
 
+* Add back `return_value_from_void_function` opt-in rule to warn against using
+  `return <expression>` in a function that returns `Void`.  
+  [Marcelo Fabri](https://github.com/marcelofabri)
+
 * Don't skip autocorrect on files that have parser warnings. Only files with
   errors reported by the Swift parser will be skipped.  
   [Marcelo Fabri](https://github.com/marcelofabri)

Source/SwiftLintFramework/Models/PrimaryRuleList.swift

@@ -161,6 +161,7 @@ public let primaryRuleList = RuleList(rules: [
     RequiredDeinitRule.self,
     RequiredEnumCaseRule.self,
     ReturnArrowWhitespaceRule.self,
+    ReturnValueFromVoidFunctionRule.self,
     SelfInPropertyInitializationRule.self,
     ShorthandOperatorRule.self,
     SingleTestClassRule.self,

Source/SwiftLintFramework/Rules/Idiomatic/ReturnValueFromVoidFunctionRule.swift

@@ -0,0 +1,72 @@
+import SourceKittenFramework
+import SwiftSyntax
+
+public struct ReturnValueFromVoidFunctionRule: ConfigurationProviderRule, OptInRule, AutomaticTestableRule {
+    public var configuration = SeverityConfiguration(.warning)
+
+    public init() {}
+
+    public static let description = RuleDescription(
+        identifier: "return_value_from_void_function",
+        name: "Return Value from Void Function",
+        description: "Returning values from Void functions should be avoided.",
+        kind: .idiomatic,
+        minSwiftVersion: .fiveDotOne,
+        nonTriggeringExamples: ReturnValueFromVoidFunctionRuleExamples.nonTriggeringExamples,
+        triggeringExamples: ReturnValueFromVoidFunctionRuleExamples.triggeringExamples
+    )
+
+    public func validate(file: SwiftLintFile) -> [StyleViolation] {
+        guard let tree = file.syntaxTree else {
+            return []
+        }
+
+        let visitor = ReturnValueFromVoidFunctionVisitor()
+        visitor.walk(tree)
+        return visitor.violations(for: self, in: file)
+    }
+}
+
+private final class ReturnValueFromVoidFunctionVisitor: SyntaxVisitor {
+    private var positions = [AbsolutePosition]()
+
+    override func visitPost(_ node: ReturnStmtSyntax) {
+        if node.expression != nil,
+           let functionNode = Syntax(node).enclosingFunction(),
+            functionNode.returnsVoid {
+            positions.append(node.positionAfterSkippingLeadingTrivia)
+        }
+    }
+
+    func violations(for rule: ReturnValueFromVoidFunctionRule, in file: SwiftLintFile) -> [StyleViolation] {
+        return positions.map { position in
+            StyleViolation(ruleDescription: type(of: rule).description,
+                           severity: rule.configuration.severity,
+                           location: Location(file: file, byteOffset: ByteCount(position.utf8Offset)))
+        }
+    }
+}
+
+private extension Syntax {
+    func enclosingFunction() -> FunctionDeclSyntax? {
+        if let node = self.as(FunctionDeclSyntax.self) {
+            return node
+        }
+
+        if self.is(ClosureExprSyntax.self) || self.is(VariableDeclSyntax.self) {
+            return nil
+        }
+
+        return parent?.enclosingFunction()
+    }
+}
+
+private extension FunctionDeclSyntax {
+    var returnsVoid: Bool {
+        if let type = signature.output?.returnType.as(SimpleTypeIdentifierSyntax.self) {
+            return type.name.text == "Void"
+        }
+
+        return signature.output?.returnType == nil
+    }
+}

Source/SwiftLintFramework/Rules/Idiomatic/ReturnValueFromVoidFunctionRuleExamples.swift

@@ -0,0 +1,234 @@
+// swiftlint:disable:next type_body_length
+internal struct ReturnValueFromVoidFunctionRuleExamples {
+    static let nonTriggeringExamples = [
+        Example("""
+        func foo() {
+            return
+        }
+        """),
+        Example("""
+        func foo() {
+            return /* a comment */
+        }
+        """),
+        Example("""
+        func foo() -> Int {
+            return 1
+        }
+        """),
+        Example("""
+        func foo() -> Void {
+            if condition {
+                return
+            }
+            bar()
+        }
+        """),
+        Example("""
+        func foo() {
+            return;
+            bar()
+        }
+        """),
+        Example("func test() {}"),
+        Example("""
+        init?() {
+            guard condition else {
+                return nil
+            }
+        }
+        """),
+        Example("""
+        init?(arg: String?) {
+            guard arg != nil else {
+                return nil
+            }
+        }
+        """),
+        Example("""
+        func test() {
+            guard condition else {
+                return
+            }
+        }
+        """),
+        Example("""
+        func test() -> Result<String, Error> {
+            func other() {}
+            func otherVoid() -> Void {}
+        }
+        """),
+        Example("""
+        func test() -> Int? {
+            return nil
+        }
+        """),
+        Example("""
+        func test() {
+            if bar {
+                print("")
+                return
+            }
+            let foo = [1, 2, 3].filter { return true }
+            return
+        }
+        """),
+        Example("""
+        func test() {
+            guard foo else {
+                bar()
+                return
+            }
+        }
+        """),
+        Example("""
+        func spec() {
+            var foo: Int {
+                return 0
+            }
+        """)
+    ]
+
+    static let triggeringExamples = [
+        Example("""
+        func foo() {
+            ↓return bar()
+        }
+        """),
+        Example("""
+        func foo() {
+            ↓return self.bar()
+        }
+        """),
+        Example("""
+        func foo() -> Void {
+            ↓return bar()
+        }
+        """),
+        Example("""
+        func foo() -> Void {
+            ↓return /* comment */ bar()
+        }
+        """),
+        Example("""
+        func foo() {
+            ↓return
+            self.bar()
+        }
+        """),
+        Example("""
+        func foo() {
+            variable += 1
+            ↓return
+            variable += 1
+        }
+        """),
+        Example("""
+        func initThing() {
+            guard foo else {
+                ↓return print("")
+            }
+        }
+        """),
+        Example("""
+        // Leading comment
+        func test() {
+            guard condition else {
+                ↓return assertionfailure("")
+            }
+        }
+        """),
+        Example("""
+        func test() -> Result<String, Error> {
+            func other() {
+                guard false else {
+                    ↓return assertionfailure("")
+                }
+            }
+            func otherVoid() -> Void {}
+        }
+        """),
+        Example("""
+        func test() {
+            guard conditionIsTrue else {
+                sideEffects()
+                return // comment
+            }
+            guard otherCondition else {
+                ↓return assertionfailure("")
+            }
+            differentSideEffect()
+        }
+        """),
+        Example("""
+        func test() {
+            guard otherCondition else {
+                ↓return assertionfailure(""); // comment
+            }
+            differentSideEffect()
+        }
+        """),
+        Example("""
+        func test() {
+          if x {
+            ↓return foo()
+          }
+          bar()
+        }
+        """),
+        Example("""
+        func test() {
+          switch x {
+            case .a:
+              ↓return foo() // return to skip baz()
+            case .b:
+              bar()
+          }
+          baz()
+        }
+        """),
+        Example("""
+        func test() {
+          if check {
+            if otherCheck {
+              ↓return foo()
+            }
+          }
+          bar()
+        }
+        """),
+        Example("""
+        func test() {
+            ↓return foo()
+        }
+        """),
+        Example("""
+        func test() {
+          ↓return foo({
+            return bar()
+          })
+        }
+        """),
+        Example("""
+        func test() {
+          guard x else {
+            ↓return foo()
+          }
+          bar()
+        }
+        """),
+        Example("""
+        func test() {
+          let closure: () -> () = {
+            return assert()
+          }
+          if check {
+            if otherCheck {
+              return // comments are fine
+            }
+          }
+          ↓return foo()
+        }
+        """)
+    ]
+}

Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift

@@ -701,6 +701,12 @@ class ReturnArrowWhitespaceRuleTests: XCTestCase {
     }
 }
 
+class ReturnValueFromVoidFunctionRuleTests: XCTestCase {
+    func testWithDefaultConfiguration() {
+        verifyRule(ReturnValueFromVoidFunctionRule.description)
+    }
+}
+
 class SelfInPropertyInitializationRuleTests: XCTestCase {
     func testWithDefaultConfiguration() {
         verifyRule(SelfInPropertyInitializationRule.description)