Skip to content

Commit 88a8900

Browse files
allevatoallevato
authored
Merge pull request #969 from allevato/extension-access
Fix `NoAccessLevelOnExtensionDeclaration` to update members inside `#if` blocks.
2 parents f91aaa9 + d708582 commit 88a8900

File tree

2 files changed

+235
-130
lines changed

2 files changed

+235
-130
lines changed

‎Sources/SwiftFormat/Rules/NoAccessLevelOnExtensionDeclaration.swift

+161-130
Original file line numberDiff line numberDiff line change
@@ -21,88 +21,197 @@ import SwiftSyntax
2121
/// `internal`, as that is the default access level) have the explicit access level removed.
2222
@_spi(Rules)
2323
public final class NoAccessLevelOnExtensionDeclaration: SyntaxFormatRule {
24+
private enum State {
25+
/// The rule is currently visiting top-level declarations.
26+
case topLevel
27+
28+
/// The rule is currently inside an extension that has the given access level keyword.
29+
case insideExtension(accessKeyword: Keyword)
30+
}
31+
32+
/// Tracks the state of the rule to determine which action should be taken on visited
33+
/// declarations.
34+
private var state: State = .topLevel
35+
36+
/// Findings propagated up to the extension visitor from any members that were rewritten.
37+
private var notesFromRewrittenMembers: [Finding.Note] = []
38+
2439
public override func visit(_ node: ExtensionDeclSyntax) -> DeclSyntax {
2540
guard
41+
// Skip nested extensions; these are semantic errors but they still parse successfully.
42+
case .topLevel = state,
43+
// Skip extensions that don't have an access level modifier.
2644
let accessKeyword = node.modifiers.accessLevelModifier,
2745
case .keyword(let keyword) = accessKeyword.name.tokenKind
2846
else {
2947
return DeclSyntax(node)
3048
}
3149

32-
var result = node
50+
self.notesFromRewrittenMembers = []
51+
52+
let keywordToAdd: Keyword?
53+
let message: Finding.Message
3354

3455
switch keyword {
35-
// Public, private, fileprivate, or package keywords need to be moved to members
3656
case .public, .private, .fileprivate, .package:
37-
// The effective access level of the members of a `private` extension is `fileprivate`, so
38-
// we have to update the keyword to ensure that the result is correct.
39-
var accessKeywordToAdd = accessKeyword
40-
let message: Finding.Message
57+
// These access level modifiers need to be moved to members. Additionally, `private` is a
58+
// special case, because the *effective* access level for a top-level private extension is
59+
// `fileprivate`, so we need to preserve that when we apply it to the members.
4160
if keyword == .private {
42-
accessKeywordToAdd.name.tokenKind = .keyword(.fileprivate)
61+
keywordToAdd = .fileprivate
4362
message = .moveAccessKeywordAndMakeFileprivate(keyword: accessKeyword.name.text)
4463
} else {
64+
keywordToAdd = keyword
4565
message = .moveAccessKeyword(keyword: accessKeyword.name.text)
4666
}
4767

48-
let (newMembers, notes) =
49-
addMemberAccessKeyword(accessKeywordToAdd, toMembersIn: node.memberBlock)
50-
diagnose(message, on: accessKeyword, notes: notes)
51-
52-
result.modifiers.remove(anyOf: [keyword])
53-
result.extensionKeyword.leadingTrivia = accessKeyword.leadingTrivia
54-
result.memberBlock.members = newMembers
55-
return DeclSyntax(result)
56-
57-
// Internal keyword redundant, delete
5868
case .internal:
59-
diagnose(.removeRedundantAccessKeyword, on: accessKeyword)
60-
61-
result.modifiers.remove(anyOf: [keyword])
62-
result.extensionKeyword.leadingTrivia = accessKeyword.leadingTrivia
63-
return DeclSyntax(result)
69+
// If the access level keyword was `internal`, then it's redundant and we can just remove it.
70+
// We don't need to modify the members at all in this case.
71+
message = .removeRedundantAccessKeyword
72+
keywordToAdd = nil
6473

6574
default:
66-
break
75+
// For anything else, just return the extension and its members unchanged.
76+
return DeclSyntax(node)
77+
}
78+
79+
// We don't have to worry about maintaining a stack here; even though extensions can nest from
80+
// a valid parse point of view, we ignore nested extensions because they're obviously wrong
81+
// semantically (and would be an error later during compilation).
82+
var result: ExtensionDeclSyntax
83+
if let keywordToAdd {
84+
// Visit the children in the new state to add the keyword to the extension members.
85+
self.state = .insideExtension(accessKeyword: keywordToAdd)
86+
defer { self.state = .topLevel }
87+
88+
result = super.visit(node).as(ExtensionDeclSyntax.self)!
89+
} else {
90+
// We don't need to visit the children in this case, and we don't need to update the state.
91+
result = node
6792
}
6893

94+
// Finally, emit the finding (which includes notes from any rewritten members) and remove the
95+
// access level keyword from the extension itself.
96+
diagnose(message, on: accessKeyword, notes: self.notesFromRewrittenMembers)
97+
result.modifiers.remove(anyOf: [keyword])
98+
result.extensionKeyword.leadingTrivia = accessKeyword.leadingTrivia
6999
return DeclSyntax(result)
70100
}
71101

72-
// Adds given keyword to all members in declaration block
73-
private func addMemberAccessKeyword(
74-
_ modifier: DeclModifierSyntax,
75-
toMembersIn memberBlock: MemberBlockSyntax
76-
) -> (MemberBlockItemListSyntax, [Finding.Note]) {
77-
var newMembers: [MemberBlockItemSyntax] = []
78-
var notes: [Finding.Note] = []
79-
80-
for memberItem in memberBlock.members {
81-
let decl = memberItem.decl
82-
guard
83-
let modifiers = decl.asProtocol(WithModifiersSyntax.self)?.modifiers,
84-
modifiers.accessLevelModifier == nil
85-
else {
86-
newMembers.append(memberItem)
87-
continue
88-
}
102+
public override func visit(_ node: ActorDeclSyntax) -> DeclSyntax {
103+
return applyingAccessModifierIfNone(to: node)
104+
}
105+
106+
public override func visit(_ node: ClassDeclSyntax) -> DeclSyntax {
107+
return applyingAccessModifierIfNone(to: node)
108+
}
109+
110+
public override func visit(_ node: EnumDeclSyntax) -> DeclSyntax {
111+
return applyingAccessModifierIfNone(to: node)
112+
}
113+
114+
public override func visit(_ node: FunctionDeclSyntax) -> DeclSyntax {
115+
return applyingAccessModifierIfNone(to: node)
116+
}
117+
118+
public override func visit(_ node: InitializerDeclSyntax) -> DeclSyntax {
119+
return applyingAccessModifierIfNone(to: node)
120+
}
121+
122+
public override func visit(_ node: StructDeclSyntax) -> DeclSyntax {
123+
return applyingAccessModifierIfNone(to: node)
124+
}
125+
126+
public override func visit(_ node: SubscriptDeclSyntax) -> DeclSyntax {
127+
return applyingAccessModifierIfNone(to: node)
128+
}
89129

90-
// Create a note associated with each declaration that needs to have an access level modifier
91-
// added to it.
92-
notes.append(
93-
Finding.Note(
94-
message: .addModifierToExtensionMember(keyword: modifier.name.text),
95-
location:
96-
Finding.Location(decl.startLocation(converter: context.sourceLocationConverter))
97-
)
130+
public override func visit(_ node: TypeAliasDeclSyntax) -> DeclSyntax {
131+
return applyingAccessModifierIfNone(to: node)
132+
}
133+
134+
public override func visit(_ node: VariableDeclSyntax) -> DeclSyntax {
135+
return applyingAccessModifierIfNone(to: node)
136+
}
137+
138+
/// Adds `modifier` to `decl` if it doesn't already have an explicit access level modifier and
139+
/// returns the new declaration.
140+
///
141+
/// If `decl` already has an access level modifier, it is returned unchanged.
142+
private func applyingAccessModifierIfNone(to decl: some DeclSyntaxProtocol) -> DeclSyntax {
143+
// Only go further if we are applying an access level keyword and if the decl is one that
144+
// allows modifiers but doesn't already have an access level modifier.
145+
guard
146+
case .insideExtension(let accessKeyword) = state,
147+
let modifiers = decl.asProtocol(WithModifiersSyntax.self)?.modifiers,
148+
modifiers.accessLevelModifier == nil
149+
else {
150+
return DeclSyntax(decl)
151+
}
152+
153+
// Create a note associated with each declaration that needs to have an access level modifier
154+
// added to it.
155+
self.notesFromRewrittenMembers.append(
156+
Finding.Note(
157+
message: .addModifierToExtensionMember(keyword: TokenSyntax.keyword(accessKeyword).text),
158+
location:
159+
Finding.Location(decl.startLocation(converter: context.sourceLocationConverter))
98160
)
161+
)
99162

100-
var newItem = memberItem
101-
newItem.decl = applyingAccessModifierIfNone(modifier, to: decl)
102-
newMembers.append(newItem)
163+
switch Syntax(decl).as(SyntaxEnum.self) {
164+
case .actorDecl(let actorDecl):
165+
return applyingAccessModifierIfNone(accessKeyword, to: actorDecl, declKeywordKeyPath: \.actorKeyword)
166+
case .classDecl(let classDecl):
167+
return applyingAccessModifierIfNone(accessKeyword, to: classDecl, declKeywordKeyPath: \.classKeyword)
168+
case .enumDecl(let enumDecl):
169+
return applyingAccessModifierIfNone(accessKeyword, to: enumDecl, declKeywordKeyPath: \.enumKeyword)
170+
case .initializerDecl(let initDecl):
171+
return applyingAccessModifierIfNone(accessKeyword, to: initDecl, declKeywordKeyPath: \.initKeyword)
172+
case .functionDecl(let funcDecl):
173+
return applyingAccessModifierIfNone(accessKeyword, to: funcDecl, declKeywordKeyPath: \.funcKeyword)
174+
case .structDecl(let structDecl):
175+
return applyingAccessModifierIfNone(accessKeyword, to: structDecl, declKeywordKeyPath: \.structKeyword)
176+
case .subscriptDecl(let subscriptDecl):
177+
return applyingAccessModifierIfNone(accessKeyword, to: subscriptDecl, declKeywordKeyPath: \.subscriptKeyword)
178+
case .typeAliasDecl(let typeAliasDecl):
179+
return applyingAccessModifierIfNone(accessKeyword, to: typeAliasDecl, declKeywordKeyPath: \.typealiasKeyword)
180+
case .variableDecl(let varDecl):
181+
return applyingAccessModifierIfNone(accessKeyword, to: varDecl, declKeywordKeyPath: \.bindingSpecifier)
182+
default:
183+
return DeclSyntax(decl)
184+
}
185+
}
186+
187+
private func applyingAccessModifierIfNone<Decl: DeclSyntaxProtocol & WithModifiersSyntax>(
188+
_ modifier: Keyword,
189+
to decl: Decl,
190+
declKeywordKeyPath: WritableKeyPath<Decl, TokenSyntax>
191+
) -> DeclSyntax {
192+
// If there's already an access modifier among the modifier list, bail out.
193+
guard decl.modifiers.accessLevelModifier == nil else { return DeclSyntax(decl) }
194+
195+
var result = decl
196+
var modifier = DeclModifierSyntax(name: .keyword(modifier))
197+
modifier.trailingTrivia = [.spaces(1)]
198+
199+
guard var firstModifier = decl.modifiers.first else {
200+
// If there are no modifiers at all, add the one being requested, moving the leading trivia
201+
// from the decl keyword to that modifier (to preserve leading comments, newlines, etc.).
202+
modifier.leadingTrivia = decl[keyPath: declKeywordKeyPath].leadingTrivia
203+
result[keyPath: declKeywordKeyPath].leadingTrivia = []
204+
result.modifiers = .init([modifier])
205+
return DeclSyntax(result)
103206
}
104207

105-
return (MemberBlockItemListSyntax(newMembers), notes)
208+
// Otherwise, insert the modifier at the front of the modifier list, moving the (original) first
209+
// modifier's leading trivia to the new one (to preserve leading comments, newlines, etc.).
210+
modifier.leadingTrivia = firstModifier.leadingTrivia
211+
firstModifier.leadingTrivia = []
212+
result.modifiers[result.modifiers.startIndex] = firstModifier
213+
result.modifiers.insert(modifier, at: result.modifiers.startIndex)
214+
return DeclSyntax(result)
106215
}
107216
}
108217

@@ -122,81 +231,3 @@ extension Finding.Message {
122231
"add '\(keyword)' access modifier to this declaration"
123232
}
124233
}
125-
126-
/// Adds `modifier` to `decl` if it doesn't already have an explicit access level modifier and
127-
/// returns the new declaration.
128-
///
129-
/// If `decl` already has an access level modifier, it is returned unchanged.
130-
private func applyingAccessModifierIfNone(
131-
_ modifier: DeclModifierSyntax,
132-
to decl: DeclSyntax
133-
) -> DeclSyntax {
134-
switch Syntax(decl).as(SyntaxEnum.self) {
135-
case .actorDecl(let actorDecl):
136-
return applyingAccessModifierIfNone(modifier, to: actorDecl, declKeywordKeyPath: \.actorKeyword)
137-
case .classDecl(let classDecl):
138-
return applyingAccessModifierIfNone(modifier, to: classDecl, declKeywordKeyPath: \.classKeyword)
139-
case .enumDecl(let enumDecl):
140-
return applyingAccessModifierIfNone(modifier, to: enumDecl, declKeywordKeyPath: \.enumKeyword)
141-
case .initializerDecl(let initDecl):
142-
return applyingAccessModifierIfNone(modifier, to: initDecl, declKeywordKeyPath: \.initKeyword)
143-
case .functionDecl(let funcDecl):
144-
return applyingAccessModifierIfNone(modifier, to: funcDecl, declKeywordKeyPath: \.funcKeyword)
145-
case .structDecl(let structDecl):
146-
return applyingAccessModifierIfNone(
147-
modifier,
148-
to: structDecl,
149-
declKeywordKeyPath: \.structKeyword
150-
)
151-
case .subscriptDecl(let subscriptDecl):
152-
return applyingAccessModifierIfNone(
153-
modifier,
154-
to: subscriptDecl,
155-
declKeywordKeyPath: \.subscriptKeyword
156-
)
157-
case .typeAliasDecl(let typeAliasDecl):
158-
return applyingAccessModifierIfNone(
159-
modifier,
160-
to: typeAliasDecl,
161-
declKeywordKeyPath: \.typealiasKeyword
162-
)
163-
case .variableDecl(let varDecl):
164-
return applyingAccessModifierIfNone(
165-
modifier,
166-
to: varDecl,
167-
declKeywordKeyPath: \.bindingSpecifier
168-
)
169-
default:
170-
return decl
171-
}
172-
}
173-
174-
private func applyingAccessModifierIfNone<Decl: DeclSyntaxProtocol & WithModifiersSyntax>(
175-
_ modifier: DeclModifierSyntax,
176-
to decl: Decl,
177-
declKeywordKeyPath: WritableKeyPath<Decl, TokenSyntax>
178-
) -> DeclSyntax {
179-
// If there's already an access modifier among the modifier list, bail out.
180-
guard decl.modifiers.accessLevelModifier == nil else { return DeclSyntax(decl) }
181-
182-
var result = decl
183-
var modifier = modifier
184-
modifier.trailingTrivia = [.spaces(1)]
185-
186-
guard var firstModifier = decl.modifiers.first else {
187-
// If there are no modifiers at all, add the one being requested, moving the leading trivia
188-
// from the decl keyword to that modifier (to preserve leading comments, newlines, etc.).
189-
modifier.leadingTrivia = decl[keyPath: declKeywordKeyPath].leadingTrivia
190-
result[keyPath: declKeywordKeyPath].leadingTrivia = []
191-
result.modifiers = .init([modifier])
192-
return DeclSyntax(result)
193-
}
194-
195-
// Otherwise, insert the modifier at the front of the modifier list, moving the (original) first
196-
// modifier's leading trivia to the new one (to preserve leading comments, newlines, etc.).
197-
modifier.leadingTrivia = firstModifier.leadingTrivia
198-
firstModifier.leadingTrivia = []
199-
result.modifiers[result.modifiers.startIndex] = firstModifier
200-
result.modifiers.insert(modifier, at: result.modifiers.startIndex)
201-
return DeclSyntax(result)
202-
}

0 commit comments

Comments
 (0)