Add --test-case-access-control option for testSuiteAccessControl rule (#2457)

Co-authored-by: calda <1811727+calda@users.noreply.github.com>
This commit is contained in:
Copilot
2026-03-16 20:31:03 -07:00
committed by Cal Stephens
parent 9d60158a4a
commit 357efe0348
6 changed files with 258 additions and 50 deletions
+5 -1
View File
@@ -3754,7 +3754,11 @@ Option | Description
## testSuiteAccessControl
Test methods should be internal, and other properties / functions in a test suite should be private.
Test methods should have the configured access control (default internal), and other properties / functions in a test suite should be private.
Option | Description
--- | ---
`--test-case-access-control` | Access control for test methods: "open", "public", "package", "internal" (default), "fileprivate" or "private"
<details>
<summary>Examples</summary>
+2 -2
View File
@@ -450,7 +450,7 @@ extension Declaration {
// MARK: - Visibility
/// The visibility of a declaration
enum Visibility: String, CaseIterable, Comparable {
public enum Visibility: String, CaseIterable, Comparable {
case open
case `public`
case package
@@ -458,7 +458,7 @@ enum Visibility: String, CaseIterable, Comparable {
case `fileprivate`
case `private`
static func < (lhs: Visibility, rhs: Visibility) -> Bool {
public static func < (lhs: Visibility, rhs: Visibility) -> Bool {
allCases.firstIndex(of: lhs)! > allCases.firstIndex(of: rhs)!
}
}
+7
View File
@@ -1475,6 +1475,13 @@ struct _Descriptors {
keyPath: \.suiteNameFormat
)
let testCaseAccessControl = OptionDescriptor(
argumentName: "test-case-access-control",
displayName: "Test Case Access Control",
help: "Access control for test methods:",
keyPath: \.testCaseAccessControl
)
// MARK: - Internal
let fragment = OptionDescriptor(
+3
View File
@@ -909,6 +909,7 @@ public struct FormatOptions: CustomStringConvertible {
public var preferSynthesizedInitForInternalStructs: PreferSynthesizedInitMode
public var testCaseNameFormat: SwiftTestingNameFormat
public var suiteNameFormat: SwiftTestingNameFormat
public var testCaseAccessControl: Visibility
/// Deprecated
public var indentComments: Bool
@@ -1056,6 +1057,7 @@ public struct FormatOptions: CustomStringConvertible {
preferSynthesizedInitForInternalStructs: PreferSynthesizedInitMode = .never,
testCaseNameFormat: SwiftTestingNameFormat = .rawIdentifiers,
suiteNameFormat: SwiftTestingNameFormat = .preserve,
testCaseAccessControl: Visibility = .internal,
// Doesn't really belong here, but hard to put elsewhere
fragment: Bool = false,
ignoreConflictMarkers: Bool = false,
@@ -1192,6 +1194,7 @@ public struct FormatOptions: CustomStringConvertible {
self.preferSynthesizedInitForInternalStructs = preferSynthesizedInitForInternalStructs
self.testCaseNameFormat = testCaseNameFormat
self.suiteNameFormat = suiteNameFormat
self.testCaseAccessControl = testCaseAccessControl
self.indentComments = indentComments
self.fragment = fragment
self.ignoreConflictMarkers = ignoreConflictMarkers
+75 -47
View File
@@ -10,31 +10,44 @@ import Foundation
public extension FormatRule {
static let testSuiteAccessControl = FormatRule(
help: "Test methods should be internal, and other properties / functions in a test suite should be private.",
disabledByDefault: true
help: "Test methods should have the configured access control (default internal), and other properties / functions in a test suite should be private.",
disabledByDefault: true,
options: ["test-case-access-control"]
) { formatter in
guard let testFramework = formatter.detectTestingFramework() else {
return
}
// Determine the effective test visibility based on options and framework.
// XCTest requires test methods to be at least internal so the runtime can discover them.
let configuredVisibility = formatter.options.testCaseAccessControl
let effectiveTestVisibility: Visibility
if testFramework == .xcTest,
configuredVisibility == .private || configuredVisibility == .fileprivate
{
effectiveTestVisibility = .internal
} else {
effectiveTestVisibility = configuredVisibility
}
let declarations = formatter.parseDeclarations()
let testClasses = declarations.compactMap(\.asTypeDeclaration).filter { typeDecl in
formatter.isSimpleTestSuite(typeDecl, for: testFramework)
}
for testClass in testClasses {
// The test class itself should be internal unless marked as open
formatter.validateTestTypeAccessControl(testClass)
// The test class itself should have the configured visibility unless marked as open
testClass.ensureTestDeclarationAccessControl(visibility: effectiveTestVisibility)
// Process each member of the test class
for member in testClass.body {
switch member.keyword {
case "func":
formatter.validateTestFunctionAccessControl(member, for: testFramework)
formatter.validateTestFunctionAccessControl(member, for: testFramework, testCaseAccessControl: effectiveTestVisibility)
case "init":
// Initializers should be internal unless marked as open
formatter.validateTestTypeAccessControl(member)
// Initializers should have the configured visibility unless marked as open
member.ensureTestDeclarationAccessControl(visibility: effectiveTestVisibility)
case "let", "var":
// Properties should be private unless they have special attributes
@@ -83,19 +96,8 @@ public extension FormatRule {
}
extension Formatter {
/// Validates that a test type (class/struct) or its initializer has internal access control.
func validateTestTypeAccessControl(_ declaration: Declaration) {
// If marked as open, leave it as is
if declaration.modifiers.contains("open") {
return
}
// Remove any non-internal, non-open ACL modifiers
removeACLModifiers(from: declaration, except: ["internal", "open"])
}
/// Validates that a function in a test class has the correct access control.
func validateTestFunctionAccessControl(_ function: Declaration, for framework: TestingFramework) {
func validateTestFunctionAccessControl(_ function: Declaration, for framework: TestingFramework, testCaseAccessControl: Visibility) {
guard let functionDecl = parseFunctionDeclaration(keywordIndex: function.keywordIndex) else {
return
}
@@ -117,32 +119,20 @@ extension Formatter {
if treatAsTestCase {
// For XCTest: Skip if it's already private/fileprivate (respect explicit access control)
// For Swift Testing: Always make internal (private @Test functions are still executed)
if framework == .xcTest, modifiers.contains("private") || modifiers.contains("fileprivate") {
return
}
// Test methods should be internal
validateTestMethodAccessControl(function)
// Test methods should have the configured test visibility
function.ensureTestDeclarationAccessControl(visibility: testCaseAccessControl)
} else {
// Non-test methods should be private (but skip if already private/fileprivate)
if modifiers.contains("private") || modifiers.contains("fileprivate") {
return
}
ensurePrivateAccessControl(function)
function.ensurePrivateAccessControl()
}
}
/// Ensures a test method has internal access control (removes public/private modifiers).
func validateTestMethodAccessControl(_ declaration: Declaration) {
// If marked as open, leave it as is
if declaration.modifiers.contains("open") {
return
}
// Remove any explicit ACL modifiers except internal and open
removeACLModifiers(from: declaration, except: ["internal", "open"])
}
/// Validates that a property in a test class is private.
func validateTestProperty(_ property: Declaration, for _: TestingFramework) {
let modifiers = property.modifiers
@@ -163,26 +153,64 @@ extension Formatter {
}
// Make it private
ensurePrivateAccessControl(property)
property.ensurePrivateAccessControl()
}
}
extension Declaration {
/// Validates that a test type (class/struct) or its initializer has the required access control.
func ensureTestDeclarationAccessControl(visibility: Visibility) {
// If marked as open, leave it as is
if modifiers.contains("open") {
return
}
ensureAccessControl(visibility: visibility)
}
/// Removes ACL modifiers from a declaration, except for the specified exceptions.
func removeACLModifiers(from declaration: Declaration, except exceptions: [String]) {
/// Ensures this declaration has the specified access control level.
func ensureAccessControl(visibility: Visibility) {
// internal is the default (implicit) visibility in Swift
if visibility == .internal {
// Remove any explicit non-internal, non-open ACL modifiers
removeACLModifiers(except: ["internal", "open"])
return
}
// If already at the right visibility, do nothing
if modifiers.contains(visibility.rawValue) {
return
}
// Look for an existing ACL modifier to replace
for aclModifier in _FormatRules.aclModifiers where aclModifier != "open" {
if let modifierIndex = formatter.indexOfModifier(aclModifier, forDeclarationAt: keywordIndex) {
formatter.replaceToken(at: modifierIndex, with: .keyword(visibility.rawValue))
return
}
}
// No ACL modifier exists, so add the visibility before the keyword
formatter.insert([.keyword(visibility.rawValue), .space(" ")], at: keywordIndex)
}
/// Removes ACL modifiers from this declaration, except for the specified exceptions.
func removeACLModifiers(except exceptions: [String]) {
for aclModifier in _FormatRules.aclModifiers where !exceptions.contains(aclModifier) {
if let modifierIndex = indexOfModifier(aclModifier, forDeclarationAt: declaration.keywordIndex) {
if let modifierIndex = formatter.indexOfModifier(aclModifier, forDeclarationAt: keywordIndex) {
// Remove the modifier and its trailing space
if let nextIndex = index(of: .nonSpace, after: modifierIndex), nextIndex > modifierIndex + 1 {
removeTokens(in: modifierIndex ... (modifierIndex + 1))
if let nextIndex = formatter.index(of: .nonSpace, after: modifierIndex), nextIndex > modifierIndex + 1 {
formatter.removeTokens(in: modifierIndex ... (modifierIndex + 1))
} else {
removeToken(at: modifierIndex)
formatter.removeToken(at: modifierIndex)
}
}
}
}
/// Ensures a declaration has private access control.
func ensurePrivateAccessControl(_ declaration: Declaration) {
let modifiers = declaration.modifiers
/// Ensures this declaration has private access control.
func ensurePrivateAccessControl() {
let modifiers = modifiers
// If already private, do nothing
if modifiers.contains("private") || modifiers.contains("fileprivate") {
@@ -191,14 +219,14 @@ extension Formatter {
// Remove any existing ACL modifier
for aclModifier in _FormatRules.aclModifiers {
if let modifierIndex = indexOfModifier(aclModifier, forDeclarationAt: declaration.keywordIndex) {
if let modifierIndex = formatter.indexOfModifier(aclModifier, forDeclarationAt: keywordIndex) {
// Replace the modifier with "private"
replaceToken(at: modifierIndex, with: .keyword("private"))
formatter.replaceToken(at: modifierIndex, with: .keyword("private"))
return
}
}
// No ACL modifier exists, so add "private" before the keyword
insert([.keyword("private"), .space(" ")], at: declaration.keywordIndex)
formatter.insert([.keyword("private"), .space(" ")], at: keywordIndex)
}
}
@@ -648,4 +648,170 @@ final class TestSuiteAccessControlTests: XCTestCase {
// No changes should be made - types with parameterized init are not test suites
testFormatting(for: input, rule: .testSuiteAccessControl, exclude: [.unusedArguments, .validateTestCases, .redundantMemberwiseInit])
}
func testSwiftTestingPrivateVisibilityOption() {
let input = """
import Testing
struct MyFeatureTests {
@Test func featureWorks() {
#expect(true)
}
func helperMethod() {
// helper code
}
}
"""
let output = """
import Testing
private struct MyFeatureTests {
@Test private func featureWorks() {
#expect(true)
}
private func helperMethod() {
// helper code
}
}
"""
let options = FormatOptions(testCaseAccessControl: .private)
testFormatting(for: input, output, rule: .testSuiteAccessControl, options: options, exclude: [.unusedArguments])
}
func testSwiftTestingPublicVisibilityOption() {
let input = """
import Testing
struct MyFeatureTests {
@Test func featureWorks() {
#expect(true)
}
func helperMethod() {
// helper code
}
}
"""
let output = """
import Testing
public struct MyFeatureTests {
@Test public func featureWorks() {
#expect(true)
}
private func helperMethod() {
// helper code
}
}
"""
let options = FormatOptions(testCaseAccessControl: .public)
testFormatting(for: input, output, rule: .testSuiteAccessControl, options: options, exclude: [.unusedArguments])
}
func testXCTestPrivateVisibilityFallsBackToInternal() {
let input = """
import XCTest
final class MyTests: XCTestCase {
public func testExample() {
XCTAssertTrue(true)
}
func helperMethod() {
// helper code
}
}
"""
let output = """
import XCTest
final class MyTests: XCTestCase {
func testExample() {
XCTAssertTrue(true)
}
private func helperMethod() {
// helper code
}
}
"""
// XCTest doesn't support private tests, so private/fileprivate falls back to internal
let options = FormatOptions(testCaseAccessControl: .private)
testFormatting(for: input, output, rule: .testSuiteAccessControl, options: options, exclude: [.unusedArguments])
}
func testXCTestFileprivateVisibilityFallsBackToInternal() {
let input = """
import XCTest
final class MyTests: XCTestCase {
public func testExample() {
XCTAssertTrue(true)
}
}
"""
let output = """
import XCTest
final class MyTests: XCTestCase {
func testExample() {
XCTAssertTrue(true)
}
}
"""
// XCTest doesn't support fileprivate tests, so fileprivate falls back to internal
let options = FormatOptions(testCaseAccessControl: .fileprivate)
testFormatting(for: input, output, rule: .testSuiteAccessControl, options: options, exclude: [.unusedArguments])
}
func testSwiftTestingFileprivateVisibilityOption() {
let input = """
import Testing
struct MyFeatureTests {
@Test func featureWorks() {
#expect(true)
}
}
"""
let output = """
import Testing
fileprivate struct MyFeatureTests {
@Test fileprivate func featureWorks() {
#expect(true)
}
}
"""
let options = FormatOptions(testCaseAccessControl: .fileprivate)
testFormatting(for: input, output, rule: .testSuiteAccessControl, options: options, exclude: [.unusedArguments])
}
func testSwiftTestingPrivateVisibilityPreservesExistingPrivate() {
let input = """
import Testing
private struct MyFeatureTests {
@Test private func featureWorks() {
#expect(true)
}
}
"""
let options = FormatOptions(testCaseAccessControl: .private)
testFormatting(for: input, rule: .testSuiteAccessControl, options: options, exclude: [.unusedArguments])
}
}