diff --git a/CHANGELOG.md b/CHANGELOG.md index 60d8f09e53..8d647c7c56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ #### Enhancements +* Add `LowerACLThanParent` rule + [Keith Smiley](https://github.com/keith) + [#2136](https://github.com/realm/SwiftLint/pull/2136) + * Add `UIOffsetMake` to `legacy_constructor` rule. [Nealon Young](https://github.com/nealyoung) [#2126](https://github.com/realm/SwiftLint/issues/2126) diff --git a/Rules.md b/Rules.md index 8e6c747f13..770cbf2f6b 100644 --- a/Rules.md +++ b/Rules.md @@ -61,6 +61,7 @@ * [Variable Declaration Whitespace](#variable-declaration-whitespace) * [Line Length](#line-length) * [Literal Expression End Indentation](#literal-expression-end-indentation) +* [Lower ACL than parent](#lower-acl-than-parent) * [Mark](#mark) * [Multiline Arguments](#multiline-arguments) * [Multiline Parameters](#multiline-parameters) @@ -8436,6 +8437,83 @@ let x = [ +## Lower ACL than parent + +Identifier | Enabled by default | Supports autocorrection | Kind +--- | --- | --- | --- +`lower_acl_than_parent` | Disabled | No | lint + +Ensure definitions have a lower access control level than their enclosing parent + +### Examples + +
+Non Triggering Examples + +```swift +public struct Foo { public func bar() {} } +``` + +```swift +internal struct Foo { func bar() {} } +``` + +```swift +struct Foo { func bar() {} } +``` + +```swift +open class Foo { public func bar() {} } +``` + +```swift +open class Foo { open func bar() {} } +``` + +```swift +fileprivate struct Foo { private func bar() {} } +``` + +```swift +private struct Foo { private func bar(id: String) } +``` + +```swift +private func foo(id: String) {} +``` + +
+
+Triggering Examples + +```swift +struct Foo { public func bar() {} } +``` + +```swift +extension Foo { public func bar() {} } +``` + +```swift +enum Foo { public func bar() {} } +``` + +```swift +public class Foo { open func bar() } +``` + +```swift +private struct Foo { fileprivate func bar() {} } +``` + +```swift +class Foo { public private(set) var bar: String? } +``` + +
+ + + ## Mark Identifier | Enabled by default | Supports autocorrection | Kind diff --git a/Source/SwiftLintFramework/Models/AccessControlLevel.swift b/Source/SwiftLintFramework/Models/AccessControlLevel.swift index 1bfe50c145..f5006afece 100644 --- a/Source/SwiftLintFramework/Models/AccessControlLevel.swift +++ b/Source/SwiftLintFramework/Models/AccessControlLevel.swift @@ -46,3 +46,19 @@ public enum AccessControlLevel: String, CustomStringConvertible { } } + +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 + } + } + + public static func < (lhs: AccessControlLevel, rhs: AccessControlLevel) -> Bool { + return lhs.priority < rhs.priority + } +} diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index acfa470b9e..786bc1ed70 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -47,17 +47,17 @@ public let masterRuleList = RuleList(rules: [ FileHeaderRule.self, FileLengthRule.self, FirstWhereRule.self, - ForWhereRule.self, ForceCastRule.self, ForceTryRule.self, ForceUnwrappingRule.self, + ForWhereRule.self, FunctionBodyLengthRule.self, FunctionParameterCountRule.self, GenericTypeNameRule.self, IdentifierNameRule.self, ImplicitGetterRule.self, - ImplicitReturnRule.self, ImplicitlyUnwrappedOptionalRule.self, + ImplicitReturnRule.self, IsDisjointRule.self, JoinedDefaultParameterRule.self, LargeTupleRule.self, @@ -69,6 +69,7 @@ public let masterRuleList = RuleList(rules: [ LetVarWhitespaceRule.self, LineLengthRule.self, LiteralExpressionEndIdentationRule.self, + LowerACLThanParentRule.self, MarkRule.self, MultilineArgumentsRule.self, MultilineParametersRule.self, diff --git a/Source/SwiftLintFramework/Rules/LowerACLThanBodyRule.swift b/Source/SwiftLintFramework/Rules/LowerACLThanBodyRule.swift new file mode 100644 index 0000000000..47ac0d7e84 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/LowerACLThanBodyRule.swift @@ -0,0 +1,87 @@ +// +// LowerACLThanParentRule.swift +// SwiftLint +// +// Created by Keith Smiley on 4/3/18. +// Copyright © 2018 Realm. All rights reserved. +// + +import Foundation +import SourceKittenFramework + +public struct LowerACLThanParentRule: OptInRule, ConfigurationProviderRule { + public var configuration = SeverityConfiguration(.warning) + + public init() {} + + public static let description = RuleDescription( + identifier: "lower_acl_than_parent", + name: "Lower ACL than parent", + description: "Ensure definitions have a lower access control level than their enclosing parent", + kind: .lint, + nonTriggeringExamples: [ + "public struct Foo { public func bar() {} }", + "internal struct Foo { func bar() {} }", + "struct Foo { func bar() {} }", + "open class Foo { public func bar() {} }", + "open class Foo { open func bar() {} }", + "fileprivate struct Foo { private func bar() {} }", + "private struct Foo { private func bar(id: String) }", + "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? }" + ] + ) + + public func validate(file: File) -> [StyleViolation] { + return validateACL(isHigherThan: .open, in: file.structure.dictionary).map { + StyleViolation(ruleDescription: type(of: self).description, + severity: configuration.severity, + location: Location(file: file, byteOffset: $0)) + } + } + + private func validateACL(isHigherThan parentAccessibility: AccessControlLevel, + in substructure: [String: SourceKitRepresentable]) -> [Int] { + return substructure.substructure.flatMap { element -> [Int] in + guard let elementKind = element.kind.flatMap(SwiftDeclarationKind.init(rawValue:)), + elementKind.isRelevantDeclaration else { + return [] + } + + var violationOffset: Int? + let accessibility = element.accessibility.flatMap(AccessControlLevel.init(identifier:)) + ?? .`internal` + if accessibility > parentAccessibility { + violationOffset = element.offset + } + + return [violationOffset].flatMap { $0 } + self.validateACL(isHigherThan: accessibility, in: element) + } + } +} + +private extension SwiftDeclarationKind { + var isRelevantDeclaration: Bool { + switch self { + case .`associatedtype`, .enumcase, .enumelement, .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: + return true + } + } +} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index 07dd2bd283..418e5dfd7c 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -149,6 +149,7 @@ B89F3BCF1FD5EE1400931E59 /* RequiredEnumCaseRuleConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = B89F3BC71FD5ED7D00931E59 /* RequiredEnumCaseRuleConfiguration.swift */; }; BB00B4E91F5216090079869F /* MultipleClosuresWithTrailingClosureRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB00B4E71F5216070079869F /* MultipleClosuresWithTrailingClosureRule.swift */; }; BFF028AE1CBCF8A500B38A9D /* TrailingWhitespaceConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = BF48D2D61CBCCA5F0080BDAE /* TrailingWhitespaceConfiguration.swift */; }; + C26330382073DAC500D7B4FD /* LowerACLThanBodyRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = C26330352073DAA200D7B4FD /* LowerACLThanBodyRule.swift */; }; C328A2F71E6759AE00A9E4D7 /* ExplicitTypeInterfaceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = C328A2F51E67595500A9E4D7 /* ExplicitTypeInterfaceRule.swift */; }; C3DE5DAC1E7DF9CA00761483 /* FatalErrorMessageRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = C3DE5DAA1E7DF99B00761483 /* FatalErrorMessageRule.swift */; }; C946FECB1EAE67EE007DD778 /* LetVarWhitespaceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = C946FEC91EAE5E20007DD778 /* LetVarWhitespaceRule.swift */; }; @@ -509,6 +510,7 @@ B89F3BCB1FD5EDA900931E59 /* RequiredEnumCaseRuleTestCase.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RequiredEnumCaseRuleTestCase.swift; sourceTree = ""; }; BB00B4E71F5216070079869F /* MultipleClosuresWithTrailingClosureRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MultipleClosuresWithTrailingClosureRule.swift; sourceTree = ""; }; BF48D2D61CBCCA5F0080BDAE /* TrailingWhitespaceConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TrailingWhitespaceConfiguration.swift; sourceTree = ""; }; + C26330352073DAA200D7B4FD /* LowerACLThanBodyRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LowerACLThanBodyRule.swift; sourceTree = ""; }; C328A2F51E67595500A9E4D7 /* ExplicitTypeInterfaceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ExplicitTypeInterfaceRule.swift; sourceTree = ""; }; C3DE5DAA1E7DF99B00761483 /* FatalErrorMessageRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FatalErrorMessageRule.swift; sourceTree = ""; }; C946FEC91EAE5E20007DD778 /* LetVarWhitespaceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LetVarWhitespaceRule.swift; sourceTree = ""; }; @@ -1067,6 +1069,7 @@ 62622F6A1F2F2E3500D5D099 /* DiscouragedDirectInitRule.swift */, 6258783A1FFC458100AC34F2 /* DiscouragedObjectLiteralRule.swift */, 62640150201552E0005B9C4A /* DiscouragedOptionalBooleanRule.swift */, + C26330352073DAA200D7B4FD /* LowerACLThanBodyRule.swift */, 6264015320155533005B9C4A /* DiscouragedOptionalBooleanRuleExamples.swift */, 629ADD052006302D0009E362 /* DiscouragedOptionalCollectionRule.swift */, 62FE5D30200CAB6E00F68793 /* DiscouragedOptionalCollectionRuleExamples.swift */, @@ -1520,6 +1523,7 @@ D47079A71DFCEB2D00027086 /* EmptyParenthesesWithTrailingClosureRule.swift in Sources */, E881985E1BEA982100333A11 /* TypeBodyLengthRule.swift in Sources */, 69F88BF71BDA38A6005E7CAE /* OpeningBraceRule.swift in Sources */, + C26330382073DAC500D7B4FD /* LowerACLThanBodyRule.swift in Sources */, 78F032461D7C877E00BE709A /* OverriddenSuperCallRule.swift in Sources */, E80E018D1B92C0F60078EB70 /* Command.swift in Sources */, E88198571BEA953300333A11 /* ForceCastRule.swift in Sources */, diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 4d5e911769..b16447f9b1 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -438,6 +438,7 @@ extension RulesTests { ("testEmptyCount", testEmptyCount), ("testEmptyEnumArguments", testEmptyEnumArguments), ("testEmptyParameters", testEmptyParameters), + ("testLowerACLThanParent", testLowerACLThanParent), ("testEmptyParenthesesWithTrailingClosure", testEmptyParenthesesWithTrailingClosure), ("testEmptyString", testEmptyString), ("testExplicitACL", testExplicitACL), diff --git a/Tests/SwiftLintFrameworkTests/RulesTests.swift b/Tests/SwiftLintFrameworkTests/RulesTests.swift index 2242930d9e..32b3fb3e4c 100644 --- a/Tests/SwiftLintFrameworkTests/RulesTests.swift +++ b/Tests/SwiftLintFrameworkTests/RulesTests.swift @@ -98,6 +98,10 @@ class RulesTests: XCTestCase { verifyRule(EmptyParametersRule.description) } + func testLowerACLThanParent() { + verifyRule(LowerACLThanParentRule.description) + } + func testEmptyParenthesesWithTrailingClosure() { verifyRule(EmptyParenthesesWithTrailingClosureRule.description) }