Add new `superfluous_else` rule (#4696)

.swiftlint.yml

@@ -44,6 +44,7 @@ disabled_rules:
   - self_binding
   - sorted_enum_cases
   - strict_fileprivate
+  - superfluous_else
   - switch_case_on_newline
   - trailing_closure
   - type_contents_order

CHANGELOG.md

@@ -10,6 +10,11 @@
 
 #### Enhancements
 
+* Add new `superfluous_else` rule that triggers on `if`-statements when an
+  attached `else`-block can be removed, because all branches of the previous
+  `if`-block(s) would certainly exit the current scope already.  
+  [SimplyDanny](https://github.com/SimplyDanny)
+
 * Add `sorted_enum_cases` rule which warns when enum cases are not sorted.  
   [kimdv](https://github.com/kimdv)
 

Source/SwiftLintFramework/Models/PrimaryRuleList.swift

@@ -187,6 +187,7 @@ let builtInRules: [Rule.Type] = [
     StrictFilePrivateRule.self,
     StrongIBOutletRule.self,
     SuperfluousDisableCommandRule.self,
+    SuperfluousElseRule.self,
     SwitchCaseAlignmentRule.self,
     SwitchCaseOnNewlineRule.self,
     SyntacticSugarRule.self,

Source/SwiftLintFramework/Rules/Style/SuperfluousElseRule.swift

@@ -0,0 +1,152 @@
+import SwiftSyntax
+
+struct SuperfluousElseRule: SwiftSyntaxRule, ConfigurationProviderRule, OptInRule {
+    var configuration = SeverityConfiguration(.warning)
+
+    init() {}
+
+    static var description = RuleDescription(
+        identifier: "superfluous_else",
+        name: "Superfluous Else",
+        description: "Else branches should be avoided when the previous if-block exits the current scope",
+        kind: .style,
+        nonTriggeringExamples: [
+            Example("""
+                if i > 0 {
+                    // comment
+                } else if i < 12 {
+                    return 2
+                } else {
+                    return 3
+                }
+            """),
+            Example("""
+                if i > 0 {
+                    let a = 1
+                    if a > 1 {
+                        // comment
+                    } else {
+                        return 1
+                    }
+                    // comment
+                } else {
+                    return 3
+                }
+            """),
+            Example("""
+                if i > 0 {
+                    if a > 1 {
+                        return 1
+                    }
+                } else {
+                    return 3
+                }
+            """),
+            Example("""
+                if i > 0 {
+                    if a > 1 {
+                        if a > 1 {
+                            // comment
+                        } else {
+                            return 1
+                        }
+                    }
+                } else {
+                    return 3
+                }
+            """, excludeFromDocumentation: true)
+        ],
+        triggeringExamples: [
+            Example("""
+                ↓if i > 0 {
+                    return 1
+                    // comment
+                } else {
+                    return 2
+                }
+            """),
+            Example("""
+                ↓if i > 0 {
+                    return 1
+                } else ↓if i < 12 {
+                    return 2
+                } else if i > 18 {
+                    return 3
+                }
+            """),
+            Example("""
+                ↓if i > 0 {
+                    ↓if i < 12 {
+                        return 5
+                    } else {
+                        ↓if i > 11 {
+                            return 6
+                        } else {
+                            return 7
+                        }
+                    }
+                } else ↓if i < 12 {
+                    return 2
+                } else ↓if i < 24 {
+                    return 8
+                } else {
+                    return 3
+                }
+            """)
+        ]
+    )
+
+    func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor {
+        Visitor(viewMode: .sourceAccurate)
+    }
+}
+
+private class Visitor: ViolationsSyntaxVisitor {
+    override var skippableDeclarations: [DeclSyntaxProtocol.Type] { [ProtocolDeclSyntax.self] }
+
+    override func visitPost(_ node: IfExprSyntax) {
+        if node.violatesRule {
+            violations.append(node.ifKeyword.positionAfterSkippingLeadingTrivia)
+        }
+    }
+}
+
+private extension IfExprSyntax {
+    var violatesRule: Bool {
+        if elseKeyword == nil {
+            return false
+        }
+        let thenBodyReturns = lastStatementReturns(in: body)
+        if thenBodyReturns, let parent = parent?.as(IfExprSyntax.self) {
+            return parent.violatesRule
+        }
+        return thenBodyReturns
+    }
+
+    private var returnsInAllBranches: Bool {
+        guard lastStatementReturns(in: body) else {
+            return false
+        }
+        if case let .ifExpr(nestedIfStmt) = elseBody {
+            return nestedIfStmt.returnsInAllBranches
+        }
+        if case let .codeBlock(block) = elseBody {
+            return lastStatementReturns(in: block)
+        }
+        return false
+    }
+
+    private func lastStatementReturns(in block: CodeBlockSyntax) -> Bool {
+        guard let lastItem = block.statements.last?.as(CodeBlockItemSyntax.self)?.item else {
+            return false
+        }
+        if lastItem.is(ReturnStmtSyntax.self) {
+            return true
+        }
+        if let exprStmt = lastItem.as(ExpressionStmtSyntax.self),
+           let lastIfStmt = exprStmt.expression.as(IfExprSyntax.self) {
+            return lastIfStmt.returnsInAllBranches
+        }
+        return false
+    }
+}

Tests/GeneratedTests/GeneratedTests.swift

@@ -1106,6 +1106,12 @@ class SuperfluousDisableCommandRuleGeneratedTests: XCTestCase {
     }
 }
 
+class SuperfluousElseRuleGeneratedTests: XCTestCase {
+    func testWithDefaultConfiguration() {
+        verifyRule(SuperfluousElseRule.description)
+    }
+}
+
 class SwitchCaseAlignmentRuleGeneratedTests: XCTestCase {
     func testWithDefaultConfiguration() {
         verifyRule(SwitchCaseAlignmentRule.description)