Skip to content

Commit 740ca79

Browse files
LaurenWhiteallevato
LaurenWhite
authored and
allevato
committed
Clean up NoAccessLevelOnExtension (swiftlang#93)
1 parent aa3331a commit 740ca79

File tree

3 files changed

+46
-79
lines changed

3 files changed

+46
-79
lines changed

‎Sources/Rules/AddModifierRewriter.swift

Lines changed: 18 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ private final class AddModifierRewriter: SyntaxRewriter {
1818
return newDecl.addModifier(modifierKeyword)
1919
}
2020
// If variable already has an accessor keyword, skip (do not overwrite)
21-
guard !hasAccessorKeyword(modifiers: modifiers) else { return node }
21+
guard modifiers.accessLevelModifier == nil else { return node }
2222

2323
// Put accessor keyword before the first modifier keyword in the declaration
24-
let newModifiers = insertAccessorKeyword(curModifiers: modifiers)
24+
let newModifiers = modifiers.prepend(modifier: modifierKeyword)
2525
return node.withModifiers(newModifiers)
2626
}
2727

@@ -31,8 +31,8 @@ private final class AddModifierRewriter: SyntaxRewriter {
3131
as? FunctionDeclSyntax else { return node }
3232
return newDecl.addModifier(modifierKeyword)
3333
}
34-
guard !hasAccessorKeyword(modifiers: modifiers) else { return node }
35-
let newModifiers = insertAccessorKeyword(curModifiers: modifiers)
34+
guard modifiers.accessLevelModifier == nil else { return node }
35+
let newModifiers = modifiers.prepend(modifier: modifierKeyword)
3636
return node.withModifiers(newModifiers)
3737
}
3838

@@ -42,8 +42,8 @@ private final class AddModifierRewriter: SyntaxRewriter {
4242
as? AssociatedtypeDeclSyntax else { return node }
4343
return newDecl.addModifier(modifierKeyword)
4444
}
45-
guard !hasAccessorKeyword(modifiers: modifiers) else { return node }
46-
let newModifiers = insertAccessorKeyword(curModifiers: modifiers)
45+
guard modifiers.accessLevelModifier == nil else { return node }
46+
let newModifiers = modifiers.prepend(modifier: modifierKeyword)
4747
return node.withModifiers(newModifiers)
4848
}
4949

@@ -53,8 +53,8 @@ private final class AddModifierRewriter: SyntaxRewriter {
5353
as? ClassDeclSyntax else { return node }
5454
return newDecl.addModifier(modifierKeyword)
5555
}
56-
guard !hasAccessorKeyword(modifiers: modifiers) else { return node }
57-
let newModifiers = insertAccessorKeyword(curModifiers: modifiers)
56+
guard modifiers.accessLevelModifier == nil else { return node }
57+
let newModifiers = modifiers.prepend(modifier: modifierKeyword)
5858
return node.withModifiers(newModifiers)
5959
}
6060

@@ -64,8 +64,8 @@ private final class AddModifierRewriter: SyntaxRewriter {
6464
as? EnumDeclSyntax else { return node }
6565
return newDecl.addModifier(modifierKeyword)
6666
}
67-
guard !hasAccessorKeyword(modifiers: modifiers) else { return node }
68-
let newModifiers = insertAccessorKeyword(curModifiers: modifiers)
67+
guard modifiers.accessLevelModifier == nil else { return node }
68+
let newModifiers = modifiers.prepend(modifier: modifierKeyword)
6969
return node.withModifiers(newModifiers)
7070
}
7171

@@ -75,8 +75,8 @@ private final class AddModifierRewriter: SyntaxRewriter {
7575
as? ProtocolDeclSyntax else { return node }
7676
return newDecl.addModifier(modifierKeyword)
7777
}
78-
guard !hasAccessorKeyword(modifiers: modifiers) else { return node }
79-
let newModifiers = insertAccessorKeyword(curModifiers: modifiers)
78+
guard modifiers.accessLevelModifier == nil else { return node }
79+
let newModifiers = modifiers.prepend(modifier: modifierKeyword)
8080
return node.withModifiers(newModifiers)
8181
}
8282

@@ -86,8 +86,8 @@ private final class AddModifierRewriter: SyntaxRewriter {
8686
as? StructDeclSyntax else { return node }
8787
return newDecl.addModifier(modifierKeyword)
8888
}
89-
guard !hasAccessorKeyword(modifiers: modifiers) else { return node }
90-
let newModifiers = insertAccessorKeyword(curModifiers: modifiers)
89+
guard modifiers.accessLevelModifier == nil else { return node }
90+
let newModifiers = modifiers.prepend(modifier: modifierKeyword)
9191
return node.withModifiers(newModifiers)
9292
}
9393

@@ -97,8 +97,8 @@ private final class AddModifierRewriter: SyntaxRewriter {
9797
as? TypealiasDeclSyntax else { return node }
9898
return newDecl.addModifier(modifierKeyword)
9999
}
100-
guard !hasAccessorKeyword(modifiers: modifiers) else { return node }
101-
let newModifiers = insertAccessorKeyword(curModifiers: modifiers)
100+
guard modifiers.accessLevelModifier == nil else { return node }
101+
let newModifiers = modifiers.prepend(modifier: modifierKeyword)
102102
return node.withModifiers(newModifiers)
103103
}
104104

@@ -108,35 +108,12 @@ private final class AddModifierRewriter: SyntaxRewriter {
108108
as? InitializerDeclSyntax else { return node }
109109
return newDecl.addModifier(modifierKeyword)
110110
}
111-
guard !hasAccessorKeyword(modifiers: modifiers) else { return node }
112-
let newModifiers = insertAccessorKeyword(curModifiers: modifiers)
111+
guard modifiers.accessLevelModifier == nil else { return node }
112+
let newModifiers = modifiers.prepend(modifier: modifierKeyword)
113113
return node.withModifiers(newModifiers)
114114
}
115115

116116

117-
// Determines if declaration already has an access keyword in modifiers
118-
func hasAccessorKeyword(modifiers: ModifierListSyntax) -> Bool {
119-
for modifier in modifiers {
120-
let keywordKind = modifier.name.tokenKind
121-
switch keywordKind {
122-
case .publicKeyword, .privateKeyword, .fileprivateKeyword, .internalKeyword:
123-
return true
124-
default:
125-
continue
126-
}
127-
}
128-
return false
129-
}
130-
131-
// Puts the access keyword at the beginning of the given modifier list
132-
func insertAccessorKeyword(curModifiers: ModifierListSyntax) -> ModifierListSyntax {
133-
var newModifiers: [DeclModifierSyntax] = []
134-
newModifiers.append(contentsOf: curModifiers)
135-
newModifiers[0] = newModifiers[0].withName(newModifiers[0].name.withoutLeadingTrivia())
136-
newModifiers.insert(modifierKeyword, at: 0)
137-
return SyntaxFactory.makeModifierList(newModifiers)
138-
}
139-
140117
func removeFirstTokLeadingTrivia(node: DeclSyntax) -> DeclSyntax {
141118
let withoutLeadTrivia = replaceTrivia(on: node,
142119
token: node.firstToken,

‎Sources/Rules/NoAccessLevelOnExtensionDeclaration.swift

Lines changed: 24 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -17,53 +17,39 @@ public final class NoAccessLevelOnExtensionDeclaration: SyntaxFormatRule {
1717

1818
public override func visit(_ node: ExtensionDeclSyntax) -> DeclSyntax {
1919
guard let modifiers = node.modifiers, modifiers.count != 0 else { return node }
20+
guard let accessKeyword = modifiers.accessLevelModifier else { return node }
2021

21-
for accessKeyword in modifiers {
22-
23-
let keywordKind = accessKeyword.name.tokenKind
24-
switch keywordKind {
25-
// Public, private, or fileprivate keywords need to be moved to members
26-
case .publicKeyword, .privateKeyword, .fileprivateKeyword:
27-
diagnose(.moveAccessKeyword(keyword: accessKeyword.name.text), on: accessKeyword)
28-
let newMembers = SyntaxFactory.makeMemberDeclBlock(
29-
leftBrace: node.members.leftBrace,
30-
members: addMemberAccessKeywords(memDeclBlock: node.members, keyword: accessKeyword),
31-
rightBrace: node.members.rightBrace)
32-
return node.withMembers(newMembers)
33-
.withModifiers(removeModifier(curModifiers: modifiers, removal: accessKeyword))
34-
// Internal keyword redundant, delete
35-
case .internalKeyword:
36-
diagnose(.removeRedundantAccessKeyword(name: node.extendedType.description),
37-
on: accessKeyword)
38-
let newKeyword = replaceTrivia(on: node.extensionKeyword,
39-
token: node.extensionKeyword,
40-
leadingTrivia: accessKeyword.leadingTrivia) as! TokenSyntax
41-
return node.withModifiers(removeModifier(curModifiers: modifiers, removal: accessKeyword))
42-
.withExtensionKeyword(newKeyword)
43-
default:
44-
return node
45-
}
22+
let keywordKind = accessKeyword.name.tokenKind
23+
switch keywordKind {
24+
// Public, private, or fileprivate keywords need to be moved to members
25+
case .publicKeyword, .privateKeyword, .fileprivateKeyword:
26+
diagnose(.moveAccessKeyword(keyword: accessKeyword.name.text), on: accessKeyword)
27+
let newMembers = SyntaxFactory.makeMemberDeclBlock(
28+
leftBrace: node.members.leftBrace,
29+
members: addMemberAccessKeywords(memDeclBlock: node.members, keyword: accessKeyword),
30+
rightBrace: node.members.rightBrace)
31+
return node.withMembers(newMembers)
32+
.withModifiers(modifiers.remove(name: accessKeyword.name.text))
33+
// Internal keyword redundant, delete
34+
case .internalKeyword:
35+
diagnose(.removeRedundantAccessKeyword(name: node.extendedType.description),
36+
on: accessKeyword)
37+
let newKeyword = replaceTrivia(on: node.extensionKeyword,
38+
token: node.extensionKeyword,
39+
leadingTrivia: accessKeyword.leadingTrivia) as! TokenSyntax
40+
return node.withModifiers(modifiers.remove(name: accessKeyword.name.text))
41+
.withExtensionKeyword(newKeyword)
42+
default:
43+
break
4644
}
4745
return node
4846
}
4947

50-
// Returns modifier list without the access modifier
51-
func removeModifier(curModifiers: ModifierListSyntax,
52-
removal: DeclModifierSyntax) -> ModifierListSyntax {
53-
var newMods: [DeclModifierSyntax] = []
54-
for modifier in curModifiers {
55-
if modifier.name != removal.name {
56-
newMods.append(modifier)
57-
}
58-
}
59-
return SyntaxFactory.makeModifierList(newMods)
60-
}
61-
6248
// Adds given keyword to all members in declaration block
6349
func addMemberAccessKeywords(memDeclBlock: MemberDeclBlockSyntax,
6450
keyword: DeclModifierSyntax) -> MemberDeclListSyntax {
6551
var newMembers: [MemberDeclListItemSyntax] = []
66-
52+
6753
for member in memDeclBlock.members {
6854
guard let firstTokInDecl = member.firstToken else { continue }
6955
let formattedKeyword = replaceTrivia(on: keyword,

‎Tests/SwiftFormatTests/NoAccessLevelOnExtensionDeclarationTests.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ public class NoAccessLevelOnExtensionDeclarationTests: DiagnosingTestCase {
1111
input: """
1212
public extension Foo {
1313
var x: Bool
14+
// Comment 1
1415
internal var y: Bool
16+
// Comment 2
1517
static var z: Bool
1618
static func someFunc() {}
1719
init() {}
@@ -28,7 +30,9 @@ public class NoAccessLevelOnExtensionDeclarationTests: DiagnosingTestCase {
2830
expected: """
2931
extension Foo {
3032
public var x: Bool
33+
// Comment 1
3134
internal var y: Bool
35+
// Comment 2
3236
public static var z: Bool
3337
public static func someFunc() {}
3438
public init() {}

0 commit comments

Comments
 (0)