Merge pull request swiftlang#152 from allevato/fileprivate-configurable

Make the `FileprivateAtFileScope` rule configurable.

Author: allevato

Sources/SwiftFormat/Pipelines+Generated.swift

@@ -206,7 +206,7 @@ class LintPipeline: SyntaxVisitor {
 
   override func visit(_ node: SourceFileSyntax) -> SyntaxVisitorContinueKind {
     visitIfEnabled(AmbiguousTrailingClosureOverload.visit, in: context, for: node)
-    visitIfEnabled(FileprivateAtFileScope.visit, in: context, for: node)
+    visitIfEnabled(FileScopedDeclarationPrivacy.visit, in: context, for: node)
     visitIfEnabled(NeverForceUnwrap.visit, in: context, for: node)
     visitIfEnabled(NeverUseForceTry.visit, in: context, for: node)
     visitIfEnabled(NeverUseImplicitlyUnwrappedOptionals.visit, in: context, for: node)
@@ -286,7 +286,7 @@ extension FormatPipeline {
   func visit(_ node: Syntax) -> Syntax {
     var node = node
     node = DoNotUseSemicolons(context: context).visit(node)
-    node = FileprivateAtFileScope(context: context).visit(node)
+    node = FileScopedDeclarationPrivacy(context: context).visit(node)
     node = FullyIndirectEnum(context: context).visit(node)
     node = GroupNumericLiterals(context: context).visit(node)
     node = NoAccessLevelOnExtensionDeclaration(context: context).visit(node)

Sources/SwiftFormatConfiguration/Configuration.swift

@@ -32,6 +32,7 @@ public struct Configuration: Codable, Equatable {
     case prioritizeKeepingFunctionOutputTogether
     case indentConditionalCompilationBlocks
     case lineBreakAroundMultilineExpressionChainComponents
+    case fileScopedDeclarationPrivacy
     case rules
   }
 
@@ -115,6 +116,10 @@ public struct Configuration: Codable, Equatable {
   /// (i.e. right paren, right bracket, right brace, etc.).
   public var lineBreakAroundMultilineExpressionChainComponents = false
 
+  /// Determines the formal access level (i.e., the level specified in source code) for file-scoped
+  /// declarations whose effective access level is private to the containing file.
+  public var fileScopedDeclarationPrivacy = FileScopedDeclarationPrivacyConfiguration()
+
   /// Constructs a Configuration with all default values.
   public init() {
     self.version = highestSupportedConfigurationVersion
@@ -167,6 +172,10 @@ public struct Configuration: Codable, Equatable {
     self.lineBreakAroundMultilineExpressionChainComponents =
       try container.decodeIfPresent(
         Bool.self, forKey: .lineBreakAroundMultilineExpressionChainComponents) ?? false
+    self.fileScopedDeclarationPrivacy =
+      try container.decodeIfPresent(
+        FileScopedDeclarationPrivacyConfiguration.self, forKey: .fileScopedDeclarationPrivacy)
+      ?? FileScopedDeclarationPrivacyConfiguration()
 
     // If the `rules` key is not present at all, default it to the built-in set
     // so that the behavior is the same as if the configuration had been
@@ -193,6 +202,7 @@ public struct Configuration: Codable, Equatable {
     try container.encode(
       lineBreakAroundMultilineExpressionChainComponents,
       forKey: .lineBreakAroundMultilineExpressionChainComponents)
+    try container.encode(fileScopedDeclarationPrivacy, forKey: .fileScopedDeclarationPrivacy)
     try container.encode(rules, forKey: .rules)
   }
 
@@ -234,3 +244,24 @@ public struct NoPlaygroundLiteralsConfiguration: Codable, Equatable {
   /// Resolution behavior to use when encountering an ambiguous `#colorLiteral`.
   public let resolveAmbiguousColor: ResolveBehavior = .useUIColor
 }
+
+/// Configuration for the `FileScopedDeclarationPrivacy` rule.
+public struct FileScopedDeclarationPrivacyConfiguration: Codable, Equatable {
+  public enum AccessLevel: String, Codable {
+    /// Private file-scoped declarations should be declared `private`.
+    ///
+    /// If a file-scoped declaration is declared `fileprivate`, it will be diagnosed (in lint mode)
+    /// or changed to `private` (in format mode).
+    case `private`
+
+    /// Private file-scoped declarations should be declared `fileprivate`.
+    ///
+    /// If a file-scoped declaration is declared `private`, it will be diagnosed (in lint mode) or
+    /// changed to `fileprivate` (in format mode).
+    case `fileprivate`
+  }
+
+  /// The formal access level to use when encountering a file-scoped declaration with effective
+  /// private access.
+  public var accessLevel: AccessLevel = .private
+}

Sources/SwiftFormatRules/FileprivateAtFileScope.swift renamed to Sources/SwiftFormatRules/FileScopedDeclarationPrivacy.swift

@@ -13,23 +13,24 @@
 import SwiftFormatCore
 import SwiftSyntax
 
-/// Declarations at file scope should be declared `fileprivate`, not `private`.
+/// Declarations at file scope with effective private access should be consistently declared as
+/// either `fileprivate` or `private`, determined by configuration.
 ///
-/// Using `private` at file scope actually gives the declaration `fileprivate` visibility, so using
-/// `fileprivate` explicitly is a better indicator of intent.
+/// Lint: If a file-scoped declaration has formal access opposite to the desired access level in the
+///       formatter's configuration, a lint error is raised.
 ///
-/// Lint: If a file-scoped declaration has `private` visibility, a lint error is raised.
-///
-/// Format: File-scoped declarations that have `private` visibility will have their visibility
-///         changed to `fileprivate`.
-public final class FileprivateAtFileScope: SyntaxFormatRule {
+/// Format: File-scoped declarations that have formal access opposite to the desired access level in
+///         the formatter's configuration will have their access level changed.
+public final class FileScopedDeclarationPrivacy: SyntaxFormatRule {
   public override func visit(_ node: SourceFileSyntax) -> Syntax {
     let newStatements = rewrittenCodeBlockItems(node.statements)
     return Syntax(node.withStatements(newStatements))
   }
 
   /// Returns a list of code block items equivalent to the given list, but where any file-scoped
-  /// declarations have had their `private` modifier replaced by `fileprivate` if present.
+  /// declarations with effective private access have had their formal access level rewritten, if
+  /// necessary, to be either `private` or `fileprivate`, as determined by the formatter
+  /// configuration.
   ///
   /// - Parameter codeBlockItems: The list of code block items to rewrite.
   /// - Returns: A new `CodeBlockItemListSyntax` that has possibly been rewritten.
@@ -100,7 +101,9 @@ public final class FileprivateAtFileScope: SyntaxFormatRule {
   }
 
   /// Returns a new `IfConfigDeclSyntax` equivalent to the given node, but where any file-scoped
-  /// declarations have had their `private` modifier replaced by `fileprivate` if present.
+  /// declarations with effective private access have had their formal access level rewritten, if
+  /// necessary, to be either `private` or `fileprivate`, as determined by the formatter
+  /// configuration.
   ///
   /// - Parameter ifConfigDecl: The `IfConfigDeclSyntax` to rewrite.
   /// - Returns: A new `IfConfigDeclSyntax` that has possibly been rewritten.
@@ -119,8 +122,8 @@ public final class FileprivateAtFileScope: SyntaxFormatRule {
   /// Returns a rewritten version of the given declaration if its modifier list contains `private`
   /// that contains `fileprivate` instead.
   ///
-  /// If the modifier list does not contain `private`, the original declaration is returned
-  /// unchanged.
+  /// If the modifier list is not inconsistent with the configured access level, the original
+  /// declaration is returned unchanged.
   ///
   /// - Parameters:
   ///   - decl: The declaration to possibly rewrite.
@@ -133,15 +136,30 @@ public final class FileprivateAtFileScope: SyntaxFormatRule {
     modifiers: ModifierListSyntax?,
     factory: (ModifierListSyntax?) -> DeclType
   ) -> DeclType {
-    guard let modifiers = modifiers, modifiers.has(modifier: "private") else {
+    let invalidAccess: TokenKind
+    let validAccess: TokenKind
+    let diagnostic: Diagnostic.Message
+
+    switch context.configuration.fileScopedDeclarationPrivacy.accessLevel {
+    case .private:
+      invalidAccess = .fileprivateKeyword
+      validAccess = .privateKeyword
+      diagnostic = .replaceFileprivateWithPrivate
+    case .fileprivate:
+      invalidAccess = .privateKeyword
+      validAccess = .fileprivateKeyword
+      diagnostic = .replacePrivateWithFileprivate
+    }
+
+    guard let modifiers = modifiers, modifiers.has(modifier: invalidAccess) else {
       return decl
     }
 
     let newModifiers = modifiers.map { modifier -> DeclModifierSyntax in
       let name = modifier.name
-      if name.tokenKind == .privateKeyword {
-        diagnose(.replacePrivateWithFileprivate, on: name)
-        return modifier.withName(name.withKind(.fileprivateKeyword))
+      if name.tokenKind == invalidAccess {
+        diagnose(diagnostic, on: name)
+        return modifier.withName(name.withKind(validAccess))
       }
       return modifier
     }
@@ -152,4 +170,7 @@ public final class FileprivateAtFileScope: SyntaxFormatRule {
 extension Diagnostic.Message {
   public static let replacePrivateWithFileprivate =
     Diagnostic.Message(.warning, "replace 'private' with 'fileprivate' on file-scoped declarations")
+
+  public static let replaceFileprivateWithPrivate =
+    Diagnostic.Message(.warning, "replace 'fileprivate' with 'private' on file-scoped declarations")
 }

Sources/SwiftFormatRules/ModifierListSyntax+Convenience.swift

@@ -18,6 +18,10 @@ extension ModifierListSyntax {
     return contains { $0.name.text == modifier }
   }
 
+  func has(modifier: TokenKind) -> Bool {
+    return contains { $0.name.tokenKind == modifier }
+  }
+
   /// Returns the declaration's access level modifier, if present.
   var accessLevelModifier: DeclModifierSyntax? {
     for modifier in self {