From 537dec1d37852c9b13f089bc2517b1a4ff693b2a Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Tue, 9 Apr 2019 10:43:57 -0700 Subject: [PATCH] Fix false positives on `explicit_acl` and `explicit_top_level_acl` Fixes #2705 --- CHANGELOG.md | 5 +++++ Rules.md | 8 ++++++++ .../Rules/Idiomatic/ExplicitACLRule.swift | 12 +++++++++++- .../Rules/Idiomatic/ExplicitTopLevelACLRule.swift | 13 ++++++++++++- 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ae643b861..ccdd7624b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,11 @@ [Keith Smiley](https://github.com/keith) [#2703](https://github.com/realm/SwiftLint/issues/2703) +* Fix false positives on `explicit_acl` and `explicit_top_level_acl` rules when + declaring extensions that add protocol conformances with Swift 5. + [Marcelo Fabri](https://github.com/marcelofabri) + [#2705](https://github.com/realm/SwiftLint/issues/2705) + ## 0.31.0: Busy Laundromat #### Breaking diff --git a/Rules.md b/Rules.md index d157789f1a..b6137b4047 100644 --- a/Rules.md +++ b/Rules.md @@ -5728,6 +5728,10 @@ internal protocol A { internal class A { deinit {} } ``` +```swift +extension A: Equatable {} +``` +
Triggering Examples @@ -6034,6 +6038,10 @@ internal func a() {} ``` +```swift +extension A: Equatable {} +``` +
Triggering Examples diff --git a/Source/SwiftLintFramework/Rules/Idiomatic/ExplicitACLRule.swift b/Source/SwiftLintFramework/Rules/Idiomatic/ExplicitACLRule.swift index 19434ed9ea..329dacb2dc 100644 --- a/Source/SwiftLintFramework/Rules/Idiomatic/ExplicitACLRule.swift +++ b/Source/SwiftLintFramework/Rules/Idiomatic/ExplicitACLRule.swift @@ -39,7 +39,8 @@ public struct ExplicitACLRule: OptInRule, ConfigurationProviderRule, AutomaticTe var b: Int } """, - "internal class A { deinit {} }" + "internal class A { deinit {} }", + "extension A: Equatable {}" ], triggeringExamples: [ "enum A {}\n", @@ -60,11 +61,20 @@ public struct ExplicitACLRule: OptInRule, ConfigurationProviderRule, AutomaticTe private func offsetOfElements(from elements: [SourceKittenElement], in file: File, thatAreNotInRanges ranges: [NSRange]) -> [Int] { + let extensionKinds: Set = [.extension, .extensionClass, .extensionEnum, + .extensionProtocol, .extensionStruct] + return elements.compactMap { element in guard let typeOffset = element.offset else { return nil } + guard let kind = element.kind.flatMap(SwiftDeclarationKind.init(rawValue:)), + case let isConformanceExtension = extensionKinds.contains(kind) && !element.inheritedTypes.isEmpty, + !isConformanceExtension else { + return nil + } + // find the last "internal" token before the type guard let previousInternalByteRange = lastInternalByteRange(before: typeOffset, in: ranges) else { return typeOffset diff --git a/Source/SwiftLintFramework/Rules/Idiomatic/ExplicitTopLevelACLRule.swift b/Source/SwiftLintFramework/Rules/Idiomatic/ExplicitTopLevelACLRule.swift index 81d78d18cd..7159e14646 100644 --- a/Source/SwiftLintFramework/Rules/Idiomatic/ExplicitTopLevelACLRule.swift +++ b/Source/SwiftLintFramework/Rules/Idiomatic/ExplicitTopLevelACLRule.swift @@ -18,7 +18,8 @@ public struct ExplicitTopLevelACLRule: OptInRule, ConfigurationProviderRule, Aut "internal enum A {\n enum B {}\n}", "internal final class Foo {}", "internal\nclass Foo {}", - "internal func a() {}\n" + "internal func a() {}\n", + "extension A: Equatable {}" ], triggeringExamples: [ "enum A {}\n", @@ -30,8 +31,18 @@ public struct ExplicitTopLevelACLRule: OptInRule, ConfigurationProviderRule, Aut ) public func validate(file: File) -> [StyleViolation] { + let extensionKinds: Set = [.extension, .extensionClass, .extensionEnum, + .extensionProtocol, .extensionStruct] + // find all top-level types marked as internal (either explictly or implictly) let internalTypesOffsets = file.structure.dictionary.substructure.compactMap { element -> Int? in + // ignore extensions that declare protocol conformance + guard let kind = element.kind.flatMap(SwiftDeclarationKind.init(rawValue:)), + case let isConformanceExtension = extensionKinds.contains(kind) && !element.inheritedTypes.isEmpty, + !isConformanceExtension else { + return nil + } + if element.accessibility.flatMap(AccessControlLevel.init(identifier:)) == .internal { return element.offset }