Skip to content

Commit

Permalink
Add new rule to ensure @State properties are private
Browse files Browse the repository at this point in the history
  • Loading branch information
Dave Paul authored and nicklockwood committed Sep 15, 2024
1 parent acfbb62 commit 3f947f6
Show file tree
Hide file tree
Showing 9 changed files with 184 additions and 16 deletions.
19 changes: 19 additions & 0 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@
* [markTypes](#markTypes)
* [noExplicitOwnership](#noExplicitOwnership)
* [organizeDeclarations](#organizeDeclarations)
* [privateStateVariables](#privateStateVariables)
* [propertyType](#propertyType)
* [redundantProperty](#redundantProperty)
* [sortSwitchCases](#sortSwitchCases)
Expand Down Expand Up @@ -1583,6 +1584,24 @@ Convert trivial `map { $0.foo }` closures to keyPath-based syntax.
</details>
<br/>

## privateStateVariables

Adds `private` access control to @State properties without existing access control modifiers.

<details>
<summary>Examples</summary>

```diff
- @State var anInt: Int
+ @State private var anInt: Int

- @StateObject var myInstance: MyObject
+ @StateObject private var myInstace: MyObject
```

</details>
<br/>

## propertyType

Convert property declarations to use inferred types (`let foo = Foo()`) or explicit types (`let foo: Foo = .init()`).
Expand Down
1 change: 1 addition & 0 deletions Sources/RuleRegistry.generated.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ let ruleRegistry: [String: FormatRule] = [
"organizeDeclarations": .organizeDeclarations,
"preferForLoop": .preferForLoop,
"preferKeyPath": .preferKeyPath,
"privateStateVariables": .privateStateVariables,
"propertyType": .propertyType,
"redundantBackticks": .redundantBackticks,
"redundantBreak": .redundantBreak,
Expand Down
47 changes: 47 additions & 0 deletions Sources/Rules/PrivateStateVariables.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//
// PrivateStateVariables.swift
// SwiftFormatTests
//
// Created by Dave Paul on 9/13/24.
// Copyright © 2024 Nick Lockwood. All rights reserved.
//

import Foundation

public extension FormatRule {
static let privateStateVariables = FormatRule(
help: "Adds `private` access control to @State properties without existing access control modifiers.",
disabledByDefault: true
) { formatter in
formatter.forEachToken(where: { $0 == .keyword("@State") || $0 == .keyword("@StateObject") }) { stateIndex, _ in
guard let endOfScope = formatter.index(after: stateIndex, where: {
$0 == .keyword("let") || $0 == .keyword("var")
}) else { return }

// Don't override any existing access control:
guard !formatter.tokens[stateIndex ..< endOfScope].contains(where: {
_FormatRules.aclModifiers.contains($0.string) || _FormatRules.aclSetterModifiers.contains($0.string)
}) else {
return
}

// Check for @Previewable - we won't modify @Previewable macros.
let lineStart = formatter.startOfLine(at: stateIndex)
guard !formatter.tokens[lineStart ..< stateIndex].contains(where: { $0 == .keyword("@Previewable") }) else {
return
}

formatter.insert([.keyword("private"), .space(" ")], at: endOfScope)
}
} examples: {
"""
```diff
- @State var anInt: Int
+ @State private var anInt: Int
- @StateObject var myInstance: MyObject
+ @StateObject private var myInstace: MyObject
```
"""
}
}
14 changes: 14 additions & 0 deletions SwiftFormat.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,11 @@
90C4B6EB1DA4B059009EB000 /* SwiftFormat.appex in Embed Foundation Extensions */ = {isa = PBXBuildFile; fileRef = 90C4B6DD1DA4B059009EB000 /* SwiftFormat.appex */; settings = {ATTRIBUTES = (RemoveHeadersOnCopy, ); }; };
90F16AF81DA5EB4600EB4EA1 /* FormatFileCommand.swift in Sources */ = {isa = PBXBuildFile; fileRef = 90F16AF71DA5EB4600EB4EA1 /* FormatFileCommand.swift */; };
90F16AFB1DA5ED9A00EB4EA1 /* CommandErrors.swift in Sources */ = {isa = PBXBuildFile; fileRef = 90F16AFA1DA5ED9A00EB4EA1 /* CommandErrors.swift */; };
9BDB4F1B2C94760000C93995 /* PrivateStateVariablesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9BDB4F1A2C94760000C93995 /* PrivateStateVariablesTests.swift */; };
9BDB4F1E2C9477FF00C93995 /* PrivateStateVariables.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9BDB4F1C2C94773600C93995 /* PrivateStateVariables.swift */; };
9BDB4F1F2C94780000C93995 /* PrivateStateVariables.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9BDB4F1C2C94773600C93995 /* PrivateStateVariables.swift */; };
9BDB4F202C94780100C93995 /* PrivateStateVariables.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9BDB4F1C2C94773600C93995 /* PrivateStateVariables.swift */; };
9BDB4F212C94780200C93995 /* PrivateStateVariables.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9BDB4F1C2C94773600C93995 /* PrivateStateVariables.swift */; };
A3DF48252620E03600F45A5F /* JSONReporter.swift in Sources */ = {isa = PBXBuildFile; fileRef = A3DF48242620E03600F45A5F /* JSONReporter.swift */; };
A3DF48262620E03600F45A5F /* JSONReporter.swift in Sources */ = {isa = PBXBuildFile; fileRef = A3DF48242620E03600F45A5F /* JSONReporter.swift */; };
B9C4F55C2387FA3E0088DBEE /* SupportedContentUTIs.swift in Sources */ = {isa = PBXBuildFile; fileRef = B9C4F55B2387FA3E0088DBEE /* SupportedContentUTIs.swift */; };
Expand Down Expand Up @@ -1012,6 +1017,8 @@
90CB44D81DA4B56500F86C22 /* SwiftFormatter.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = SwiftFormatter.entitlements; sourceTree = "<group>"; };
90F16AF71DA5EB4600EB4EA1 /* FormatFileCommand.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FormatFileCommand.swift; sourceTree = "<group>"; };
90F16AFA1DA5ED9A00EB4EA1 /* CommandErrors.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CommandErrors.swift; sourceTree = "<group>"; };
9BDB4F1A2C94760000C93995 /* PrivateStateVariablesTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrivateStateVariablesTests.swift; sourceTree = "<group>"; };
9BDB4F1C2C94773600C93995 /* PrivateStateVariables.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrivateStateVariables.swift; sourceTree = "<group>"; };
A3DF48242620E03600F45A5F /* JSONReporter.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = JSONReporter.swift; sourceTree = "<group>"; };
B9C4F55B2387FA3E0088DBEE /* SupportedContentUTIs.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SupportedContentUTIs.swift; sourceTree = "<group>"; };
C2FFD1812BD13C9E00774F55 /* XMLReporter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XMLReporter.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1251,6 +1258,7 @@
2E2BABBF2C57F6DD00590239 /* OrganizeDeclarations.swift */,
2E2BABCE2C57F6DD00590239 /* PreferForLoop.swift */,
2E2BABDB2C57F6DD00590239 /* PreferKeyPath.swift */,
9BDB4F1C2C94773600C93995 /* PrivateStateVariables.swift */,
2E2BABF42C57F6DD00590239 /* PropertyType.swift */,
2E2BABE92C57F6DD00590239 /* RedundantBackticks.swift */,
2E2BAB922C57F6DD00590239 /* RedundantBreak.swift */,
Expand Down Expand Up @@ -1370,6 +1378,7 @@
2E8DE6E22C57FEB30032BF25 /* OrganizeDeclarationsTests.swift */,
2E8DE6982C57FEB30032BF25 /* PreferForLoopTests.swift */,
2E8DE6BF2C57FEB30032BF25 /* PreferKeyPathTests.swift */,
9BDB4F1A2C94760000C93995 /* PrivateStateVariablesTests.swift */,
2E8DE6C92C57FEB30032BF25 /* PropertyTypeTests.swift */,
2E8DE6C42C57FEB30032BF25 /* RedundantBackticksTests.swift */,
2E8DE6BC2C57FEB30032BF25 /* RedundantBreakTests.swift */,
Expand Down Expand Up @@ -1904,6 +1913,7 @@
2E2BAD072C57F6DD00590239 /* RedundantPattern.swift in Sources */,
2E2BACE72C57F6DD00590239 /* TrailingSpace.swift in Sources */,
2E2BACEF2C57F6DD00590239 /* ConditionalAssignment.swift in Sources */,
9BDB4F1E2C9477FF00C93995 /* PrivateStateVariables.swift in Sources */,
2E2BAD172C57F6DD00590239 /* BlankLineAfterImports.swift in Sources */,
2E2BAD332C57F6DD00590239 /* WrapConditionalBodies.swift in Sources */,
2E2BAC2F2C57F6DD00590239 /* SpaceInsideParens.swift in Sources */,
Expand Down Expand Up @@ -2032,6 +2042,7 @@
2E8DE6FB2C57FEB30032BF25 /* BlankLinesAroundMarkTests.swift in Sources */,
2E8DE7582C57FEB30032BF25 /* RedundantVoidReturnTypeTests.swift in Sources */,
2E8DE7352C57FEB30032BF25 /* SpaceAroundOperatorsTests.swift in Sources */,
9BDB4F1B2C94760000C93995 /* PrivateStateVariablesTests.swift in Sources */,
2E8DE7412C57FEB30032BF25 /* RedundantOptionalBindingTests.swift in Sources */,
2E8DE7512C57FEB30032BF25 /* LinebreaksTests.swift in Sources */,
01F3DF901DBA003E00454944 /* InferenceTests.swift in Sources */,
Expand Down Expand Up @@ -2194,6 +2205,7 @@
2E2BACD42C57F6DD00590239 /* BlankLinesAtEndOfScope.swift in Sources */,
2E2BAC542C57F6DD00590239 /* SortedImports.swift in Sources */,
2E2BAD8C2C57F6DD00590239 /* PropertyType.swift in Sources */,
9BDB4F1F2C94780000C93995 /* PrivateStateVariables.swift in Sources */,
2E2BAC202C57F6DD00590239 /* RedundantOptionalBinding.swift in Sources */,
2E2BAD6C2C57F6DD00590239 /* BlankLinesAtStartOfScope.swift in Sources */,
2E2BAC302C57F6DD00590239 /* SpaceInsideParens.swift in Sources */,
Expand Down Expand Up @@ -2293,6 +2305,7 @@
2E2BAD5D2C57F6DD00590239 /* AnyObjectProtocol.swift in Sources */,
2E2BAD492C57F6DD00590239 /* WrapLoopBodies.swift in Sources */,
2E2BACF52C57F6DD00590239 /* PreferForLoop.swift in Sources */,
9BDB4F202C94780100C93995 /* PrivateStateVariables.swift in Sources */,
2E2BACCD2C57F6DD00590239 /* IsEmpty.swift in Sources */,
2E2BAD3D2C57F6DD00590239 /* Braces.swift in Sources */,
2E2BAD712C57F6DD00590239 /* OpaqueGenericParameters.swift in Sources */,
Expand Down Expand Up @@ -2437,6 +2450,7 @@
2E2BAC322C57F6DD00590239 /* SpaceInsideParens.swift in Sources */,
2E2BAD5E2C57F6DD00590239 /* AnyObjectProtocol.swift in Sources */,
2E2BAD4A2C57F6DD00590239 /* WrapLoopBodies.swift in Sources */,
9BDB4F212C94780200C93995 /* PrivateStateVariables.swift in Sources */,
2E2BACF62C57F6DD00590239 /* PreferForLoop.swift in Sources */,
2E2BACCE2C57F6DD00590239 /* IsEmpty.swift in Sources */,
2E2BAD3E2C57F6DD00590239 /* Braces.swift in Sources */,
Expand Down
4 changes: 2 additions & 2 deletions Tests/Rules/DocCommentsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class DocCommentsTests: XCTestCase {
// Single line comment before property with property wrapper
@State
let bar = Bar()
private let bar = Bar()
// Single line comment
func foo() {}
Expand Down Expand Up @@ -62,7 +62,7 @@ class DocCommentsTests: XCTestCase {
/// Single line comment before property with property wrapper
@State
let bar = Bar()
private let bar = Bar()
/// Single line comment
func foo() {}
Expand Down
18 changes: 9 additions & 9 deletions Tests/Rules/OrganizeDeclarationsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ class OrganizeDeclarationsTests: XCTestCase {
for: input, output,
rule: .organizeDeclarations,
options: FormatOptions(categoryMarkComment: "MARK: %c", organizationMode: .type),
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope]
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope, .privateStateVariables]
)
}

Expand Down Expand Up @@ -537,7 +537,7 @@ class OrganizeDeclarationsTests: XCTestCase {
for: input, output,
rule: .organizeDeclarations,
options: FormatOptions(categoryMarkComment: "MARK: %c", organizationMode: .type),
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope]
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope, .privateStateVariables]
)
}

Expand Down Expand Up @@ -574,7 +574,7 @@ class OrganizeDeclarationsTests: XCTestCase {
visibilityOrder: ["private", "internal", "public"],
typeOrder: DeclarationType.allCases.map(\.rawValue)
),
exclude: [.blankLinesAtStartOfScope]
exclude: [.blankLinesAtStartOfScope, .privateStateVariables]
)
}

Expand Down Expand Up @@ -2767,7 +2767,7 @@ class OrganizeDeclarationsTests: XCTestCase {
for: input, output,
rule: .organizeDeclarations,
options: FormatOptions(organizeTypes: ["struct"], organizationMode: .visibility),
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope]
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope, .privateStateVariables]
)
}

Expand Down Expand Up @@ -2834,7 +2834,7 @@ class OrganizeDeclarationsTests: XCTestCase {
for: input, output,
rule: .organizeDeclarations,
options: FormatOptions(organizeTypes: ["struct"], organizationMode: .visibility),
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope]
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope, .privateStateVariables]
)
}

Expand Down Expand Up @@ -3052,7 +3052,7 @@ class OrganizeDeclarationsTests: XCTestCase {
blankLineAfterSubgroups: false,
swiftUIPropertiesSortMode: .alphabetize
),
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope]
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope, .privateStateVariables]
)
}

Expand Down Expand Up @@ -3109,7 +3109,7 @@ class OrganizeDeclarationsTests: XCTestCase {
blankLineAfterSubgroups: false,
swiftUIPropertiesSortMode: .alphabetize
),
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope]
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope, .privateStateVariables]
)
}

Expand Down Expand Up @@ -3160,7 +3160,7 @@ class OrganizeDeclarationsTests: XCTestCase {
blankLineAfterSubgroups: false,
swiftUIPropertiesSortMode: .firstAppearanceSort
),
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope]
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope, .privateStateVariables]
)
}

Expand Down Expand Up @@ -3217,7 +3217,7 @@ class OrganizeDeclarationsTests: XCTestCase {
blankLineAfterSubgroups: false,
swiftUIPropertiesSortMode: .firstAppearanceSort
),
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope]
exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope, .privateStateVariables]
)
}

Expand Down
87 changes: 87 additions & 0 deletions Tests/Rules/PrivateStateVariablesTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
//
// PrivateStateVariablesTests.swift
// SwiftFormatTests
//
// Created by Dave Paul on 9/9/24.
// Copyright © 2024 Nick Lockwood. All rights reserved.
//

import XCTest
@testable import SwiftFormat

class PrivateStateVariablesTests: XCTestCase {
func testPrivateState() {
let input = """
@State var counter: Int
"""
let output = """
@State private var counter: Int
"""
testFormatting(for: input, output, rule: .privateStateVariables)
}

func testPrivateStateObject() {
let input = """
@StateObject var counter: Int
"""
let output = """
@StateObject private var counter: Int
"""
testFormatting(for: input, output, rule: .privateStateVariables)
}

func testUseExisting() {
let input = """
@State private var counter: Int
"""
testFormatting(for: input, rule: .privateStateVariables)
}

func testRespectingPublicOverride() {
let input = """
@StateObject public var counter: Int
"""
testFormatting(for: input, rule: .privateStateVariables)
}

func testRespectingPackageOverride() {
let input = """
@State package var counter: Int
"""
testFormatting(for: input, rule: .privateStateVariables)
}

func testRespectingOverrideWithSetterModifier() {
let input = """
@State private(set) var counter: Int
"""
testFormatting(for: input, rule: .privateStateVariables)
}

func testRespectingOverrideWithExistingAccessAndSetterModifier() {
let input = """
@StateObject public private(set) var counter: Int
"""
testFormatting(for: input, rule: .privateStateVariables)
}

func testStateVariableOnPreviousLine() {
let input = """
@State
var counter: Int
"""
let output = """
@State
private var counter: Int
"""
testFormatting(for: input, output, rule: .privateStateVariables)
}

func testWithPreviewable() {
// Don't add `private` to @Previewable property wrappers:
let input = """
@Previewable @StateObject var counter: Int
"""
testFormatting(for: input, rule: .privateStateVariables)
}
}
2 changes: 1 addition & 1 deletion Tests/Rules/UnusedPrivateDeclarationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ class UnusedPrivateDeclarationTests: XCTestCase {
@State private var showButton: Bool
}
"""
testFormatting(for: input, rule: .unusedPrivateDeclaration)
testFormatting(for: input, rule: .unusedPrivateDeclaration, exclude: [.privateStateVariables])
}

func testDoesNotRemoveUnderscoredDeclarationIfUsed() {
Expand Down
8 changes: 4 additions & 4 deletions Tests/Rules/WrapAttributesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ class WrapAttributesTests: XCTestCase {
var foo
@State
var myStoredFoo: String = "myStoredFoo" {
private var myStoredFoo: String = "myStoredFoo" {
didSet {
print(newValue)
}
Expand All @@ -583,7 +583,7 @@ class WrapAttributesTests: XCTestCase {
@Environment(\\.myEnvironmentVar) var foo
@State var myStoredFoo: String = "myStoredFoo" {
@State private var myStoredFoo: String = "myStoredFoo" {
didSet {
print(newValue)
}
Expand All @@ -597,7 +597,7 @@ class WrapAttributesTests: XCTestCase {
func testWrapOrDontAttributesInSwiftUIView() {
let input = """
struct MyView: View {
@State var textContent: String
@State private var textContent: String
var body: some View {
childView
Expand All @@ -617,7 +617,7 @@ class WrapAttributesTests: XCTestCase {
func testWrapAttributesInSwiftUIView() {
let input = """
struct MyView: View {
@State var textContent: String
@State private var textContent: String
@Environment(\\.myEnvironmentVar) var environmentVar
var body: some View {
Expand Down

0 comments on commit 3f947f6

Please sign in to comment.