From 0a9ef40677b9bbcc1e666e1d504a4e67c54f6e4a Mon Sep 17 00:00:00 2001 From: tonyskansf Date: Thu, 24 Apr 2025 16:11:21 +0200 Subject: [PATCH 01/19] Add opt-in unneeded_throws_rethrows rule --- .swiftlint.yml | 1 + CHANGELOG.md | 3 + .../Models/BuiltInRules.swift | 1 + .../Rules/Lint/UnneededThrowsRule.swift | 165 +++++++++++ .../Lint/UnneededThrowsRuleExamples.swift | 258 ++++++++++++++++++ .../default_rule_configurations.yml | 5 + 6 files changed, 433 insertions(+) create mode 100644 Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift create mode 100644 Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift diff --git a/.swiftlint.yml b/.swiftlint.yml index 5c349312a8..4201a9a70f 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -47,6 +47,7 @@ disabled_rules: - todo - trailing_closure - type_contents_order + - unneeded_throws_rethrows - vertical_whitespace_between_cases # Configurations diff --git a/CHANGELOG.md b/CHANGELOG.md index 694500781c..2961e214da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,9 @@ * Support deinitializers and subscripts in `function_body_length` rule. [SimplyDanny](https://github.com/SimplyDanny) +* Add opt-in `unneeded_throws_rethrows` rules that triggers when declarations + marked `throws`/`rethrows` never actually throw or call any throwing code. + [Tony Ngo](https://github.com/tonyskansf) ### Bug Fixes diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index bea8255b9c..c759c75998 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -222,6 +222,7 @@ public let builtInRules: [any Rule.Type] = [ UnneededOverrideRule.self, UnneededParenthesesInClosureArgumentRule.self, UnneededSynthesizedInitializerRule.self, + UnneededThrowsRule.self, UnownedVariableCaptureRule.self, UntypedErrorInCatchRule.self, UnusedClosureParameterRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift new file mode 100644 index 0000000000..c103d4c61d --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -0,0 +1,165 @@ +import SwiftLintCore +import SwiftSyntax + +@SwiftSyntaxRule(correctable: true, optIn: true) +struct UnneededThrowsRule: Rule { + var configuration = SeverityConfiguration(.warning) + + static let description = RuleDescription( + identifier: "unneeded_throws_rethrows", + name: "Unneeded (re)throws keyword", + description: "Non-throwing functions/variables should not me marked as `throws` or `rethrows`", + kind: .lint, + nonTriggeringExamples: UnneededThrowsRuleExamples.nonTriggeringExamples, + triggeringExamples: UnneededThrowsRuleExamples.triggeringExamples, + corrections: UnneededThrowsRuleExamples.corrections + ) +} + +private extension UnneededThrowsRule { + typealias Scope = [ThrowsClauseSyntax] + final class Visitor: ViolationsSyntaxVisitor { + private var scopes = Stack() + + override var skippableDeclarations: [any DeclSyntaxProtocol.Type] { [ + ProtocolDeclSyntax.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() { + validateScope( + closedScope, + reason: "The initializer does not throw any error" + ) + } + } + + override func visit(_ node: AccessorDeclSyntax) -> SyntaxVisitorContinueKind { + scopes.openScope(with: node.effectSpecifiers?.throwsClause) + return .visitChildren + } + + override func visitPost(_: AccessorDeclSyntax) { + if let closedScope = scopes.closeScope() { + validateScope( + closedScope, + reason: "The accessor does not throw any error" + ) + } + } + + override func visitPost(_: VariableDeclSyntax) { + if let closedScope = scopes.closeScope() { + validateScope( + closedScope, + reason: "The variable does not throw any error" + ) + } + } + + override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind { + scopes.openScope(with: node.signature.effectSpecifiers?.throwsClause) + return .visitChildren + } + + override func visitPost(_: FunctionDeclSyntax) { + if let closedScope = scopes.closeScope() { + validateScope( + closedScope, + reason: "The body of this function does not throw any error" + ) + } + } + + override func visit(_ node: FunctionTypeSyntax) -> SyntaxVisitorContinueKind { + scopes.openScope(with: node.effectSpecifiers?.throwsClause) + return .visitChildren + } + + override func visitPost(_: FunctionTypeSyntax) { + if let closedScope = scopes.closeScope() { + validateScope( + closedScope, + reason: "The closure type does not throw any error" + ) + } + } + + override func visit(_: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind { + scopes.openScope() + return .visitChildren + } + + override func visitPost(_: FunctionCallExprSyntax) { + 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: TryExprSyntax) { + if node.questionOrExclamationMark == nil { + scopes.markCurrentScopeAsThrowing() + } + } + + override func visitPost(_: ThrowStmtSyntax) { + scopes.markCurrentScopeAsThrowing() + } + + private func validateScope(_ scope: Scope, reason: String) { + guard let throwsToken = scope.last?.throwsSpecifier else { return } + violations.append( + ReasonedRuleViolation( + position: throwsToken.positionAfterSkippingLeadingTrivia, + reason: reason, + correction: ReasonedRuleViolation.ViolationCorrection( + start: throwsToken.positionAfterSkippingLeadingTrivia, + end: throwsToken.endPosition, + replacement: "" + ) + ) + ) + } + } +} + +private extension Stack where Element == UnneededThrowsRule.Scope { + mutating func markCurrentScopeAsThrowing() { + modifyLast { currentScope in + _ = currentScope.popLast() + } + } + + mutating func openScope() { + push([]) + } + + mutating func openScope(with throwsClause: ThrowsClauseSyntax?) { + if let throwsClause { + push([throwsClause]) + } + } + + @discardableResult + mutating func closeScope() -> [ThrowsClauseSyntax]? { + pop() + } +} diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift new file mode 100644 index 0000000000..feab00ad0a --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -0,0 +1,258 @@ +internal struct UnneededThrowsRuleExamples { + static let nonTriggeringExamples = [ + Example(""" + func foo() throws { + try bar() + } + """), + Example(""" + func foo() throws { + 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 + } + } + """), + ] + + static let triggeringExamples = [ + Example("func foo() ↓throws {}"), + Example("let foo: () ↓throws -> Void = {}"), + Example("let foo: (() ↓throws -> Void)? = {}"), + Example("func foo(bar: () throws -> Void) ↓rethrows {}"), + Example("init() ↓throws {}"), + Example(""" + func foo() ↓throws { + 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() { + 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 + } + } + """), + ] + + 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(""" + let foo: () ↓throws -> Void = {} + """): Example(""" + let foo: () -> Void = {} + """), + Example(""" + func foo() ↓throws { + do {} + catch {} + } + """): Example(""" + func foo() { + do {} + catch {} + } + """), + ] +} diff --git a/Tests/IntegrationTests/default_rule_configurations.yml b/Tests/IntegrationTests/default_rule_configurations.yml index aaeffb97b6..fa73f30e2c 100644 --- a/Tests/IntegrationTests/default_rule_configurations.yml +++ b/Tests/IntegrationTests/default_rule_configurations.yml @@ -1270,6 +1270,11 @@ unneeded_synthesized_initializer: meta: opt-in: false correctable: true +unneeded_throws: + severity: warning + meta: + opt-in: true + correctable: false unowned_variable_capture: severity: warning meta: From 73c3a9e10eb26768a3660cb187454c2a1d87467b Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Mon, 28 Apr 2025 11:03:14 +0200 Subject: [PATCH 02/19] Resolve testSwiftLintAutoCorrects integration test fail --- .../Rules/Lint/UnneededThrowsRule.swift | 1 + .../Rules/Lint/UnneededThrowsRuleExamples.swift | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index c103d4c61d..4cb02b53da 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -18,6 +18,7 @@ struct UnneededThrowsRule: Rule { private extension UnneededThrowsRule { typealias Scope = [ThrowsClauseSyntax] + final class Visitor: ViolationsSyntaxVisitor { private var scopes = Stack() diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index feab00ad0a..9dc2fe4e78 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -37,7 +37,7 @@ internal struct UnneededThrowsRuleExamples { """), Example(""" func foo() throws { - do { try bar() } + do { try bar() } catch { throw Example.failure } @@ -45,7 +45,7 @@ internal struct UnneededThrowsRuleExamples { """), Example(""" func foo() throws { - do { try bar() } + do { try bar() } catch { try baz() } @@ -123,13 +123,13 @@ internal struct UnneededThrowsRuleExamples { """), Example(""" func foo() ↓throws { - do { try bar() } + do { try bar() } catch {} } """), Example(""" func foo() ↓throws { - do {} + do {} catch {} } """), From e366373c8bb1e324091d7340bad71de9122a5f15 Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Mon, 28 Apr 2025 11:12:48 +0200 Subject: [PATCH 03/19] Resolve false positives --- .../Rules/Lint/UnneededThrowsRule.swift | 50 +++++++++++++------ .../Lint/UnneededThrowsRuleExamples.swift | 28 +++++++++++ 2 files changed, 62 insertions(+), 16 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index 4cb02b53da..bb9e4a103a 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -22,9 +22,17 @@ private extension UnneededThrowsRule { final class Visitor: ViolationsSyntaxVisitor { private var scopes = Stack() - override var skippableDeclarations: [any DeclSyntaxProtocol.Type] { [ - ProtocolDeclSyntax.self, - ] } + override func visit(_ node: ProtocolDeclSyntax) -> SyntaxVisitorContinueKind { + .skipChildren + } + + override func visit(_ node: TypeAliasDeclSyntax) -> SyntaxVisitorContinueKind { + .skipChildren + } + + override func visit(_ node: EnumCaseDeclSyntax) -> SyntaxVisitorContinueKind { + .skipChildren + } override func visit(_: FunctionParameterClauseSyntax) -> SyntaxVisitorContinueKind { .skipChildren @@ -58,15 +66,6 @@ private extension UnneededThrowsRule { } } - override func visitPost(_: VariableDeclSyntax) { - if let closedScope = scopes.closeScope() { - validateScope( - closedScope, - reason: "The variable does not throw any error" - ) - } - } - override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind { scopes.openScope(with: node.signature.effectSpecifiers?.throwsClause) return .visitChildren @@ -95,13 +94,17 @@ private extension UnneededThrowsRule { } } - override func visit(_: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind { - scopes.openScope() + override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind { + if node.containsTaskDeclaration { + scopes.openScope() + } return .visitChildren } - override func visitPost(_: FunctionCallExprSyntax) { - scopes.closeScope() + override func visitPost(_ node: FunctionCallExprSyntax) { + if node.containsTaskDeclaration { + scopes.closeScope() + } } override func visit(_: DoStmtSyntax) -> SyntaxVisitorContinueKind { @@ -115,6 +118,13 @@ private extension UnneededThrowsRule { } } + override func visit(_ node: ForStmtSyntax) -> SyntaxVisitorContinueKind { + if node.tryKeyword != nil { + scopes.markCurrentScopeAsThrowing() + } + return .visitChildren + } + override func visitPost(_ node: TryExprSyntax) { if node.questionOrExclamationMark == nil { scopes.markCurrentScopeAsThrowing() @@ -164,3 +174,11 @@ private extension Stack where Element == UnneededThrowsRule.Scope { pop() } } + +private extension FunctionCallExprSyntax { + var containsTaskDeclaration: Bool { + children(viewMode: .sourceAccurate).contains { child in + child.as(DeclReferenceExprSyntax.self)?.baseName.tokenKind == .identifier("Task") + } + } +} diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index 9dc2fe4e78..2011c73f6d 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -79,6 +79,34 @@ internal struct UnneededThrowsRuleExamples { } } """), + 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 rethrows { + for try await item in items {} + } + """), ] static let triggeringExamples = [ From 3d77f3a566e605d132f8f260a3dd5740f68ba455 Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Mon, 28 Apr 2025 14:15:00 +0200 Subject: [PATCH 04/19] Resolve testSwiftLintAutoCorrects integration test fail --- .../Rules/Lint/UnneededThrowsRule.swift | 6 +++--- .../Rules/Lint/UnneededThrowsRuleExamples.swift | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index bb9e4a103a..d1b9d46939 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -22,15 +22,15 @@ private extension UnneededThrowsRule { final class Visitor: ViolationsSyntaxVisitor { private var scopes = Stack() - override func visit(_ node: ProtocolDeclSyntax) -> SyntaxVisitorContinueKind { + override func visit(_: ProtocolDeclSyntax) -> SyntaxVisitorContinueKind { .skipChildren } - override func visit(_ node: TypeAliasDeclSyntax) -> SyntaxVisitorContinueKind { + override func visit(_: TypeAliasDeclSyntax) -> SyntaxVisitorContinueKind { .skipChildren } - override func visit(_ node: EnumCaseDeclSyntax) -> SyntaxVisitorContinueKind { + override func visit(_: EnumCaseDeclSyntax) -> SyntaxVisitorContinueKind { .skipChildren } diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index 2011c73f6d..baf09e6e57 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -82,7 +82,7 @@ internal struct UnneededThrowsRuleExamples { Example(""" func foo() throws{ let bar = Bar() - + if bar.boolean { throw Example.failure } From 04bf492a18663c8158cc12f7b72908a1173a34fc Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Mon, 28 Apr 2025 15:52:17 +0200 Subject: [PATCH 05/19] Resolve unhandled cases --- .../Rules/Lint/UnneededThrowsRule.swift | 29 +++++++++++++++++-- .../Lint/UnneededThrowsRuleExamples.swift | 12 ++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index d1b9d46939..be9e1fec9c 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -80,8 +80,10 @@ private extension UnneededThrowsRule { } } - override func visit(_ node: FunctionTypeSyntax) -> SyntaxVisitorContinueKind { - scopes.openScope(with: node.effectSpecifiers?.throwsClause) + override func visit(_ node: PatternBindingSyntax) -> SyntaxVisitorContinueKind { + if node.containsInitializerClause, let functionTypeSyntax = node.functionTypeSyntax { + scopes.openScope(with: functionTypeSyntax.effectSpecifiers?.throwsClause) + } return .visitChildren } @@ -182,3 +184,26 @@ private extension FunctionCallExprSyntax { } } } + +private extension PatternBindingSyntax { + var containsInitializerClause: Bool { + children(viewMode: .sourceAccurate).contains { child in + child.is(InitializerClauseSyntax.self) + } + } + + var functionTypeSyntax: FunctionTypeSyntax? { + guard let typeAnnotation else { return nil } + + var children = Set(typeAnnotation.children(viewMode: .sourceAccurate)) + + while let child = children.popFirst() { + if let functionType = child.as(FunctionTypeSyntax.self) { + return functionType + } + children.formUnion(child.children(viewMode: .sourceAccurate)) + } + + return nil + } +} diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index baf09e6e57..2ee9906441 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -107,6 +107,18 @@ internal struct UnneededThrowsRuleExamples { 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 = {}) {} + """), ] static let triggeringExamples = [ From 556be9348ef07bf4f5087484b3b1ca9ca48ea281 Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Sun, 4 May 2025 16:36:53 +0200 Subject: [PATCH 06/19] Handle premature closing scope --- .../Rules/Lint/UnneededThrowsRule.swift | 20 ++++++++----------- .../Lint/UnneededThrowsRuleExamples.swift | 9 +++++++++ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index be9e1fec9c..fffed8f09a 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -17,7 +17,9 @@ struct UnneededThrowsRule: Rule { } private extension UnneededThrowsRule { - typealias Scope = [ThrowsClauseSyntax] + struct Scope { + var throwsClause: ThrowsClauseSyntax? + } final class Visitor: ViolationsSyntaxVisitor { private var scopes = Stack() @@ -138,7 +140,7 @@ private extension UnneededThrowsRule { } private func validateScope(_ scope: Scope, reason: String) { - guard let throwsToken = scope.last?.throwsSpecifier else { return } + guard let throwsToken = scope.throwsClause?.throwsSpecifier else { return } violations.append( ReasonedRuleViolation( position: throwsToken.positionAfterSkippingLeadingTrivia, @@ -157,22 +159,16 @@ private extension UnneededThrowsRule { private extension Stack where Element == UnneededThrowsRule.Scope { mutating func markCurrentScopeAsThrowing() { modifyLast { currentScope in - _ = currentScope.popLast() + currentScope.throwsClause = nil } } - mutating func openScope() { - push([]) - } - - mutating func openScope(with throwsClause: ThrowsClauseSyntax?) { - if let throwsClause { - push([throwsClause]) - } + mutating func openScope(with throwsClause: ThrowsClauseSyntax? = nil) { + push(UnneededThrowsRule.Scope(throwsClause: throwsClause)) } @discardableResult - mutating func closeScope() -> [ThrowsClauseSyntax]? { + mutating func closeScope() -> Element? { pop() } } diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index 2ee9906441..da70656fe3 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -119,6 +119,15 @@ internal struct UnneededThrowsRuleExamples { Example(""" func foo(_ bar: () throws -> Void = {}) {} """), + Example(""" + func foo() async throws { + func foo() {} + for _ in 0.. Date: Sun, 4 May 2025 21:47:48 +0200 Subject: [PATCH 07/19] Handle more do-catch edge cases --- .../Rules/Lint/UnneededThrowsRule.swift | 9 ++++++++ .../Lint/UnneededThrowsRuleExamples.swift | 21 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index fffed8f09a..b007884ae3 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -122,6 +122,15 @@ private extension UnneededThrowsRule { } } + override func visitPost(_ node: DoStmtSyntax) { + let doesNotContainCatchClauseWithoutPattern = !node.catchClauses.contains { catchClause in + catchClause.catchItems.isEmpty + } + if doesNotContainCatchClauseWithoutPattern { + scopes.markCurrentScopeAsThrowing() + } + } + override func visit(_ node: ForStmtSyntax) -> SyntaxVisitorContinueKind { if node.tryKeyword != nil { scopes.markCurrentScopeAsThrowing() diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index da70656fe3..9fec809734 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -128,6 +128,19 @@ internal struct UnneededThrowsRuleExamples { } } """), + 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 {} + } + """) ] static let triggeringExamples = [ @@ -258,6 +271,14 @@ internal struct UnneededThrowsRuleExamples { } } """), + Example(""" + func foo() ↓throws { + do { try bar() } + catch Example.failure {} + catch is SomeError {} + catch {} + } + """), ] static let corrections = [ From bb93bfe728a5b5d4a3c23caca257330ffd5cd6ed Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Thu, 8 May 2025 09:54:30 +0200 Subject: [PATCH 08/19] Correct typo --- .../SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index b007884ae3..310b6b7726 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -8,7 +8,7 @@ struct UnneededThrowsRule: Rule { static let description = RuleDescription( identifier: "unneeded_throws_rethrows", name: "Unneeded (re)throws keyword", - description: "Non-throwing functions/variables should not me marked as `throws` or `rethrows`", + description: "Non-throwing functions/variables should not be marked as `throws` or `rethrows`", kind: .lint, nonTriggeringExamples: UnneededThrowsRuleExamples.nonTriggeringExamples, triggeringExamples: UnneededThrowsRuleExamples.triggeringExamples, From 9e45610b9cf2cb87ad0ff3988111d163c488d2c6 Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Tue, 22 Jul 2025 10:12:40 +0200 Subject: [PATCH 09/19] Run swift run swiftlint-dev rules register --- Tests/GeneratedTests/GeneratedTests_09.swift | 12 ++++++------ Tests/GeneratedTests/GeneratedTests_10.swift | 6 ++++++ .../IntegrationTests/default_rule_configurations.yml | 4 ++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/Tests/GeneratedTests/GeneratedTests_09.swift b/Tests/GeneratedTests/GeneratedTests_09.swift index 1d8dc7614d..19cd5dc595 100644 --- a/Tests/GeneratedTests/GeneratedTests_09.swift +++ b/Tests/GeneratedTests/GeneratedTests_09.swift @@ -127,6 +127,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 UnusedControlFlowLabelRuleGeneratedTests: SwiftLintTestCase { verifyRule(UnusedControlFlowLabelRule.description) } } - -final class UnusedDeclarationRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(UnusedDeclarationRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_10.swift b/Tests/GeneratedTests/GeneratedTests_10.swift index a5ce334fc5..648dde1162 100644 --- a/Tests/GeneratedTests/GeneratedTests_10.swift +++ b/Tests/GeneratedTests/GeneratedTests_10.swift @@ -7,6 +7,12 @@ @testable import SwiftLintCore import TestHelpers +final class UnusedDeclarationRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(UnusedDeclarationRule.description) + } +} + final class UnusedEnumeratedRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(UnusedEnumeratedRule.description) diff --git a/Tests/IntegrationTests/default_rule_configurations.yml b/Tests/IntegrationTests/default_rule_configurations.yml index fa73f30e2c..8f869b3a7a 100644 --- a/Tests/IntegrationTests/default_rule_configurations.yml +++ b/Tests/IntegrationTests/default_rule_configurations.yml @@ -1270,11 +1270,11 @@ unneeded_synthesized_initializer: meta: opt-in: false correctable: true -unneeded_throws: +unneeded_throws_rethrows: severity: warning meta: opt-in: true - correctable: false + correctable: true unowned_variable_capture: severity: warning meta: From 5898e7a86708b502e9c834725e92f1b052fd78e5 Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Tue, 22 Jul 2025 10:59:50 +0200 Subject: [PATCH 10/19] Address batch of comments --- CHANGELOG.md | 2 +- .../Rules/Lint/UnneededThrowsRule.swift | 62 ++++++++----------- .../Lint/UnneededThrowsRuleExamples.swift | 4 +- 3 files changed, 30 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2961e214da..d0cfb0297b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,7 +83,7 @@ * Support deinitializers and subscripts in `function_body_length` rule. [SimplyDanny](https://github.com/SimplyDanny) * Add opt-in `unneeded_throws_rethrows` rules that triggers when declarations - marked `throws`/`rethrows` never actually throw or call any throwing code. + marked `throws`/`rethrows` never actually throw or call any throwing code. [Tony Ngo](https://github.com/tonyskansf) ### Bug Fixes diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index 310b6b7726..ea4bc13efb 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -8,7 +8,7 @@ struct UnneededThrowsRule: Rule { static let description = RuleDescription( identifier: "unneeded_throws_rethrows", name: "Unneeded (re)throws keyword", - description: "Non-throwing functions/variables should not be marked as `throws` or `rethrows`", + description: "Non-throwing functions/properties/closures should not be marked as `throws` or `rethrows`.", kind: .lint, nonTriggeringExamples: UnneededThrowsRuleExamples.nonTriggeringExamples, triggeringExamples: UnneededThrowsRuleExamples.triggeringExamples, @@ -24,16 +24,12 @@ private extension UnneededThrowsRule { final class Visitor: ViolationsSyntaxVisitor { private var scopes = Stack() - override func visit(_: ProtocolDeclSyntax) -> SyntaxVisitorContinueKind { - .skipChildren - } - - override func visit(_: TypeAliasDeclSyntax) -> SyntaxVisitorContinueKind { - .skipChildren - } - - override func visit(_: EnumCaseDeclSyntax) -> SyntaxVisitorContinueKind { - .skipChildren + override var skippableDeclarations: [any DeclSyntaxProtocol.Type] { + [ + ProtocolDeclSyntax.self, + TypeAliasDeclSyntax.self, + EnumCaseDeclSyntax.self, + ] } override func visit(_: FunctionParameterClauseSyntax) -> SyntaxVisitorContinueKind { @@ -47,9 +43,9 @@ private extension UnneededThrowsRule { override func visitPost(_: InitializerDeclSyntax) { if let closedScope = scopes.closeScope() { - validateScope( - closedScope, - reason: "The initializer does not throw any error" + validate( + scope: closedScope, + reason: "initializer does not throw any error" ) } } @@ -61,9 +57,9 @@ private extension UnneededThrowsRule { override func visitPost(_: AccessorDeclSyntax) { if let closedScope = scopes.closeScope() { - validateScope( - closedScope, - reason: "The accessor does not throw any error" + validate( + scope: closedScope, + reason: "accessor does not throw any error" ) } } @@ -75,9 +71,9 @@ private extension UnneededThrowsRule { override func visitPost(_: FunctionDeclSyntax) { if let closedScope = scopes.closeScope() { - validateScope( - closedScope, - reason: "The body of this function does not throw any error" + validate( + scope: closedScope, + reason: "body of this function does not throw any error" ) } } @@ -91,9 +87,9 @@ private extension UnneededThrowsRule { override func visitPost(_: FunctionTypeSyntax) { if let closedScope = scopes.closeScope() { - validateScope( - closedScope, - reason: "The closure type does not throw any error" + validate( + scope: closedScope, + reason: "closure type does not throw any error" ) } } @@ -123,19 +119,17 @@ private extension UnneededThrowsRule { } override func visitPost(_ node: DoStmtSyntax) { - let doesNotContainCatchClauseWithoutPattern = !node.catchClauses.contains { catchClause in - catchClause.catchItems.isEmpty - } - if doesNotContainCatchClauseWithoutPattern { - scopes.markCurrentScopeAsThrowing() + if node.catchClauses.contains(where: { $0.catchItems.isEmpty }) { + // All errors will be caught. + return } + scopes.markCurrentScopeAsThrowing() } - override func visit(_ node: ForStmtSyntax) -> SyntaxVisitorContinueKind { + override func visitPost(_ node: ForStmtSyntax) { if node.tryKeyword != nil { scopes.markCurrentScopeAsThrowing() } - return .visitChildren } override func visitPost(_ node: TryExprSyntax) { @@ -148,12 +142,12 @@ private extension UnneededThrowsRule { scopes.markCurrentScopeAsThrowing() } - private func validateScope(_ scope: Scope, reason: String) { + private func validate(scope: Scope, reason: String) { guard let throwsToken = scope.throwsClause?.throwsSpecifier else { return } violations.append( ReasonedRuleViolation( position: throwsToken.positionAfterSkippingLeadingTrivia, - reason: reason, + reason: "Superfluous 'throws'; " + reason, correction: ReasonedRuleViolation.ViolationCorrection( start: throwsToken.positionAfterSkippingLeadingTrivia, end: throwsToken.endPosition, @@ -192,9 +186,7 @@ private extension FunctionCallExprSyntax { private extension PatternBindingSyntax { var containsInitializerClause: Bool { - children(viewMode: .sourceAccurate).contains { child in - child.is(InitializerClauseSyntax.self) - } + initializer != nil } var functionTypeSyntax: FunctionTypeSyntax? { diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index 9fec809734..3fecda61ef 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -80,7 +80,7 @@ internal struct UnneededThrowsRuleExamples { } """), Example(""" - func foo() throws{ + func foo() throws { let bar = Bar() if bar.boolean { @@ -103,7 +103,7 @@ internal struct UnneededThrowsRuleExamples { } """), Example(""" - func foo() async rethrows { + func foo() async throws { for try await item in items {} } """), From 25c4905f420cb4742790ccecfe92da689a838c74 Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Tue, 22 Jul 2025 16:04:18 +0200 Subject: [PATCH 11/19] Recursively check type annotation instead of iterating over children --- .../Rules/Lint/UnneededThrowsRule.swift | 28 ++++++++++++------- .../Lint/UnneededThrowsRuleExamples.swift | 3 +- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index ea4bc13efb..7d613be8f8 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -190,17 +190,25 @@ private extension PatternBindingSyntax { } var functionTypeSyntax: FunctionTypeSyntax? { - guard let typeAnnotation else { return nil } - - var children = Set(typeAnnotation.children(viewMode: .sourceAccurate)) + typeAnnotation?.type.baseFunctionTypeSyntax + } +} - while let child = children.popFirst() { - if let functionType = child.as(FunctionTypeSyntax.self) { - return functionType - } - children.formUnion(child.children(viewMode: .sourceAccurate)) +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): + tuple.elements + .compactMap { $0.type.baseFunctionTypeSyntax } + .first + default: + nil } - - return nil } } diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index 3fecda61ef..6b13bfad9e 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -140,7 +140,8 @@ internal struct UnneededThrowsRuleExamples { catch is SomeError { throw AnotherError } catch is AnotherError {} } - """) + """), + Example("let s: S<() throws -> Void> = S()"), ] static let triggeringExamples = [ From ff94069211472e0595eb7721cfefe25e319d78bd Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Tue, 22 Jul 2025 16:24:35 +0200 Subject: [PATCH 12/19] Correct typed throws correction, add examples --- .../Rules/Lint/UnneededThrowsRule.swift | 8 ++-- .../Lint/UnneededThrowsRuleExamples.swift | 45 +++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index 7d613be8f8..36e1db9f30 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -143,14 +143,14 @@ private extension UnneededThrowsRule { } private func validate(scope: Scope, reason: String) { - guard let throwsToken = scope.throwsClause?.throwsSpecifier else { return } + guard let throwsClause = scope.throwsClause else { return } violations.append( ReasonedRuleViolation( - position: throwsToken.positionAfterSkippingLeadingTrivia, + position: throwsClause.positionAfterSkippingLeadingTrivia, reason: "Superfluous 'throws'; " + reason, correction: ReasonedRuleViolation.ViolationCorrection( - start: throwsToken.positionAfterSkippingLeadingTrivia, - end: throwsToken.endPosition, + start: throwsClause.positionAfterSkippingLeadingTrivia, + end: throwsClause.endPosition, replacement: "" ) ) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index 6b13bfad9e..82849dc224 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -10,6 +10,11 @@ internal struct UnneededThrowsRuleExamples { throw Example.failure } """), + Example(""" + func foo() throws(Example) { + throw Example.failure + } + """), Example(""" func foo(_ bar: () throws -> T) rethrows -> Int { try items.map { try bar() } @@ -155,6 +160,11 @@ internal struct UnneededThrowsRuleExamples { bar() } """), + Example(""" + func foo() ↓throws(Example) { + bar() + } + """), Example(""" func foo() { func bar() ↓throws {} @@ -196,6 +206,12 @@ internal struct UnneededThrowsRuleExamples { catch {} } """), + Example(""" + func foo() ↓throws(Example) { + do {} + catch {} + } + """), Example(""" func foo() { do { @@ -309,11 +325,29 @@ internal struct UnneededThrowsRuleExamples { } } """), + 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 {} @@ -325,5 +359,16 @@ internal struct UnneededThrowsRuleExamples { catch {} } """), + Example(""" + func foo() ↓throws(Example) { + do {} + catch {} + } + """): Example(""" + func foo() { + do {} + catch {} + } + """), ] } From 3416302ad62dbd2517dfd8252524aba45f6dc87e Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Mon, 8 Sep 2025 20:48:49 +0200 Subject: [PATCH 13/19] Fix corrections with trailing trivia --- .../Rules/Lint/UnneededThrowsRule.swift | 5 +++-- .../Rules/Lint/UnneededThrowsRuleExamples.swift | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index 36e1db9f30..2ec0aa5bc5 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -149,8 +149,9 @@ private extension UnneededThrowsRule { position: throwsClause.positionAfterSkippingLeadingTrivia, reason: "Superfluous 'throws'; " + reason, correction: ReasonedRuleViolation.ViolationCorrection( - start: throwsClause.positionAfterSkippingLeadingTrivia, - end: throwsClause.endPosition, + // Move start position back by 1 to include the space before the throwsClause + start: throwsClause.positionAfterSkippingLeadingTrivia.advanced(by: -1), + end: throwsClause.endPositionBeforeTrailingTrivia, replacement: "" ) ) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index 82849dc224..b60a96496e 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -370,5 +370,7 @@ internal struct UnneededThrowsRuleExamples { catch {} } """), + Example("func f() ↓throws /* comment */ {}"): Example("func f() /* comment */ {}"), + Example("func f() /* comment */ ↓throws /* comment */ {}"): Example("func f() /* comment */ /* comment */ {}"), ] } From 2977a298ac89d24ebe056a9a0acea158a3136d8c Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Thu, 11 Sep 2025 09:49:19 +0200 Subject: [PATCH 14/19] Add newline --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0cfb0297b..c02e6e3ad0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,7 @@ * Support deinitializers and subscripts in `function_body_length` rule. [SimplyDanny](https://github.com/SimplyDanny) + * Add opt-in `unneeded_throws_rethrows` rules that triggers when declarations marked `throws`/`rethrows` never actually throw or call any throwing code. [Tony Ngo](https://github.com/tonyskansf) From c5cd3db6e40d7db23d778dcd9e528c16f86b14af Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Thu, 11 Sep 2025 09:53:24 +0200 Subject: [PATCH 15/19] Refactor validate method --- .../Rules/Lint/UnneededThrowsRule.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index 2ec0aa5bc5..1e770f1d74 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -45,7 +45,7 @@ private extension UnneededThrowsRule { if let closedScope = scopes.closeScope() { validate( scope: closedScope, - reason: "initializer does not throw any error" + construct: "initializer" ) } } @@ -59,7 +59,7 @@ private extension UnneededThrowsRule { if let closedScope = scopes.closeScope() { validate( scope: closedScope, - reason: "accessor does not throw any error" + construct: "accessor" ) } } @@ -73,7 +73,7 @@ private extension UnneededThrowsRule { if let closedScope = scopes.closeScope() { validate( scope: closedScope, - reason: "body of this function does not throw any error" + construct: "body of this function" ) } } @@ -89,7 +89,7 @@ private extension UnneededThrowsRule { if let closedScope = scopes.closeScope() { validate( scope: closedScope, - reason: "closure type does not throw any error" + construct: "closure type" ) } } @@ -142,12 +142,12 @@ private extension UnneededThrowsRule { scopes.markCurrentScopeAsThrowing() } - private func validate(scope: Scope, reason: String) { + private func validate(scope: Scope, construct: String) { guard let throwsClause = scope.throwsClause else { return } violations.append( ReasonedRuleViolation( position: throwsClause.positionAfterSkippingLeadingTrivia, - reason: "Superfluous 'throws'; " + reason, + reason: "Superfluous 'throws'; \(construct) does not throw any error", correction: ReasonedRuleViolation.ViolationCorrection( // Move start position back by 1 to include the space before the throwsClause start: throwsClause.positionAfterSkippingLeadingTrivia.advanced(by: -1), From 84b708187ef210fd1075e76f601db33176d061e0 Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Thu, 11 Sep 2025 10:16:19 +0200 Subject: [PATCH 16/19] Only check single-element tuples for unneeded throws --- .../Rules/Lint/UnneededThrowsRule.swift | 9 ++++++--- .../Rules/Lint/UnneededThrowsRuleExamples.swift | 4 ++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index 1e770f1d74..9172e9f69b 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -205,9 +205,12 @@ private extension TypeSyntax { case .attributedType(let attributed): attributed.baseType.baseFunctionTypeSyntax case .tupleType(let tuple): - tuple.elements - .compactMap { $0.type.baseFunctionTypeSyntax } - .first + // 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 } diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index b60a96496e..e267b59e00 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -147,12 +147,16 @@ internal struct UnneededThrowsRuleExamples { } """), 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, {})"), ] static let triggeringExamples = [ Example("func foo() ↓throws {}"), 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(""" From 1f5411716b55e60e619b30f1dc42e8e3375af096 Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Thu, 11 Sep 2025 10:20:52 +0200 Subject: [PATCH 17/19] Add test case for single-element tuple type --- .../Rules/Lint/UnneededThrowsRuleExamples.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index e267b59e00..052559fcf5 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -155,6 +155,7 @@ internal struct UnneededThrowsRuleExamples { 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 {}"), From d3b4bf0538e270c2c0955b10752e2b7fce234c98 Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Thu, 11 Sep 2025 10:52:04 +0200 Subject: [PATCH 18/19] Remove trailing whitespace --- .../Rules/Lint/UnneededThrowsRuleExamples.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index 052559fcf5..d76ff1edb6 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -135,14 +135,14 @@ internal struct UnneededThrowsRuleExamples { """), Example(""" func foo() throws { - do { try bar() } + do { try bar() } catch Example.failure {} } """), Example(""" func foo() throws { - do { try bar() } - catch is SomeError { throw AnotherError } + do { try bar() } + catch is SomeError { throw AnotherError } catch is AnotherError {} } """), @@ -295,7 +295,7 @@ internal struct UnneededThrowsRuleExamples { """), Example(""" func foo() ↓throws { - do { try bar() } + do { try bar() } catch Example.failure {} catch is SomeError {} catch {} From 032538bc288bfd4cfc14fab1cd236b24c81f96d5 Mon Sep 17 00:00:00 2001 From: Tony Ngo Date: Thu, 11 Sep 2025 14:50:09 +0200 Subject: [PATCH 19/19] Fix false positive for throwing closures in variable declarations --- .../Rules/Lint/UnneededThrowsRule.swift | 14 ++++++++------ .../Rules/Lint/UnneededThrowsRuleExamples.swift | 1 + 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift index 9172e9f69b..d34cdc04e5 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRule.swift @@ -85,12 +85,14 @@ private extension UnneededThrowsRule { return .visitChildren } - override func visitPost(_: FunctionTypeSyntax) { - if let closedScope = scopes.closeScope() { - validate( - scope: closedScope, - construct: "closure type" - ) + override func visitPost(_ node: PatternBindingSyntax) { + if node.containsInitializerClause, node.functionTypeSyntax != nil { + if let closedScope = scopes.closeScope() { + validate( + scope: closedScope, + construct: "closure type" + ) + } } } diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift index d76ff1edb6..59fd6738da 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnneededThrowsRuleExamples.swift @@ -150,6 +150,7 @@ internal struct UnneededThrowsRuleExamples { 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() }"), ] static let triggeringExamples = [