Add new unneeded_throws_rethrows rule (#6069)

Co-authored-by: Danny Mösch <danny.moesch@icloud.com>

.swiftlint.yml

@@ -46,6 +46,7 @@ disabled_rules:
   - todo
   - trailing_closure
   - type_contents_order
+  - unneeded_throws_rethrows
   - vertical_whitespace_between_cases
 
 # Configurations

CHANGELOG.md

@@ -423,6 +423,10 @@
   [p4checo](https://github.com/p4checo)
   [#5965](https://github.com/realm/SwiftLint/issues/5965)
 
+* Add opt-in `unneeded_throws_rethrows` rule that triggers when declarations
+  marked `throws`/`rethrows` never actually throw or call any throwing code.  
+  [Tony Ngo](https://github.com/tonyskansf)
+
 ### Bug Fixes
 
 * Fix `no_extension_access_modifier` rule incorrectly triggering for `nonisolated extension`.

Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift

@@ -224,6 +224,7 @@ public let builtInRules: [any Rule.Type] = [
     UnneededOverrideRule.self,
     UnneededParenthesesInClosureArgumentRule.self,
     UnneededSynthesizedInitializerRule.self,
+    UnneededThrowsRule.self,
     UnownedVariableCaptureRule.self,
     UntypedErrorInCatchRule.self,
     UnusedClosureParameterRule.self,

Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift

@@ -0,0 +1,242 @@
+import SwiftLintCore
+import SwiftSyntax
+
+@SwiftSyntaxRule(correctable: true, optIn: true)
+struct UnneededThrowsRule: Rule {
+    var configuration = SeverityConfiguration<Self>(.warning)
+
+    static let description = RuleDescription(
+        identifier: "unneeded_throws_rethrows",
+        name: "Unneeded (Re)Throws Keyword",
+        description: "Non-throwing functions/properties/closures should not be marked as `throws` or `rethrows`.",
+        kind: .lint,
+        nonTriggeringExamples: UnneededThrowsRuleExamples.nonTriggeringExamples,
+        triggeringExamples: UnneededThrowsRuleExamples.triggeringExamples,
+        corrections: UnneededThrowsRuleExamples.corrections
+    )
+}
+
+private extension UnneededThrowsRule {
+    struct Scope {
+        var throwsClause: ThrowsClauseSyntax?
+    }
+
+    final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
+        private var scopes = Stack<Scope>()
+
+        override var skippableDeclarations: [any DeclSyntaxProtocol.Type] {
+            [
+                ProtocolDeclSyntax.self,
+                TypeAliasDeclSyntax.self,
+            ]
+        }
+
+        override func visit(_: FunctionParameterClauseSyntax) -> SyntaxVisitorContinueKind {
+            .skipChildren
+        }
+
+        override func visit(_ node: InitializerDeclSyntax) -> SyntaxVisitorContinueKind {
+            scopes.openScope(with: node.signature.effectSpecifiers?.throwsClause)
+            return .visitChildren
+        }
+
+        override func visitPost(_: InitializerDeclSyntax) {
+            if let closedScope = scopes.closeScope() {
+                validate(
+                    scope: closedScope,
+                    construct: "initializer"
+                )
+            }
+        }
+
+        override func visit(_ node: AccessorDeclSyntax) -> SyntaxVisitorContinueKind {
+            scopes.openScope(with: node.effectSpecifiers?.throwsClause)
+            return .visitChildren
+        }
+
+        override func visitPost(_: AccessorDeclSyntax) {
+            if let closedScope = scopes.closeScope() {
+                validate(
+                    scope: closedScope,
+                    construct: "accessor"
+                )
+            }
+        }
+
+        override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind {
+            scopes.openScope(with: node.signature.effectSpecifiers?.throwsClause)
+            return .visitChildren
+        }
+
+        override func visitPost(_: FunctionDeclSyntax) {
+            if let closedScope = scopes.closeScope() {
+                validate(
+                    scope: closedScope,
+                    construct: "body of this function"
+                )
+            }
+        }
+
+        override func visit(_ node: PatternBindingSyntax) -> SyntaxVisitorContinueKind {
+            if let lintableFunctionType = node.lintableFunctionType {
+                scopes.openScope(with: lintableFunctionType.effectSpecifiers?.throwsClause)
+            }
+            return .visitChildren
+        }
+
+        override func visitPost(_ node: PatternBindingSyntax) {
+            if node.lintableFunctionType != nil {
+                if let closedScope = scopes.closeScope() {
+                    validate(
+                        scope: closedScope,
+                        construct: "closure type"
+                    )
+                }
+            }
+        }
+
+        override func visit(_ node: ClosureExprSyntax) -> SyntaxVisitorContinueKind {
+            if let throwsClause = node.signature?.effectSpecifiers?.throwsClause {
+                scopes.openScope(with: throwsClause)
+            }
+            return .visitChildren
+        }
+
+        override func visitPost(_ node: ClosureExprSyntax) {
+            if node.signature?.effectSpecifiers?.throwsClause != nil, let closedScope = scopes.closeScope() {
+                validate(
+                    scope: closedScope,
+                    construct: "body of this closure"
+                )
+            }
+        }
+
+        override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind {
+            if node.containsClosureDeclaration {
+                scopes.openScope()
+            }
+            return .visitChildren
+        }
+
+        override func visitPost(_ node: FunctionCallExprSyntax) {
+            if node.containsClosureDeclaration {
+                scopes.closeScope()
+            }
+        }
+
+        override func visit(_: DoStmtSyntax) -> SyntaxVisitorContinueKind {
+            scopes.openScope()
+            return .visitChildren
+        }
+
+        override func visitPost(_ node: CodeBlockSyntax) {
+            if node.parent?.is(DoStmtSyntax.self) == true {
+                scopes.closeScope()
+            }
+        }
+
+        override func visitPost(_ node: DoStmtSyntax) {
+            if node.catchClauses.contains(where: \.catchItems.isEmpty) {
+                // All errors will be caught.
+                return
+            }
+            scopes.markCurrentScopeAsThrowing()
+        }
+
+        override func visitPost(_ node: ForStmtSyntax) {
+            if node.tryKeyword != nil {
+                scopes.markCurrentScopeAsThrowing()
+            }
+        }
+
+        override func visitPost(_ node: TryExprSyntax) {
+            if node.questionOrExclamationMark == nil {
+                scopes.markCurrentScopeAsThrowing()
+            }
+        }
+
+        override func visitPost(_: ThrowStmtSyntax) {
+            scopes.markCurrentScopeAsThrowing()
+        }
+
+        private func validate(scope: Scope, construct: String) {
+            guard let throwsClause = scope.throwsClause else { return }
+            violations.append(
+                .init(
+                    position: throwsClause.positionAfterSkippingLeadingTrivia,
+                    reason: "Superfluous 'throws'; \(construct) does not throw any error",
+                    correction: .init(
+                        // Move start position back by 1 to include the space before the keyword.
+                        start: throwsClause.positionAfterSkippingLeadingTrivia.advanced(by: -1),
+                        end: throwsClause.endPositionBeforeTrailingTrivia,
+                        replacement: ""
+                    )
+                )
+            )
+        }
+    }
+}
+
+private extension Stack where Element == UnneededThrowsRule.Scope {
+    mutating func markCurrentScopeAsThrowing() {
+        modifyLast { currentScope in
+            currentScope.throwsClause = nil
+        }
+    }
+
+    mutating func openScope(with throwsClause: ThrowsClauseSyntax? = nil) {
+        push(UnneededThrowsRule.Scope(throwsClause: throwsClause))
+    }
+
+    @discardableResult
+    mutating func closeScope() -> Element? {
+        pop()
+    }
+}
+
+private extension FunctionCallExprSyntax {
+    var containsClosureDeclaration: Bool {
+        children(viewMode: .sourceAccurate).contains { $0.is(ClosureExprSyntax.self) }
+    }
+}
+
+private extension PatternBindingSyntax {
+    private var hasNonReferenceInitializer: Bool {
+        ![.declReferenceExpr, .memberAccessExpr, .functionCallExpr, nil].contains(initializer?.value.kind)
+    }
+
+    private var isLetBinding: Bool {
+        parent?.as(PatternBindingListSyntax.self)?
+            .parent?.as(VariableDeclSyntax.self)?
+            .bindingSpecifier.tokenKind == .keyword(.let)
+    }
+
+    var lintableFunctionType: FunctionTypeSyntax? {
+        guard isLetBinding, hasNonReferenceInitializer else {
+            return nil
+        }
+        return typeAnnotation?.type.baseFunctionTypeSyntax
+    }
+}
+
+private extension TypeSyntax {
+    var baseFunctionTypeSyntax: FunctionTypeSyntax? {
+        switch Syntax(self).as(SyntaxEnum.self) {
+        case .functionType(let function):
+            function
+        case .optionalType(let optional):
+            optional.wrappedType.baseFunctionTypeSyntax
+        case .attributedType(let attributed):
+            attributed.baseType.baseFunctionTypeSyntax
+        case .tupleType(let tuple):
+            // It's hard to check for the necessity of throws keyword in multi-element tuples.
+            if tuple.elements.count == 1 {
+                tuple.elements.first?.type.baseFunctionTypeSyntax
+            } else {
+                nil
+            }
+        default:
+            nil
+        }
+    }
+}

Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift

@@ -0,0 +1,394 @@
+internal struct UnneededThrowsRuleExamples {
+    static let nonTriggeringExamples = [
+        Example("""
+            func foo() throws {
+                try bar()
+            }
+            """),
+        Example("""
+            func foo() throws {
+                throw Example.failure
+            }
+            """),
+        Example("""
+            func foo() throws(Example) {
+                throw Example.failure
+            }
+            """),
+        Example("""
+            func foo(_ bar: () throws -> T) rethrows -> Int {
+                try items.map { try bar() }
+            }
+            """),
+        Example("""
+            func foo() {
+                func bar() throws {
+                    try baz()
+                }
+                try? bar()
+            }
+            """),
+        Example("""
+            protocol Foo {
+                func bar() throws
+            }
+            """),
+        Example("""
+            func foo() throws {
+                guard false else {
+                    throw Example.failure
+                }
+            }
+            """),
+        Example("""
+            func foo() throws {
+                do { try bar() }
+                catch {
+                    throw Example.failure
+                }
+            }
+            """),
+        Example("""
+            func foo() throws {
+                do { try bar() }
+                catch {
+                    try baz()
+                }
+            }
+            """),
+        Example("""
+        func foo() throws {
+            do {
+                throw Example.failure
+            } catch {
+                do {
+                    throw Example.failure
+                } catch {
+                    throw Example.failure
+                }
+            }
+        }
+        """),
+        Example("""
+            func foo() throws {
+                switch bar {
+                case 1: break
+                default: try bar()
+                }
+            }
+            """),
+        Example("""
+            var foo: Int {
+                get throws {
+                    try bar
+                }
+            }
+            """),
+        Example("""
+        func foo() throws {
+            let bar = Bar()
+
+            if bar.boolean {
+                throw Example.failure
+            }
+        }
+        """),
+        Example("""
+        func foo() throws -> Bar? {
+            Bar(try baz())
+        }
+        """),
+        Example("""
+        typealias Foo = () throws -> Void
+        """),
+        Example("""
+        enum Foo {
+            case foo
+            case bar(() throws -> Void)
+        }
+        """),
+        Example("""
+        func foo() async throws {
+            for try await item in items {}
+        }
+        """),
+        Example("""
+        let foo: () throws -> Void
+        """),
+        Example("""
+        let foo: @Sendable () throws -> Void
+        """),
+        Example("""
+        let foo: (() throws -> Void)?
+        """),
+        Example("""
+        func foo(_ bar: () throws -> Void = {}) {}
+        """),
+        Example("""
+        func foo() async throws {
+            func foo() {}
+            for _ in 0..<count {
+                foo()
+                try await bar()
+            }
+        }
+        """),
+        Example("""
+        func foo() throws {
+            do { try bar() }
+            catch Example.failure {}
+        }
+        """),
+        Example("""
+        func foo() throws {
+            do { try bar() }
+            catch is SomeError { throw AnotherError }
+            catch is AnotherError {}
+        }
+        """),
+        Example("let s: S<() throws -> Void> = S()"),
+        Example("let foo: (() throws -> Void, Int) = ({}, 1)"),
+        Example("let foo: (Int, () throws -> Void) = (1, {})"),
+        Example("let foo: (Int, Int, () throws -> Void) = (1, 1, {})"),
+        Example("let foo: () throws -> Void = { try bar() }"),
+        Example("let foo: () throws -> Void = bar"),
+        Example("var foo: () throws -> Void = {}"),
+        Example("let x = { () throws -> Void in try baz() }"),
+    ]
+
+    static let triggeringExamples = [
+        Example("func foo() ↓throws {}"),
+        Example("let foo: () ↓throws -> Void = {}"),
+        Example("let foo: (() ↓throws -> Void) = {}"),
+        Example("let foo: (() ↓throws -> Void)? = {}"),
+        Example("let foo: @Sendable () ↓throws -> Void = {}"),
+        Example("func foo(bar: () throws -> Void) ↓rethrows {}"),
+        Example("init() ↓throws {}"),
+        Example("""
+            func foo() ↓throws {
+                bar()
+            }
+            """),
+        Example("""
+            func foo() ↓throws(Example) {
+                bar()
+            }
+            """),
+        Example("""
+            func foo() {
+                func bar() ↓throws {}
+                bar()
+            }
+            """),
+        Example("""
+            func foo() {
+                func bar() ↓throws {
+                    baz()
+                }
+                bar()
+            }
+            """),
+        Example("""
+            func foo() {
+                func bar() ↓throws {
+                    baz()
+                }
+                try? bar()
+            }
+            """),
+        Example("""
+            func foo() ↓throws {
+                func bar() ↓throws {
+                    baz()
+                }
+            }
+            """),
+        Example("""
+            func foo() ↓throws {
+                do { try bar() }
+                catch {}
+            }
+            """),
+        Example("""
+            func foo() ↓throws {
+                do {}
+                catch {}
+            }
+            """),
+        Example("""
+            func foo() ↓throws(Example) {
+                do {}
+                catch {}
+            }
+            """),
+        Example("""
+            func foo() {
+                do {
+                    func bar() ↓throws {}
+                    try bar()
+                } catch {}
+            }
+            """),
+        Example("""
+            func foo() ↓throws {
+                do {
+                    try bar()
+                    func baz() throws { try bar() }
+                    try baz()
+                } catch {}
+            }
+            """),
+        Example("""
+            func foo() ↓throws {
+                do {
+                    try bar()
+                } catch {
+                    do {
+                        throw Example.failure
+                    } catch {}
+                }
+            }
+            """),
+        Example("""
+            func foo() ↓throws {
+                do {
+                    try bar()
+                } catch {
+                    do {
+                        try bar()
+                        func baz() ↓throws {}
+                        try baz()
+                    } catch {}
+                }
+            }
+            """),
+        Example("""
+            func foo() ↓throws {
+                switch bar {
+                case 1: break
+                default: break
+                }
+            }
+            """),
+        Example("""
+            func foo() ↓throws {
+                _ = try? bar()
+            }
+            """),
+        Example("""
+            func foo() ↓throws {
+                Task {
+                    try bar()
+                }
+            }
+            """),
+        Example("""
+            func foo() throws {
+                try bar()
+                Task {
+                    func baz() ↓throws {}
+                }
+            }
+            """),
+        Example("""
+            var foo: Int {
+                get ↓throws {
+                    0
+                }
+            }
+            """),
+        Example("""
+        func foo() ↓throws {
+            do { try bar() }
+            catch Example.failure {}
+            catch is SomeError {}
+            catch {}
+        }
+        """),
+        Example("""
+        func foo() ↓throws {
+            bar(1) {
+                try baz()
+            }
+        }
+        """),
+        Example("let x = { () ↓throws -> Void in baz() }"),
+    ]
+
+    static let corrections = [
+        Example("func foo() ↓throws {}"): Example("func foo() {}"),
+        Example("init() ↓throws {}"): Example("init() {}"),
+        Example("""
+        func foo() {
+            func bar() ↓throws {}
+            bar()
+        }
+        """): Example("""
+            func foo() {
+                func bar() {}
+                bar()
+            }
+            """),
+        Example("""
+            var foo: Int {
+                get ↓throws {
+                    0
+                }
+            }
+            """): Example("""
+            var foo: Int {
+                get {
+                    0
+                }
+            }
+            """),
+        Example("""
+            var foo: Int {
+                get ↓throws(Example) {
+                    0
+                }
+            }
+            """): Example("""
+            var foo: Int {
+                get {
+                    0
+                }
+            }
+            """),
+        Example("""
+            let foo: () ↓throws -> Void = {}
+            """): Example("""
+            let foo: () -> Void = {}
+            """),
+        Example("""
+            let foo: () ↓throws(Example) -> Void = {}
+            """): Example("""
+            let foo: () -> Void = {}
+            """),
+        Example("""
+            func foo() ↓throws {
+                do {}
+                catch {}
+            }
+            """): Example("""
+            func foo() {
+                do {}
+                catch {}
+            }
+            """),
+        Example("""
+            func foo() ↓throws(Example) {
+                do {}
+                catch {}
+            }
+            """): Example("""
+            func foo() {
+                do {}
+                catch {}
+            }
+            """),
+        Example("func f() ↓throws /* comment */ {}"): Example("func f() /* comment */ {}"),
+        Example("func f() /* comment */ ↓throws /* comment */ {}"): Example("func f() /* comment */ /* comment */ {}"),
+        Example("let foo: @Sendable () ↓throws -> Void = {}"): Example("let foo: @Sendable () -> Void = {}"),
+    ]
+}

Tests/GeneratedTests/GeneratedTests_09.swift

@@ -139,6 +139,12 @@ final class UnneededSynthesizedInitializerRuleGeneratedTests: SwiftLintTestCase
     }
 }
 
+final class UnneededThrowsRuleGeneratedTests: SwiftLintTestCase {
+    func testWithDefaultConfiguration() {
+        verifyRule(UnneededThrowsRule.description)
+    }
+}
+
 final class UnownedVariableCaptureRuleGeneratedTests: SwiftLintTestCase {
     func testWithDefaultConfiguration() {
         verifyRule(UnownedVariableCaptureRule.description)
@@ -150,9 +156,3 @@ final class UntypedErrorInCatchRuleGeneratedTests: SwiftLintTestCase {
         verifyRule(UntypedErrorInCatchRule.description)
     }
 }
-
-final class UnusedClosureParameterRuleGeneratedTests: SwiftLintTestCase {
-    func testWithDefaultConfiguration() {
-        verifyRule(UnusedClosureParameterRule.description)
-    }
-}

Tests/GeneratedTests/GeneratedTests_10.swift

@@ -7,6 +7,12 @@
 @testable import SwiftLintCore
 import TestHelpers
 
+final class UnusedClosureParameterRuleGeneratedTests: SwiftLintTestCase {
+    func testWithDefaultConfiguration() {
+        verifyRule(UnusedClosureParameterRule.description)
+    }
+}
+
 final class UnusedControlFlowLabelRuleGeneratedTests: SwiftLintTestCase {
     func testWithDefaultConfiguration() {
         verifyRule(UnusedControlFlowLabelRule.description)

Tests/IntegrationTests/default_rule_configurations.yml

@@ -1287,6 +1287,11 @@ unneeded_synthesized_initializer:
   meta:
     opt-in: false
     correctable: true
+unneeded_throws_rethrows:
+  severity: warning
+  meta:
+    opt-in: true
+    correctable: true
 unowned_variable_capture:
   severity: warning
   meta: