From 3f947f6b3581706dcd31e2f3c3a5b55407e5650d Mon Sep 17 00:00:00 2001 From: Dave Paul Date: Fri, 13 Sep 2024 13:18:41 -0500 Subject: [PATCH] Add new rule to ensure `@State` properties are private --- Rules.md | 19 ++++ Sources/RuleRegistry.generated.swift | 1 + Sources/Rules/PrivateStateVariables.swift | 47 ++++++++++ SwiftFormat.xcodeproj/project.pbxproj | 14 +++ Tests/Rules/DocCommentsTests.swift | 4 +- Tests/Rules/OrganizeDeclarationsTests.swift | 18 ++-- Tests/Rules/PrivateStateVariablesTests.swift | 87 +++++++++++++++++++ .../Rules/UnusedPrivateDeclarationTests.swift | 2 +- Tests/Rules/WrapAttributesTests.swift | 8 +- 9 files changed, 184 insertions(+), 16 deletions(-) create mode 100644 Sources/Rules/PrivateStateVariables.swift create mode 100644 Tests/Rules/PrivateStateVariablesTests.swift diff --git a/Rules.md b/Rules.md index ce8c8d81f..b8de9b101 100644 --- a/Rules.md +++ b/Rules.md @@ -103,6 +103,7 @@ * [markTypes](#markTypes) * [noExplicitOwnership](#noExplicitOwnership) * [organizeDeclarations](#organizeDeclarations) +* [privateStateVariables](#privateStateVariables) * [propertyType](#propertyType) * [redundantProperty](#redundantProperty) * [sortSwitchCases](#sortSwitchCases) @@ -1583,6 +1584,24 @@ Convert trivial `map { $0.foo }` closures to keyPath-based syntax.
+## privateStateVariables + +Adds `private` access control to @State properties without existing access control modifiers. + +
+Examples + +```diff +- @State var anInt: Int ++ @State private var anInt: Int + +- @StateObject var myInstance: MyObject ++ @StateObject private var myInstace: MyObject +``` + +
+
+ ## propertyType Convert property declarations to use inferred types (`let foo = Foo()`) or explicit types (`let foo: Foo = .init()`). diff --git a/Sources/RuleRegistry.generated.swift b/Sources/RuleRegistry.generated.swift index 53dd4972f..5435312db 100644 --- a/Sources/RuleRegistry.generated.swift +++ b/Sources/RuleRegistry.generated.swift @@ -56,6 +56,7 @@ let ruleRegistry: [String: FormatRule] = [ "organizeDeclarations": .organizeDeclarations, "preferForLoop": .preferForLoop, "preferKeyPath": .preferKeyPath, + "privateStateVariables": .privateStateVariables, "propertyType": .propertyType, "redundantBackticks": .redundantBackticks, "redundantBreak": .redundantBreak, diff --git a/Sources/Rules/PrivateStateVariables.swift b/Sources/Rules/PrivateStateVariables.swift new file mode 100644 index 000000000..fa046d9d8 --- /dev/null +++ b/Sources/Rules/PrivateStateVariables.swift @@ -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 + ``` + """ + } +} diff --git a/SwiftFormat.xcodeproj/project.pbxproj b/SwiftFormat.xcodeproj/project.pbxproj index 555b2b8dd..81d7a695a 100644 --- a/SwiftFormat.xcodeproj/project.pbxproj +++ b/SwiftFormat.xcodeproj/project.pbxproj @@ -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 */; }; @@ -1012,6 +1017,8 @@ 90CB44D81DA4B56500F86C22 /* SwiftFormatter.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = SwiftFormatter.entitlements; sourceTree = ""; }; 90F16AF71DA5EB4600EB4EA1 /* FormatFileCommand.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FormatFileCommand.swift; sourceTree = ""; }; 90F16AFA1DA5ED9A00EB4EA1 /* CommandErrors.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CommandErrors.swift; sourceTree = ""; }; + 9BDB4F1A2C94760000C93995 /* PrivateStateVariablesTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrivateStateVariablesTests.swift; sourceTree = ""; }; + 9BDB4F1C2C94773600C93995 /* PrivateStateVariables.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrivateStateVariables.swift; sourceTree = ""; }; A3DF48242620E03600F45A5F /* JSONReporter.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = JSONReporter.swift; sourceTree = ""; }; B9C4F55B2387FA3E0088DBEE /* SupportedContentUTIs.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SupportedContentUTIs.swift; sourceTree = ""; }; C2FFD1812BD13C9E00774F55 /* XMLReporter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XMLReporter.swift; sourceTree = ""; }; @@ -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 */, @@ -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 */, @@ -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 */, @@ -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 */, @@ -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 */, @@ -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 */, @@ -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 */, diff --git a/Tests/Rules/DocCommentsTests.swift b/Tests/Rules/DocCommentsTests.swift index 50b6913d5..1b450db5f 100644 --- a/Tests/Rules/DocCommentsTests.swift +++ b/Tests/Rules/DocCommentsTests.swift @@ -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() {} @@ -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() {} diff --git a/Tests/Rules/OrganizeDeclarationsTests.swift b/Tests/Rules/OrganizeDeclarationsTests.swift index ee81abce3..be892c552 100644 --- a/Tests/Rules/OrganizeDeclarationsTests.swift +++ b/Tests/Rules/OrganizeDeclarationsTests.swift @@ -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] ) } @@ -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] ) } @@ -574,7 +574,7 @@ class OrganizeDeclarationsTests: XCTestCase { visibilityOrder: ["private", "internal", "public"], typeOrder: DeclarationType.allCases.map(\.rawValue) ), - exclude: [.blankLinesAtStartOfScope] + exclude: [.blankLinesAtStartOfScope, .privateStateVariables] ) } @@ -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] ) } @@ -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] ) } @@ -3052,7 +3052,7 @@ class OrganizeDeclarationsTests: XCTestCase { blankLineAfterSubgroups: false, swiftUIPropertiesSortMode: .alphabetize ), - exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope] + exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope, .privateStateVariables] ) } @@ -3109,7 +3109,7 @@ class OrganizeDeclarationsTests: XCTestCase { blankLineAfterSubgroups: false, swiftUIPropertiesSortMode: .alphabetize ), - exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope] + exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope, .privateStateVariables] ) } @@ -3160,7 +3160,7 @@ class OrganizeDeclarationsTests: XCTestCase { blankLineAfterSubgroups: false, swiftUIPropertiesSortMode: .firstAppearanceSort ), - exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope] + exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope, .privateStateVariables] ) } @@ -3217,7 +3217,7 @@ class OrganizeDeclarationsTests: XCTestCase { blankLineAfterSubgroups: false, swiftUIPropertiesSortMode: .firstAppearanceSort ), - exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope] + exclude: [.blankLinesAtStartOfScope, .blankLinesAtEndOfScope, .privateStateVariables] ) } diff --git a/Tests/Rules/PrivateStateVariablesTests.swift b/Tests/Rules/PrivateStateVariablesTests.swift new file mode 100644 index 000000000..99e90d082 --- /dev/null +++ b/Tests/Rules/PrivateStateVariablesTests.swift @@ -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) + } +} diff --git a/Tests/Rules/UnusedPrivateDeclarationTests.swift b/Tests/Rules/UnusedPrivateDeclarationTests.swift index a9a651959..fde475b7f 100644 --- a/Tests/Rules/UnusedPrivateDeclarationTests.swift +++ b/Tests/Rules/UnusedPrivateDeclarationTests.swift @@ -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() { diff --git a/Tests/Rules/WrapAttributesTests.swift b/Tests/Rules/WrapAttributesTests.swift index 0b74ae4d9..8b881d47d 100644 --- a/Tests/Rules/WrapAttributesTests.swift +++ b/Tests/Rules/WrapAttributesTests.swift @@ -562,7 +562,7 @@ class WrapAttributesTests: XCTestCase { var foo @State - var myStoredFoo: String = "myStoredFoo" { + private var myStoredFoo: String = "myStoredFoo" { didSet { print(newValue) } @@ -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) } @@ -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 @@ -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 {