Skip to content

Commit

Permalink
Remove extensions from LowerACLThanBodyRule
Browse files Browse the repository at this point in the history
Because of some oddities in the Swift extension ACL ruless, linting ACL
inside extensions isn't trivial because of a few cases:

1. Extensions cannot be open, but members inside them can:

```
open extension Foo { // Not valid
    open func bar() {}
}
```

2. Extensions cannot have an ACL modifier if they conform to a protocol:

```
public extension Foo: Bar {} // Not valid
```

3. Extensions that do have an ACL modifier affects the inner body,
making changes to this slightly riskier:

```
public extension Foo {
    func bar() {} // implicitly public
}
```

With this change we just sidestep all these problems, and instead opt to
only lint non extension types. I think this tradeoff is worth it so that
this can be enabled and catch issues in classes/structs/enums
  • Loading branch information
keith committed Apr 20, 2018
1 parent 59584f1 commit ac414e0
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 20 deletions.
8 changes: 4 additions & 4 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -8483,6 +8483,10 @@ fileprivate struct Foo { private func bar() {} }
private struct Foo { private func bar(id: String) }
```

```swift
extension Foo { public func bar() {} }
```

```swift
private func foo(id: String) {}
```
Expand All @@ -8495,10 +8499,6 @@ private func foo(id: String) {}
struct Foo { public func bar() {} }
```

```swift
extension Foo { public func bar() {} }
```

```swift
enum Foo { public func bar() {} }
```
Expand Down
8 changes: 4 additions & 4 deletions Source/SwiftLintFramework/Models/AccessControlLevel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ extension AccessControlLevel: Comparable {
private var priority: Int {
switch self {
case .private: return 1
case .fileprivate: return 2
case .internal: return 3
case .public: return 4
case .open: return 5
case .fileprivate: return 1
case .internal: return 2
case .public: return 3
case .open: return 4
}
}

Expand Down
23 changes: 11 additions & 12 deletions Source/SwiftLintFramework/Rules/LowerACLThanBodyRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ public struct LowerACLThanParentRule: OptInRule, ConfigurationProviderRule {
"open class Foo { open func bar() {} }",
"fileprivate struct Foo { private func bar() {} }",
"private struct Foo { private func bar(id: String) }",
"extension Foo { public func bar() {} }",
"private struct Foo { fileprivate func bar() {} }",
"private func foo(id: String) {}"
],
triggeringExamples: [
"struct Foo { public func bar() {} }",
"extension Foo { public func bar() {} }",
"enum Foo { public func bar() {} }",
"public class Foo { open func bar() }",
"private struct Foo { fileprivate func bar() {} }",
"class Foo { public private(set) var bar: String? }"
]
)
Expand Down Expand Up @@ -70,17 +70,16 @@ public struct LowerACLThanParentRule: OptInRule, ConfigurationProviderRule {
private extension SwiftDeclarationKind {
var isRelevantDeclaration: Bool {
switch self {
case .`associatedtype`, .enumcase, .enumelement, .functionAccessorAddress,
.functionAccessorDidset, .functionAccessorGetter, .functionAccessorMutableaddress,
.functionAccessorSetter, .functionAccessorWillset, .functionDestructor, .genericTypeParam, .module,
.precedenceGroup, .varLocal, .varParameter:
case .`associatedtype`, .enumcase, .enumelement, .`extension`, .`extensionClass`, .`extensionEnum`,
.extensionProtocol, .extensionStruct, .functionAccessorAddress, .functionAccessorDidset,
.functionAccessorGetter, .functionAccessorMutableaddress, .functionAccessorSetter,
.functionAccessorWillset, .functionDestructor, .genericTypeParam, .module, .precedenceGroup, .varLocal,
.varParameter:
return false
case .`class`, .`enum`, .`extension`, .`extensionClass`, .`extensionEnum`,
.extensionProtocol, .extensionStruct, .functionConstructor,
.functionFree, .functionMethodClass, .functionMethodInstance, .functionMethodStatic,
.functionOperator, .functionOperatorInfix, .functionOperatorPostfix, .functionOperatorPrefix,
.functionSubscript, .`protocol`, .`struct`, .`typealias`, .varClass,
.varGlobal, .varInstance, .varStatic:
case .`class`, .`enum`, .functionConstructor, .functionFree, .functionMethodClass, .functionMethodInstance,
.functionMethodStatic, .functionOperator, .functionOperatorInfix, .functionOperatorPostfix,
.functionOperatorPrefix, .functionSubscript, .`protocol`, .`struct`, .`typealias`, .varClass, .varGlobal,
.varInstance, .varStatic:
return true
}
}
Expand Down

0 comments on commit ac414e0

Please sign in to comment.