-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add redundant_final_actor opt-in rule
#6504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8ed2d15
f655ffa
aab4aab
120b9da
108aba6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| import SwiftSyntax | ||
|
|
||
| @SwiftSyntaxRule(correctable: true, optIn: true) | ||
| struct RedundantFinalActorRule: Rule { | ||
| var configuration = SeverityConfiguration<Self>(.warning) | ||
|
|
||
| static let description = RuleDescription( | ||
| identifier: "redundant_final_actor", | ||
| name: "Redundant Final on Actor", | ||
| description: "`final` is redundant on an actor declaration and its members because actors cannot be subclassed", | ||
| rationale: """ | ||
| Actors in Swift currently do not support inheritance, making `final` redundant \ | ||
| on both actor declarations and their members. Note that this may change in future \ | ||
| Swift versions if actor inheritance is introduced. | ||
| """, | ||
| kind: .idiomatic, | ||
| nonTriggeringExamples: [ | ||
| Example("actor MyActor {}"), | ||
| Example("final class MyClass {}"), | ||
| Example(""" | ||
| @globalActor | ||
| actor MyGlobalActor {} | ||
| """), | ||
| Example(""" | ||
| actor MyActor { | ||
| func doWork() {} | ||
| var value: Int { 0 } | ||
| } | ||
| """), | ||
| Example(""" | ||
| class MyClass { | ||
| final func doWork() {} | ||
| } | ||
| """), | ||
| ], | ||
| triggeringExamples: [ | ||
| Example("↓final actor MyActor {}"), | ||
| Example("public ↓final actor DataStore {}"), | ||
| Example(""" | ||
| @globalActor | ||
| ↓final actor MyGlobalActor {} | ||
| """), | ||
| Example(""" | ||
| actor MyActor { | ||
| ↓final func doWork() {} | ||
| } | ||
| """), | ||
| Example(""" | ||
| actor MyActor { | ||
| ↓final var value: Int { 0 } | ||
| } | ||
| """), | ||
| ], | ||
| corrections: [ | ||
| Example("final actor MyActor {}"): | ||
| Example("actor MyActor {}"), | ||
| Example("public final actor DataStore {}"): | ||
| Example("public actor DataStore {}"), | ||
| Example("actor MyActor {\n final func doWork() {}\n}"): | ||
| Example("actor MyActor {\n func doWork() {}\n}"), | ||
| ] | ||
| ) | ||
| } | ||
|
|
||
| private extension RedundantFinalActorRule { | ||
| final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> { | ||
| private var insideActor = false | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not sufficient. You would need to have a stack to track "is actor" when entering a type declaration. Otherwise, you cannot correctly handle nested types, e.g. struct S {
final actor A {}
}Another case that justifies a stack is actor A {
final var x: Int { 1 } // Rule triggers here.
struct S {}
final var y: Int { 2 } // Would it trigger here as well?
} |
||
|
|
||
| override func visit(_ node: ActorDeclSyntax) -> SyntaxVisitorContinueKind { | ||
| insideActor = true | ||
| return .visitChildren | ||
| } | ||
|
|
||
| override func visitPost(_ node: ActorDeclSyntax) { | ||
| if let finalModifier = node.modifiers.first(where: { $0.name.text == "final" }) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could have the keyword lookup in an extension on |
||
| addViolation(for: finalModifier) | ||
| } | ||
| insideActor = false | ||
| } | ||
|
|
||
| override func visitPost(_ node: FunctionDeclSyntax) { | ||
| guard insideActor, | ||
| let finalModifier = node.modifiers.first(where: { $0.name.text == "final" }) else { | ||
| return | ||
| } | ||
| addViolation(for: finalModifier) | ||
| } | ||
|
|
||
| override func visitPost(_ node: VariableDeclSyntax) { | ||
| guard insideActor, | ||
| let finalModifier = node.modifiers.first(where: { $0.name.text == "final" }) else { | ||
| return | ||
| } | ||
| addViolation(for: finalModifier) | ||
| } | ||
|
|
||
| override func visitPost(_ node: SubscriptDeclSyntax) { | ||
| guard insideActor, | ||
| let finalModifier = node.modifiers.first(where: { $0.name.text == "final" }) else { | ||
| return | ||
| } | ||
| addViolation(for: finalModifier) | ||
| } | ||
|
|
||
| // Don't descend into nested classes where `final` is meaningful | ||
| override func visit(_: ClassDeclSyntax) -> SyntaxVisitorContinueKind { | ||
| .skipChildren | ||
| } | ||
|
|
||
| private func addViolation(for finalModifier: DeclModifierSyntax) { | ||
| let start = finalModifier.positionAfterSkippingLeadingTrivia | ||
| let end = finalModifier.endPosition | ||
| violations.append( | ||
| ReasonedRuleViolation( | ||
| position: start, | ||
| correction: .init( | ||
| start: start, | ||
| end: end, | ||
| replacement: "" | ||
| ) | ||
| ) | ||
| ) | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep it short: