Skip to content

Commit 99ec492

Browse files
SimplyDannySimplyDanny
authored
Use effective ACL in discouraged_default_parameter rule (#6563)
1 parent 29a71dd commit 99ec492

File tree

5 files changed

+301
-185
lines changed

5 files changed

+301
-185
lines changed

‎Source/SwiftLintBuiltInRules/Rules/Lint/DiscouragedDefaultParameterRule.swift‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ struct DiscouragedDefaultParameterRule: Rule {
1717
nonTriggeringExamples: [
1818
Example("public func foo(bar: Int = 0) {}"),
1919
Example("open func foo(bar: Int = 0) {}"),
20+
Example("public extension Foo { func foo(bar: Int = 0) {} }"),
21+
Example("extension E { public func foo(bar: Int = 0) {} }"),
22+
Example("func outer() { func inner(bar: Int = 0) {} }"),
2023
Example("func foo(bar: Int) {}"),
2124
Example("private func foo(bar: Int = 0) {}"),
2225
Example("fileprivate func foo(bar: Int = 0) {}"),
@@ -32,6 +35,8 @@ struct DiscouragedDefaultParameterRule: Rule {
3235
Example("package func foo(bar: Int ↓= 0) {}"),
3336
Example("func foo(bar: Int ↓= 0, baz: String ↓= \"\") {}"),
3437
Example("init(value: Int ↓= 42) {}"),
38+
Example("class C { public func foo(bar: Int ↓= 0) {} }"),
39+
Example("struct S { public init(value: Int ↓= 42) {} }"),
3540
Example(
3641
"private func foo(bar: Int ↓= 0) {}",
3742
configuration: ["disallowed_access_levels": ["private"]]
@@ -45,47 +50,43 @@ struct DiscouragedDefaultParameterRule: Rule {
4550
}
4651

4752
private extension DiscouragedDefaultParameterRule {
48-
final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
49-
override func visitPost(_ node: FunctionDeclSyntax) {
53+
final class Visitor: EffectiveAccessControlSyntaxVisitor<ConfigurationType> {
54+
init(configuration: ConfigurationType, file: SwiftLintFile) {
55+
super.init(configuration: configuration, file: file)
56+
}
57+
58+
override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind {
5059
collectViolations(modifiers: node.modifiers, parameterClause: node.signature.parameterClause)
60+
return .skipChildren
5161
}
5262

53-
override func visitPost(_ node: InitializerDeclSyntax) {
63+
override func visit(_ node: InitializerDeclSyntax) -> SyntaxVisitorContinueKind {
5464
collectViolations(modifiers: node.modifiers, parameterClause: node.signature.parameterClause)
65+
return .skipChildren
5566
}
5667

57-
override func visitPost(_ node: SubscriptDeclSyntax) {
68+
override func visit(_ node: SubscriptDeclSyntax) -> SyntaxVisitorContinueKind {
5869
collectViolations(modifiers: node.modifiers, parameterClause: node.parameterClause)
70+
return .skipChildren
5971
}
6072

61-
private func collectViolations(
62-
modifiers: DeclModifierListSyntax,
63-
parameterClause: FunctionParameterClauseSyntax
64-
) {
65-
guard let accessLevel = effectiveAccessLevel(modifiers),
73+
private func collectViolations(modifiers: DeclModifierListSyntax,
74+
parameterClause: FunctionParameterClauseSyntax) {
75+
guard !isInLocalAccessControlScope,
76+
case let accessLevel = effectiveAccessControlLevel(for: modifiers),
6677
configuration.disallowedAccessLevels.contains(accessLevel) else {
6778
return
6879
}
69-
let levelName = accessLevel.rawValue
7080
for param in parameterClause.parameters {
7181
if let defaultValue = param.defaultValue {
7282
violations.append(
73-
ReasonedRuleViolation(
83+
.init(
7484
position: defaultValue.positionAfterSkippingLeadingTrivia,
75-
reason: "Default parameter values should not be used in '\(levelName)' functions"
85+
reason: "Default parameter values should not be used in '\(accessLevel)' functions"
7686
)
7787
)
7888
}
7989
}
8090
}
81-
82-
private func effectiveAccessLevel(_ modifiers: DeclModifierListSyntax)
83-
-> DiscouragedDefaultParameterConfiguration.AccessLevel? {
84-
if modifiers.contains(keyword: .private) { return .private }
85-
if modifiers.contains(keyword: .fileprivate) { return .fileprivate }
86-
if modifiers.contains(keyword: .package) { return .package }
87-
if modifiers.contains(keyword: .public) || modifiers.contains(keyword: .open) { return nil }
88-
return .internal
89-
}
9091
}
9192
}

‎Source/SwiftLintBuiltInRules/Rules/Lint/MissingDocsRule.swift‎

Lines changed: 25 additions & 163 deletions
Original file line numberDiff line numberDiff line change
@@ -16,47 +16,35 @@ struct MissingDocsRule: Rule {
1616
}
1717

1818
private extension MissingDocsRule {
19-
final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
20-
private var aclScope = Stack<AccessControlBehavior>()
19+
final class Visitor: EffectiveAccessControlSyntaxVisitor<ConfigurationType> {
20+
init(configuration: ConfigurationType, file: SwiftLintFile) {
21+
super.init(
22+
configuration: configuration,
23+
file: file,
24+
evaluateEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
25+
)
26+
}
2127

2228
override func visit(_ node: ActorDeclSyntax) -> SyntaxVisitorContinueKind {
23-
defer {
24-
aclScope.push(
25-
behavior: .actor(node.modifiers.accessibility),
26-
evalEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
27-
)
28-
}
2929
if node.inherits, configuration.excludesInheritedTypes {
30+
_ = super.visit(node)
3031
return .skipChildren
3132
}
3233
collectViolation(from: node, on: node.actorKeyword)
33-
return .visitChildren
34-
}
35-
36-
override func visitPost(_: ActorDeclSyntax) {
37-
aclScope.pop()
34+
return super.visit(node)
3835
}
3936

4037
override func visitPost(_ node: AssociatedTypeDeclSyntax) {
4138
collectViolation(from: node, on: node.associatedtypeKeyword)
4239
}
4340

4441
override func visit(_ node: ClassDeclSyntax) -> SyntaxVisitorContinueKind {
45-
defer {
46-
aclScope.push(
47-
behavior: .class(node.modifiers.accessibility),
48-
evalEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
49-
)
50-
}
5142
if node.inherits, configuration.excludesInheritedTypes {
43+
_ = super.visit(node)
5244
return .skipChildren
5345
}
5446
collectViolation(from: node, on: node.classKeyword)
55-
return .visitChildren
56-
}
57-
58-
override func visitPost(_: ClassDeclSyntax) {
59-
aclScope.pop()
47+
return super.visit(node)
6048
}
6149

6250
override func visit(_: ClosureExprSyntax) -> SyntaxVisitorContinueKind {
@@ -68,52 +56,38 @@ private extension MissingDocsRule {
6856
}
6957

7058
override func visitPost(_ node: EnumCaseDeclSyntax) {
71-
guard !node.hasDocComment, case let .enum(enumAcl) = aclScope.peek() else {
59+
guard !node.hasDocComment, let enumAccessControlLevel else {
7260
return
7361
}
74-
let acl = enumAcl ?? .internal
75-
if let parameter = configuration.parameters.first(where: { $0.value == acl }) {
62+
if let parameter = configuration.parameters.first(where: { $0.value == enumAccessControlLevel }) {
7663
violations.append(
77-
ReasonedRuleViolation(
64+
.init(
7865
position: node.caseKeyword.positionAfterSkippingLeadingTrivia,
79-
reason: "\(acl) declarations should be documented",
66+
reason: "\(enumAccessControlLevel) declarations should be documented",
8067
severity: parameter.severity
8168
)
8269
)
8370
}
8471
}
8572

8673
override func visit(_ node: EnumDeclSyntax) -> SyntaxVisitorContinueKind {
87-
defer {
88-
aclScope.push(
89-
behavior: .enum(node.modifiers.accessibility),
90-
evalEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
91-
)
92-
}
9374
if node.inherits, configuration.excludesInheritedTypes {
75+
_ = super.visit(node)
9476
return .skipChildren
9577
}
9678
collectViolation(from: node, on: node.enumKeyword)
97-
return .visitChildren
98-
}
99-
100-
override func visitPost(_: EnumDeclSyntax) {
101-
aclScope.pop()
79+
return super.visit(node)
10280
}
10381

10482
override func visit(_ node: ExtensionDeclSyntax) -> SyntaxVisitorContinueKind {
105-
defer { aclScope.push(.extension(node.modifiers.accessibility)) }
10683
if node.inherits, configuration.excludesInheritedTypes {
84+
_ = super.visit(node)
10785
return .skipChildren
10886
}
10987
if !configuration.excludesExtensions {
11088
collectViolation(from: node, on: node.extensionKeyword)
11189
}
112-
return .visitChildren
113-
}
114-
115-
override func visitPost(_: ExtensionDeclSyntax) {
116-
aclScope.pop()
90+
return super.visit(node)
11791
}
11892

11993
override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind {
@@ -129,39 +103,21 @@ private extension MissingDocsRule {
129103
}
130104

131105
override func visit(_ node: ProtocolDeclSyntax) -> SyntaxVisitorContinueKind {
132-
defer {
133-
aclScope.push(
134-
behavior: .protocol(node.modifiers.accessibility),
135-
evalEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
136-
)
137-
}
138106
if node.inherits, configuration.excludesInheritedTypes {
107+
_ = super.visit(node)
139108
return .skipChildren
140109
}
141110
collectViolation(from: node, on: node.protocolKeyword)
142-
return .visitChildren
143-
}
144-
145-
override func visitPost(_: ProtocolDeclSyntax) {
146-
aclScope.pop()
111+
return super.visit(node)
147112
}
148113

149114
override func visit(_ node: StructDeclSyntax) -> SyntaxVisitorContinueKind {
150-
defer {
151-
aclScope.push(
152-
behavior: .struct(node.modifiers.accessibility),
153-
evalEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
154-
)
155-
}
156115
if node.inherits, configuration.excludesInheritedTypes {
116+
_ = super.visit(node)
157117
return .skipChildren
158118
}
159119
collectViolation(from: node, on: node.structKeyword)
160-
return .visitChildren
161-
}
162-
163-
override func visitPost(_: StructDeclSyntax) {
164-
aclScope.pop()
120+
return super.visit(node)
165121
}
166122

167123
override func visit(_ node: SubscriptDeclSyntax) -> SyntaxVisitorContinueKind {
@@ -182,10 +138,7 @@ private extension MissingDocsRule {
182138
if node.modifiers.contains(keyword: .override) || node.hasDocComment {
183139
return
184140
}
185-
let acl = aclScope.computeAcl(
186-
givenExplicitAcl: node.modifiers.accessibility,
187-
evalEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
188-
)
141+
let acl = effectiveAccessControlLevel(for: node.modifiers)
189142
if let parameter = configuration.parameters.first(where: { $0.value == acl }) {
190143
violations.append(
191144
ReasonedRuleViolation(
@@ -230,94 +183,3 @@ private extension SyntaxProtocol {
230183
}
231184
}
232185
}
233-
234-
private extension DeclModifierListSyntax {
235-
var accessibility: AccessControlLevel? {
236-
filter { $0.detail == nil }.compactMap { AccessControlLevel(description: $0.name.text) }.first
237-
}
238-
}
239-
240-
private enum AccessControlBehavior {
241-
case `actor`(AccessControlLevel?)
242-
case local
243-
case `class`(AccessControlLevel?)
244-
case `enum`(AccessControlLevel?)
245-
case `extension`(AccessControlLevel?)
246-
case `protocol`(AccessControlLevel?)
247-
case `struct`(AccessControlLevel?)
248-
249-
var effectiveAcl: AccessControlLevel {
250-
explicitAcl ?? .internal
251-
}
252-
253-
var explicitAcl: AccessControlLevel? {
254-
switch self {
255-
case let .actor(acl): acl
256-
case .local: nil
257-
case let .class(acl): acl
258-
case let .enum(acl): acl
259-
case let .extension(acl): acl
260-
case let .protocol(acl): acl
261-
case let .struct(acl): acl
262-
}
263-
}
264-
265-
func sameWith(acl: AccessControlLevel) -> Self {
266-
switch self {
267-
case .actor: .actor(acl)
268-
case .local: .local
269-
case .class: .class(acl)
270-
case .enum: .enum(acl)
271-
case .extension: .extension(acl)
272-
case .protocol: .protocol(acl)
273-
case .struct: .struct(acl)
274-
}
275-
}
276-
}
277-
278-
/// Implementation of Swift's effective ACL logic. Should be moved to a specialized syntax visitor for reuse some time.
279-
private extension Stack<AccessControlBehavior> {
280-
mutating func push(behavior: AccessControlBehavior, evalEffectiveAcl: Bool) {
281-
if let parentBehavior = peek() {
282-
switch parentBehavior {
283-
case .local:
284-
push(.local)
285-
case .actor, .class, .struct, .enum:
286-
if behavior.effectiveAcl <= parentBehavior.effectiveAcl || !evalEffectiveAcl {
287-
push(behavior)
288-
} else {
289-
push(behavior.sameWith(acl: parentBehavior.effectiveAcl))
290-
}
291-
case .extension, .protocol:
292-
if behavior.explicitAcl != nil {
293-
push(behavior)
294-
} else {
295-
push(behavior.sameWith(acl: parentBehavior.effectiveAcl))
296-
}
297-
}
298-
} else {
299-
push(behavior)
300-
}
301-
}
302-
303-
func computeAcl(givenExplicitAcl acl: AccessControlLevel?, evalEffectiveAcl: Bool) -> AccessControlLevel {
304-
if let parentBehavior = peek() {
305-
switch parentBehavior {
306-
case .local:
307-
.private
308-
case .actor, .class, .struct, .enum:
309-
if let acl {
310-
acl < parentBehavior.effectiveAcl || !evalEffectiveAcl ? acl : parentBehavior.effectiveAcl
311-
} else {
312-
parentBehavior.effectiveAcl >= .internal ? .internal : parentBehavior.effectiveAcl
313-
}
314-
case .protocol:
315-
parentBehavior.effectiveAcl
316-
case .extension:
317-
acl ?? parentBehavior.effectiveAcl
318-
}
319-
} else {
320-
acl ?? .internal
321-
}
322-
}
323-
}

‎Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/DiscouragedDefaultParameterConfiguration.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ struct DiscouragedDefaultParameterConfiguration: SeverityBasedRuleConfiguration
55
@ConfigurationElement(key: "severity")
66
private(set) var severityConfiguration = SeverityConfiguration<Parent>(.warning)
77
@ConfigurationElement(key: "disallowed_access_levels")
8-
private(set) var disallowedAccessLevels: Set<AccessLevel> = [.internal, .package]
8+
private(set) var disallowedAccessLevels: Set<AccessControlLevel> = [.internal, .package]
99

1010
@AcceptableByConfigurationElement
1111
enum AccessLevel: String, Comparable {

‎Source/SwiftLintCore/Models/AccessControlLevel.swift‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,17 @@ extension AccessControlLevel: Comparable {
6565
lhs.priority < rhs.priority
6666
}
6767
}
68+
69+
extension AccessControlLevel: AcceptableByConfigurationElement {
70+
public init(fromAny value: Any, context ruleID: String) throws(Issue) {
71+
if let value = value as? String, let newSelf = Self(description: value) {
72+
self = newSelf
73+
} else {
74+
throw .invalidConfiguration(ruleID: ruleID)
75+
}
76+
}
77+
78+
public func asOption() -> OptionType {
79+
.symbol(description)
80+
}
81+
}

0 commit comments

Comments
 (0)