diff --git a/CHANGELOG.md b/CHANGELOG.md index d6e2077c9c..16dd0efdce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,11 @@ * Rewrite `control_statement` rule using SwiftSyntax. [SimplyDanny](https://github.com/SimplyDanny) +* Add new `private_swiftui_state_property` opt-in rule to encourage setting + SwiftUI `@State` properties to private. + [mt00chikin](https://github.com/mt00chikin) + [#3173](https://github.com/realm/SwiftLint/issues/3173) + * Add `unneeded_override` rule to remove function overrides that only call super. [keith](https://github.com/keith) diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index 23c9194797..359fbf4d3f 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -151,6 +151,7 @@ public let builtInRules: [Rule.Type] = [ PrivateOutletRule.self, PrivateOverFilePrivateRule.self, PrivateSubjectRule.self, + PrivateSwiftUIStatePropertyRule.self, PrivateUnitTestRule.self, ProhibitedInterfaceBuilderRule.self, ProhibitedSuperRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/PrivateSwiftUIStatePropertyRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/PrivateSwiftUIStatePropertyRule.swift new file mode 100644 index 0000000000..952a525b3d --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/PrivateSwiftUIStatePropertyRule.swift @@ -0,0 +1,246 @@ +import SwiftSyntax + +/// Require that any state properties in SwiftUI be declared as private +/// +/// State properties should only be accessible from inside a SwiftUI App, View, or Scene, or from methods called by it +struct PrivateSwiftUIStatePropertyRule: SwiftSyntaxRule, OptInRule, ConfigurationProviderRule { + var configuration = SeverityConfiguration(.warning) + + static let description = RuleDescription( + identifier: "private_swiftui_state", + name: "Private SwiftUI @State Properties", + description: "SwiftUI state properties should be private", + kind: .lint, + nonTriggeringExamples: [ + Example(""" + struct MyApp: App { + @State private var isPlaying: Bool = false + } + """), + Example(""" + struct MyScene: Scene { + @State private var isPlaying: Bool = false + } + """), + Example(""" + struct ContentView: View { + @State private var isPlaying: Bool = false + } + """), + Example(""" + struct ContentView: View { + @State fileprivate var isPlaying: Bool = false + } + """), + Example(""" + struct ContentView: View { + @State private var isPlaying: Bool = false + + struct InnerView: View { + @State private var showsIndicator: Bool = false + } + } + """), + Example(""" + struct MyStruct { + struct ContentView: View { + @State private var isPlaying: Bool = false + } + } + """), + Example(""" + struct MyStruct { + struct ContentView: View { + @State private var isPlaying: Bool = false + } + + @State var nonTriggeringState: Bool = false + } + """), + + Example(""" + struct ContentView: View { + var isPlaying = false + } + """), + Example(""" + struct ContentView: View { + @StateObject var foo = Foo() + } + """), + Example(""" + struct Foo { + @State var bar = false + } + """), + Example(""" + class Foo: ObservableObject { + @State var bar = Bar() + } + """), + Example(""" + extension MyObject { + struct ContentView: View { + @State private var isPlaying: Bool = false + } + } + """), + Example(""" + actor ContentView: View { + @State private var isPlaying: Bool = false + } + """) + ], + triggeringExamples: [ + Example(""" + struct MyApp: App { + @State ↓var isPlaying: Bool = false + } + """), + Example(""" + struct MyScene: Scene { + @State ↓var isPlaying: Bool = false + } + """), + Example(""" + struct ContentView: View { + @State ↓var isPlaying: Bool = false + } + """), + Example(""" + struct ContentView: View { + struct InnerView: View { + @State private var showsIndicator: Bool = false + } + + @State ↓var isPlaying: Bool = false + } + """), + Example(""" + struct MyStruct { + struct ContentView: View { + @State ↓var isPlaying: Bool = false + } + } + """), + Example(""" + struct MyStruct { + struct ContentView: View { + @State ↓var isPlaying: Bool = false + } + + @State var isPlaying: Bool = false + } + """), + Example(""" + final class ContentView: View { + @State ↓var isPlaying: Bool = false + } + """), + Example(""" + extension MyObject { + struct ContentView: View { + @State ↓var isPlaying: Bool = false + } + } + """), + Example(""" + actor ContentView: View { + @State ↓var isPlaying: Bool = false + } + """) + ] + ) + + func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor { + Visitor(viewMode: .sourceAccurate) + } +} + +private extension PrivateSwiftUIStatePropertyRule { + final class Visitor: ViolationsSyntaxVisitor { + override var skippableDeclarations: [DeclSyntaxProtocol.Type] { + [ProtocolDeclSyntax.self] + } + + /// LIFO stack that stores type inheritance clauses for each visited node + /// The last value is the inheritance clause for the most recently visited node + /// A nil value indicates that the node does not provide any inheritance clause + private var visitedTypeInheritances = Stack() + + override func visit(_ node: ClassDeclSyntax) -> SyntaxVisitorContinueKind { + visitedTypeInheritances.push(node.inheritanceClause) + return .visitChildren + } + + override func visitPost(_ node: ClassDeclSyntax) { + visitedTypeInheritances.pop() + } + + override func visit(_ node: StructDeclSyntax) -> SyntaxVisitorContinueKind { + visitedTypeInheritances.push(node.inheritanceClause) + return .visitChildren + } + + override func visitPost(_ node: StructDeclSyntax) { + visitedTypeInheritances.pop() + } + + override func visit(_ node: ActorDeclSyntax) -> SyntaxVisitorContinueKind { + visitedTypeInheritances.push(node.inheritanceClause) + return .visitChildren + } + + override func visitPost(_ node: ActorDeclSyntax) { + visitedTypeInheritances.pop() + } + + override func visitPost(_ node: MemberDeclListItemSyntax) { + guard + let decl = node.decl.as(VariableDeclSyntax.self), + let inheritanceClause = visitedTypeInheritances.peek() as? TypeInheritanceClauseSyntax, + inheritanceClause.conformsToApplicableSwiftUIProtocol, + decl.attributes.hasStateAttribute, + !decl.modifiers.isPrivateOrFileprivate + else { + return + } + + violations.append(decl.bindingKeyword.positionAfterSkippingLeadingTrivia) + } + } +} + +private extension TypeInheritanceClauseSyntax { + static let applicableSwiftUIProtocols: Set = ["View", "App", "Scene"] + + var conformsToApplicableSwiftUIProtocol: Bool { + inheritedTypeCollection.containsInheritedType(inheritedTypes: Self.applicableSwiftUIProtocols) + } +} + +private extension InheritedTypeListSyntax { + func containsInheritedType(inheritedTypes: Set) -> Bool { + contains { + guard let simpleType = $0.typeName.as(SimpleTypeIdentifierSyntax.self) else { return false } + + return inheritedTypes.contains(simpleType.name.text) + } + } +} + +private extension AttributeListSyntax? { + /// Returns `true` if the attribute's identifier is equal to "State" + var hasStateAttribute: Bool { + guard let attributes = self else { return false } + + return attributes.contains { attr in + guard let stateAttr = attr.as(AttributeSyntax.self), + let identifier = stateAttr.attributeName.as(SimpleTypeIdentifierSyntax.self) else { + return false + } + + return identifier.name.text == "State" + } + } +} diff --git a/Tests/GeneratedTests/GeneratedTests.swift b/Tests/GeneratedTests/GeneratedTests.swift index ad2053a90e..7aa96e698f 100644 --- a/Tests/GeneratedTests/GeneratedTests.swift +++ b/Tests/GeneratedTests/GeneratedTests.swift @@ -896,6 +896,12 @@ class PrivateSubjectRuleGeneratedTests: SwiftLintTestCase { } } +class PrivateSwiftUIStatePropertyRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(PrivateSwiftUIStatePropertyRule.description) + } +} + class PrivateUnitTestRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(PrivateUnitTestRule.description)