Skip to content

Commit 7d45ac2

Browse files
allevatoallevato
authored
Merge pull request swiftlang#137 from allevato/fileprivate-rule
Add `FileprivateAtFileScope` rule.
2 parents 000b9d5 + 24878f5 commit 7d45ac2

File tree

5 files changed

+371
-0
lines changed

5 files changed

+371
-0
lines changed

‎Sources/SwiftFormat/Pipelines+Generated.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ class LintPipeline: SyntaxVisitor {
206206

207207
override func visit(_ node: SourceFileSyntax) -> SyntaxVisitorContinueKind {
208208
visitIfEnabled(AmbiguousTrailingClosureOverload.visit, in: context, for: node)
209+
visitIfEnabled(FileprivateAtFileScope.visit, in: context, for: node)
209210
visitIfEnabled(NeverForceUnwrap.visit, in: context, for: node)
210211
visitIfEnabled(NeverUseForceTry.visit, in: context, for: node)
211212
visitIfEnabled(NeverUseImplicitlyUnwrappedOptionals.visit, in: context, for: node)
@@ -286,6 +287,7 @@ extension FormatPipeline {
286287
func visit(_ node: Syntax) -> Syntax {
287288
var node = node
288289
node = DoNotUseSemicolons(context: context).visit(node)
290+
node = FileprivateAtFileScope(context: context).visit(node)
289291
node = FullyIndirectEnum(context: context).visit(node)
290292
node = GroupNumericLiterals(context: context).visit(node)
291293
node = NoAccessLevelOnExtensionDeclaration(context: context).visit(node)

‎Sources/SwiftFormatConfiguration/RuleRegistry+Generated.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ enum RuleRegistry {
2020
"BeginDocumentationCommentWithOneLineSummary": true,
2121
"DoNotUseSemicolons": true,
2222
"DontRepeatTypeInStaticProperties": true,
23+
"FileprivateAtFileScope": true,
2324
"FullyIndirectEnum": true,
2425
"GroupNumericLiterals": true,
2526
"IdentifiersMustBeASCII": true,

‎Sources/SwiftFormatRules/FileprivateAtFileScope.swift

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import SwiftFormatCore
14+
import SwiftSyntax
15+
16+
/// Declarations at file scope should be declared `fileprivate`, not `private`.
17+
///
18+
/// Using `private` at file scope actually gives the declaration `fileprivate` visibility, so using
19+
/// `fileprivate` explicitly is a better indicator of intent.
20+
///
21+
/// Lint: If a file-scoped declaration has `private` visibility, a lint error is raised.
22+
///
23+
/// Format: File-scoped declarations that have `private` visibility will have their visibility
24+
/// changed to `fileprivate`.
25+
public final class FileprivateAtFileScope: SyntaxFormatRule {
26+
public override func visit(_ node: SourceFileSyntax) -> Syntax {
27+
let newStatements = rewrittenCodeBlockItems(node.statements)
28+
return Syntax(node.withStatements(newStatements))
29+
}
30+
31+
/// Returns a list of code block items equivalent to the given list, but where any file-scoped
32+
/// declarations have had their `private` modifier replaced by `fileprivate` if present.
33+
///
34+
/// - Parameter codeBlockItems: The list of code block items to rewrite.
35+
/// - Returns: A new `CodeBlockItemListSyntax` that has possibly been rewritten.
36+
private func rewrittenCodeBlockItems(_ codeBlockItems: CodeBlockItemListSyntax)
37+
-> CodeBlockItemListSyntax
38+
{
39+
let newCodeBlockItems = codeBlockItems.map { codeBlockItem -> CodeBlockItemSyntax in
40+
switch codeBlockItem.item.as(SyntaxEnum.self) {
41+
case .ifConfigDecl(let ifConfigDecl):
42+
// We need to look through `#if/#elseif/#else` blocks because the decls directly inside
43+
// them are still considered file-scope for our purposes.
44+
return codeBlockItem.withItem(Syntax(rewrittenIfConfigDecl(ifConfigDecl)))
45+
46+
case .functionDecl(let functionDecl):
47+
return codeBlockItem.withItem(
48+
Syntax(rewrittenDecl(
49+
functionDecl,
50+
modifiers: functionDecl.modifiers,
51+
factory: functionDecl.withModifiers)))
52+
53+
case .variableDecl(let variableDecl):
54+
return codeBlockItem.withItem(
55+
Syntax(rewrittenDecl(
56+
variableDecl,
57+
modifiers: variableDecl.modifiers,
58+
factory: variableDecl.withModifiers)))
59+
60+
case .classDecl(let classDecl):
61+
return codeBlockItem.withItem(
62+
Syntax(rewrittenDecl(
63+
classDecl,
64+
modifiers: classDecl.modifiers,
65+
factory: classDecl.withModifiers)))
66+
67+
case .structDecl(let structDecl):
68+
return codeBlockItem.withItem(
69+
Syntax(rewrittenDecl(
70+
structDecl,
71+
modifiers: structDecl.modifiers,
72+
factory: structDecl.withModifiers)))
73+
74+
case .enumDecl(let enumDecl):
75+
return codeBlockItem.withItem(
76+
Syntax(rewrittenDecl(
77+
enumDecl,
78+
modifiers: enumDecl.modifiers,
79+
factory: enumDecl.withModifiers)))
80+
81+
case .protocolDecl(let protocolDecl):
82+
return codeBlockItem.withItem(
83+
Syntax(rewrittenDecl(
84+
protocolDecl,
85+
modifiers: protocolDecl.modifiers,
86+
factory: protocolDecl.withModifiers)))
87+
88+
case .typealiasDecl(let typealiasDecl):
89+
return codeBlockItem.withItem(
90+
Syntax(rewrittenDecl(
91+
typealiasDecl,
92+
modifiers: typealiasDecl.modifiers,
93+
factory: typealiasDecl.withModifiers)))
94+
95+
case .extensionDecl(let extensionDecl):
96+
return codeBlockItem.withItem(
97+
Syntax(rewrittenDecl(
98+
extensionDecl,
99+
modifiers: extensionDecl.modifiers,
100+
factory: extensionDecl.withModifiers)))
101+
102+
default:
103+
return codeBlockItem
104+
}
105+
}
106+
return SyntaxFactory.makeCodeBlockItemList(newCodeBlockItems)
107+
}
108+
109+
/// Returns a new `IfConfigDeclSyntax` equivalent to the given node, but where any file-scoped
110+
/// declarations have had their `private` modifier replaced by `fileprivate` if present.
111+
///
112+
/// - Parameter ifConfigDecl: The `IfConfigDeclSyntax` to rewrite.
113+
/// - Returns: A new `IfConfigDeclSyntax` that has possibly been rewritten.
114+
private func rewrittenIfConfigDecl(_ ifConfigDecl: IfConfigDeclSyntax) -> IfConfigDeclSyntax {
115+
let newClauses = ifConfigDecl.clauses.map { clause -> IfConfigClauseSyntax in
116+
switch clause.elements.as(SyntaxEnum.self) {
117+
case .codeBlockItemList(let codeBlockItemList):
118+
return clause.withElements(Syntax(rewrittenCodeBlockItems(codeBlockItemList)))
119+
default:
120+
return clause
121+
}
122+
}
123+
return ifConfigDecl.withClauses(SyntaxFactory.makeIfConfigClauseList(newClauses))
124+
}
125+
126+
/// Returns a rewritten version of the given declaration if its modifier list contains `private`
127+
/// that contains `fileprivate` instead.
128+
///
129+
/// If the modifier list does not contain `private`, the original declaration is returned
130+
/// unchanged.
131+
///
132+
/// - Parameters:
133+
/// - decl: The declaration to possibly rewrite.
134+
/// - modifiers: The modifier list of the declaration (i.e., `decl.modifiers`).
135+
/// - factory: A reference to the `decl`'s `withModifiers` instance method that is called to
136+
/// rewrite the node if needed.
137+
/// - Returns: A new node if the modifiers were rewritten, or the original node if not.
138+
private func rewrittenDecl<DeclType: DeclSyntaxProtocol>(
139+
_ decl: DeclType,
140+
modifiers: ModifierListSyntax?,
141+
factory: (ModifierListSyntax?) -> DeclType
142+
) -> DeclType {
143+
guard let modifiers = modifiers, modifiers.has(modifier: "private") else {
144+
return decl
145+
}
146+
147+
let newModifiers = modifiers.map { modifier -> DeclModifierSyntax in
148+
let name = modifier.name
149+
if name.tokenKind == .privateKeyword {
150+
diagnose(.replacePrivateWithFileprivate, on: name)
151+
return modifier.withName(name.withKind(.fileprivateKeyword))
152+
}
153+
return modifier
154+
}
155+
return factory(SyntaxFactory.makeModifierList(newModifiers))
156+
}
157+
}
158+
159+
extension Diagnostic.Message {
160+
public static let replacePrivateWithFileprivate =
161+
Diagnostic.Message(.warning, "replace 'private' with 'fileprivate' on file-scoped declarations")
162+
}

‎Tests/SwiftFormatRulesTests/FileprivateAtFileScopeTests.swift

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
import SwiftFormatRules
2+
3+
final class FileprivateAtFileScopeTests: DiagnosingTestCase {
4+
func testFileScopeDecls() {
5+
XCTAssertFormatting(
6+
FileprivateAtFileScope.self,
7+
input: """
8+
private class Foo {}
9+
private struct Foo {}
10+
private enum Foo {}
11+
private protocol Foo {}
12+
private typealias Foo = Bar
13+
private extension Foo {}
14+
private func foo() {}
15+
private var foo: Bar
16+
""",
17+
expected: """
18+
fileprivate class Foo {}
19+
fileprivate struct Foo {}
20+
fileprivate enum Foo {}
21+
fileprivate protocol Foo {}
22+
fileprivate typealias Foo = Bar
23+
fileprivate extension Foo {}
24+
fileprivate func foo() {}
25+
fileprivate var foo: Bar
26+
""")
27+
XCTAssertDiagnosed(.replacePrivateWithFileprivate)
28+
XCTAssertDiagnosed(.replacePrivateWithFileprivate)
29+
XCTAssertDiagnosed(.replacePrivateWithFileprivate)
30+
XCTAssertDiagnosed(.replacePrivateWithFileprivate)
31+
XCTAssertDiagnosed(.replacePrivateWithFileprivate)
32+
XCTAssertDiagnosed(.replacePrivateWithFileprivate)
33+
XCTAssertDiagnosed(.replacePrivateWithFileprivate)
34+
XCTAssertDiagnosed(.replacePrivateWithFileprivate)
35+
}
36+
37+
func testNonFileScopeDeclsAreNotChanged() {
38+
XCTAssertFormatting(
39+
FileprivateAtFileScope.self,
40+
input: """
41+
enum Namespace {
42+
private class Foo {}
43+
private struct Foo {}
44+
private enum Foo {}
45+
private typealias Foo = Bar
46+
private static func foo() {}
47+
private static var foo: Bar
48+
}
49+
""",
50+
expected: """
51+
enum Namespace {
52+
private class Foo {}
53+
private struct Foo {}
54+
private enum Foo {}
55+
private typealias Foo = Bar
56+
private static func foo() {}
57+
private static var foo: Bar
58+
}
59+
""")
60+
XCTAssertNotDiagnosed(.replacePrivateWithFileprivate)
61+
}
62+
63+
func testFileScopeDeclsInsideConditionals() {
64+
XCTAssertFormatting(
65+
FileprivateAtFileScope.self,
66+
input: """
67+
#if FOO
68+
private class Foo {}
69+
private struct Foo {}
70+
private enum Foo {}
71+
private protocol Foo {}
72+
private typealias Foo = Bar
73+
private extension Foo {}
74+
private func foo() {}
75+
private var foo: Bar
76+
#elseif BAR
77+
private class Foo {}
78+
private struct Foo {}
79+
private enum Foo {}
80+
private protocol Foo {}
81+
private typealias Foo = Bar
82+
private extension Foo {}
83+
private func foo() {}
84+
private var foo: Bar
85+
#else
86+
private class Foo {}
87+
private struct Foo {}
88+
private enum Foo {}
89+
private protocol Foo {}
90+
private typealias Foo = Bar
91+
private extension Foo {}
92+
private func foo() {}
93+
private var foo: Bar
94+
#endif
95+
""",
96+
expected: """
97+
#if FOO
98+
fileprivate class Foo {}
99+
fileprivate struct Foo {}
100+
fileprivate enum Foo {}
101+
fileprivate protocol Foo {}
102+
fileprivate typealias Foo = Bar
103+
fileprivate extension Foo {}
104+
fileprivate func foo() {}
105+
fileprivate var foo: Bar
106+
#elseif BAR
107+
fileprivate class Foo {}
108+
fileprivate struct Foo {}
109+
fileprivate enum Foo {}
110+
fileprivate protocol Foo {}
111+
fileprivate typealias Foo = Bar
112+
fileprivate extension Foo {}
113+
fileprivate func foo() {}
114+
fileprivate var foo: Bar
115+
#else
116+
fileprivate class Foo {}
117+
fileprivate struct Foo {}
118+
fileprivate enum Foo {}
119+
fileprivate protocol Foo {}
120+
fileprivate typealias Foo = Bar
121+
fileprivate extension Foo {}
122+
fileprivate func foo() {}
123+
fileprivate var foo: Bar
124+
#endif
125+
""")
126+
}
127+
128+
func testFileScopeDeclsInsideNestedConditionals() {
129+
XCTAssertFormatting(
130+
FileprivateAtFileScope.self,
131+
input: """
132+
#if FOO
133+
#if BAR
134+
private class Foo {}
135+
private struct Foo {}
136+
private enum Foo {}
137+
private protocol Foo {}
138+
private typealias Foo = Bar
139+
private extension Foo {}
140+
private func foo() {}
141+
private var foo: Bar
142+
#endif
143+
#endif
144+
""",
145+
expected: """
146+
#if FOO
147+
#if BAR
148+
fileprivate class Foo {}
149+
fileprivate struct Foo {}
150+
fileprivate enum Foo {}
151+
fileprivate protocol Foo {}
152+
fileprivate typealias Foo = Bar
153+
fileprivate extension Foo {}
154+
fileprivate func foo() {}
155+
fileprivate var foo: Bar
156+
#endif
157+
#endif
158+
""")
159+
}
160+
161+
func testLeadingTriviaIsPreserved() {
162+
XCTAssertFormatting(
163+
FileprivateAtFileScope.self,
164+
input: """
165+
/// Some doc comment
166+
private class Foo {}
167+
168+
@objc /* comment */ private class Bar {}
169+
""",
170+
expected: """
171+
/// Some doc comment
172+
fileprivate class Foo {}
173+
174+
@objc /* comment */ fileprivate class Bar {}
175+
""")
176+
XCTAssertDiagnosed(.replacePrivateWithFileprivate)
177+
XCTAssertDiagnosed(.replacePrivateWithFileprivate)
178+
}
179+
180+
func testModifierDetailIsPreserved() {
181+
XCTAssertFormatting(
182+
FileprivateAtFileScope.self,
183+
input: """
184+
public private(set) var foo: Int
185+
""",
186+
expected: """
187+
public fileprivate(set) var foo: Int
188+
""")
189+
XCTAssertDiagnosed(.replacePrivateWithFileprivate)
190+
}
191+
}

0 commit comments

Comments
 (0)