From 587a92230479e72aa23028c029669f13e8441ff6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= Date: Fri, 20 Dec 2024 10:24:27 +0100 Subject: [PATCH] Add new `redundant_sendable` rule --- CHANGELOG.md | 5 + .../Models/BuiltInRules.swift | 1 + .../Rules/Lint/RedundantSendableRule.swift | 130 ++++++++++++++++++ .../RedundantSendableConfiguration.swift | 11 ++ Tests/GeneratedTests/GeneratedTests.swift | 6 + .../default_rule_configurations.yml | 3 + 6 files changed, 156 insertions(+) create mode 100644 Source/SwiftLintBuiltInRules/Rules/Lint/RedundantSendableRule.swift create mode 100644 Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/RedundantSendableConfiguration.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a283f0cfe..adfba4b187 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,11 @@ [dk-talks](https://github.com/dk-talks) [SimplyDanny](https://github.com/SimplyDanny) +* Add new `redundant_sendable` rule that triggers on `Sendable` conformances of + types that are implicitly already `Sendable` due to being actor-isolated. It + is enabled by default. + [SimplyDanny](https://github.com/SimplyDanny) + #### Bug Fixes * Ignore super calls with trailing closures in `unneeded_override` rule. diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index b93b94e752..c0403acbaa 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -176,6 +176,7 @@ public let builtInRules: [any Rule.Type] = [ RedundantObjcAttributeRule.self, RedundantOptionalInitializationRule.self, RedundantSelfInClosureRule.self, + RedundantSendableRule.self, RedundantSetAccessControlRule.self, RedundantStringEnumValueRule.self, RedundantTypeAnnotationRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/RedundantSendableRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/RedundantSendableRule.swift new file mode 100644 index 0000000000..d29cdc54eb --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/RedundantSendableRule.swift @@ -0,0 +1,130 @@ +import SwiftSyntax + +@SwiftSyntaxRule(explicitRewriter: true) +struct RedundantSendableRule: Rule { + var configuration = RedundantSendableConfiguration() + + static let description = RuleDescription( + identifier: "redundant_sendable", + name: "Redundant Sendable", + description: "Sendable conformance is redundant on an actor-isolated type", + kind: .lint, + nonTriggeringExamples: [ + Example("struct S: Sendable {}"), + Example("class C: Sendable {}"), + Example("actor A {}"), + Example("@MainActor struct S {}"), + Example("@MyActor enum E: Sendable { case a }"), + ], + triggeringExamples: [ + Example("@MainActor struct ↓S: Sendable {}"), + Example("actor ↓A: Sendable {}"), + Example("@MyActor enum ↓E: Sendable { case a }", configuration: ["global_actors": ["MyActor"]]), + ], + corrections: [ + Example("@MainActor struct S: Sendable {}"): + Example("@MainActor struct S {}"), + Example("actor A: Sendable {}"): + Example("actor A {}"), + Example("@MyActor enum E: Sendable { case a }", configuration: ["global_actors": ["MyActor"]]): + Example("@MyActor enum E { case a }"), + Example("actor A: B, Sendable, C {}"): + Example("actor A: B, C {}"), + ] + ) +} + +private extension RedundantSendableRule { + final class Visitor: ViolationsSyntaxVisitor { + override func visitPost(_ node: ActorDeclSyntax) { + if node.conformsToSendable { + violations.append(at: node.name.positionAfterSkippingLeadingTrivia) + } + } + + override func visitPost(_ node: ClassDeclSyntax) { + collectViolations(in: node) + } + + override func visitPost(_ node: EnumDeclSyntax) { + collectViolations(in: node) + } + + override func visitPost(_ node: ProtocolDeclSyntax) { + collectViolations(in: node) + } + + override func visitPost(_ node: StructDeclSyntax) { + collectViolations(in: node) + } + + private func collectViolations(in decl: some DeclGroupSyntax & NamedDeclSyntax) { + if decl.conformsToSendable, decl.isIsolatedToActor(actors: configuration.globalActors) { + violations.append(at: decl.name.positionAfterSkippingLeadingTrivia) + } + } + } + + final class Rewriter: ViolationsSyntaxRewriter { + override func visit(_ node: ActorDeclSyntax) -> DeclSyntax { + if node.conformsToSendable { + correctionPositions.append(node.name.positionAfterSkippingLeadingTrivia) + return super.visit(node.withoutSendable) + } + return super.visit(node) + } + + override func visit(_ node: ClassDeclSyntax) -> DeclSyntax { + super.visit(removeRedundantSendable(from: node)) + } + + override func visit(_ node: EnumDeclSyntax) -> DeclSyntax { + super.visit(removeRedundantSendable(from: node)) + } + + override func visit(_ node: ProtocolDeclSyntax) -> DeclSyntax { + super.visit(removeRedundantSendable(from: node)) + } + + override func visit(_ node: StructDeclSyntax) -> DeclSyntax { + super.visit(removeRedundantSendable(from: node)) + } + + private func removeRedundantSendable(from decl: T) -> T { + if decl.conformsToSendable, decl.isIsolatedToActor(actors: configuration.globalActors) { + correctionPositions.append(decl.name.positionAfterSkippingLeadingTrivia) + return decl.withoutSendable + } + return decl + } + } +} + +private extension DeclGroupSyntax where Self: NamedDeclSyntax { + var conformsToSendable: Bool { + inheritanceClause?.inheritedTypes.contains(where: \.isSendable) == true + } + + func isIsolatedToActor(actors: Set) -> Bool { + attributes.contains(attributeNamed: "MainActor") || actors.contains { attributes.contains(attributeNamed: $0) } + } + + var withoutSendable: Self { + guard let inheritanceClause else { + return self + } + let inheritedTypes = inheritanceClause.inheritedTypes.filter { !$0.isSendable } + if inheritedTypes.isEmpty { + return with(\.inheritanceClause, nil) + .with(\.name.trailingTrivia, inheritanceClause.leadingTrivia + inheritanceClause.trailingTrivia) + } + return with(\.inheritanceClause, inheritanceClause + .with(\.inheritedTypes, inheritedTypes)) + } +} + +private extension InheritedTypeSyntax { + var isSendable: Bool { + type.as(IdentifierTypeSyntax.self)?.name.text == "Sendable" + } +} diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/RedundantSendableConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/RedundantSendableConfiguration.swift new file mode 100644 index 0000000000..4f8bb3b8bd --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/RedundantSendableConfiguration.swift @@ -0,0 +1,11 @@ +import SwiftLintCore + +@AutoConfigParser +struct RedundantSendableConfiguration: SeverityBasedRuleConfiguration { + typealias Parent = RedundantSendableRule + + @ConfigurationElement(key: "severity") + private(set) var severityConfiguration = SeverityConfiguration(.warning) + @ConfigurationElement(key: "global_actors") + private(set) var globalActors = Set() +} diff --git a/Tests/GeneratedTests/GeneratedTests.swift b/Tests/GeneratedTests/GeneratedTests.swift index a67ea93b77..6ef660cb4d 100644 --- a/Tests/GeneratedTests/GeneratedTests.swift +++ b/Tests/GeneratedTests/GeneratedTests.swift @@ -1045,6 +1045,12 @@ final class RedundantSelfInClosureRuleGeneratedTests: SwiftLintTestCase { } } +final class RedundantSendableRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(RedundantSendableRule.description) + } +} + final class RedundantSetAccessControlRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(RedundantSetAccessControlRule.description) diff --git a/Tests/IntegrationTests/default_rule_configurations.yml b/Tests/IntegrationTests/default_rule_configurations.yml index 3a35616033..69bca24398 100644 --- a/Tests/IntegrationTests/default_rule_configurations.yml +++ b/Tests/IntegrationTests/default_rule_configurations.yml @@ -468,6 +468,9 @@ redundant_optional_initialization: severity: warning redundant_self_in_closure: severity: warning +redundant_sendable: + severity: warning + global_actors: [] redundant_set_access_control: severity: warning redundant_string_enum_value: