Rewrite explicit_acl rule with SwiftSyntax (#5382)

realm · Dec 20, 2023 · 4c89311 · 4c89311
1 parent 70a5ad0
commit 4c89311
Show file tree

Hide file tree

Showing 2 changed files with 113 additions and 85 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,7 @@
   [#2802](https://github.com/realm/SwiftLint/issues/2802)
 
 * Rewrite the following rules with SwiftSyntax:
+  * `explicit_acl`
   * `identifier_name`
   * `let_var_whitespace`
   * `multiline_literal_brackets`

diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ExplicitACLRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ExplicitACLRule.swift
@@ -1,8 +1,7 @@
-import Foundation
-import SourceKittenFramework
-
-private typealias SourceKittenElement = SourceKittenDictionary
+import SwiftLintCore
+import SwiftSyntax
 
+@SwiftSyntaxRule
 struct ExplicitACLRule: OptInRule {
     var configuration = SeverityConfiguration<Self>(.warning)
 
@@ -15,7 +14,7 @@ struct ExplicitACLRule: OptInRule {
             Example("internal enum A {}"),
             Example("public final class B {}"),
             Example("private struct C {}"),
-            Example("internal enum A {\n internal enum B {}\n}"),
+            Example("internal enum A { internal enum B {} }"),
             Example("internal final class Foo {}"),
             Example("""
             internal
@@ -69,126 +68,154 @@ struct ExplicitACLRule: OptInRule {
                     }
                 }
             }
+            """),
+            Example("""
+            private extension Foo {
+                var isValid: Bool { true }
+                struct S {
+                    let b = 2
+                }
+            }
             """)
         ],
         triggeringExamples: [
             Example("↓enum A {}"),
             Example("final ↓class B {}"),
             Example("internal struct C { ↓let d = 5 }"),
+            Example("public struct C { private(set) ↓var d = 5 }"),
+            Example("internal struct C { ↓static let d = 5 }"),
             Example("public struct C { ↓let d = 5 }"),
-            Example("func a() {}"),
+            Example("public struct C { ↓init() }"),
+            Example("↓func a() {}"),
             Example("internal let a = 0\n↓func b() {}"),
             Example("""
             extension Foo {
                 ↓func bar() {}
             }
+            """),
+            Example("""
+            public extension E {
+                let a = 1
+                struct S {
+                    ↓let b = 2
+                }
+            }
             """)
         ]
     )
+}
 
-    private func findAllExplicitInternalTokens(in file: SwiftLintFile) -> [ByteRange] {
-        let contents = file.stringView
-        return file.match(pattern: "internal", with: [.attributeBuiltin]).compactMap {
-            contents.NSRangeToByteRange(start: $0.location, length: $0.length)
+private enum CheckACLState {
+    case yes
+    case no // swiftlint:disable:this identifier_name
+}
+
+private extension ExplicitACLRule {
+    final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
+        private var declScope = Stack<CheckACLState>()
+
+        override var skippableDeclarations: [any DeclSyntaxProtocol.Type] {
+            [
+                FunctionDeclSyntax.self,
+                SubscriptDeclSyntax.self,
+                VariableDeclSyntax.self,
+                ProtocolDeclSyntax.self,
+                InitializerDeclSyntax.self
+            ]
         }
-    }
 
-    private func offsetOfElements(from elements: [SourceKittenElement], in file: SwiftLintFile,
-                                  thatAreNotInRanges ranges: [ByteRange]) -> [ByteCount] {
-        return elements.compactMap { element in
-            guard let typeOffset = element.offset else {
-                return nil
-            }
+        override func visit(_ node: ActorDeclSyntax) -> SyntaxVisitorContinueKind {
+            collectViolations(decl: node, token: node.actorKeyword)
+            declScope.push(.yes)
+            return node.modifiers.containsPrivateOrFileprivate() ? .skipChildren : .visitChildren
+        }
 
-            guard let kind = element.declarationKind,
-                !SwiftDeclarationKind.extensionKinds.contains(kind) else {
-                    return nil
-            }
+        override func visitPost(_ node: ActorDeclSyntax) {
+            declScope.pop()
+        }
 
-            // find the last "internal" token before the type
-            guard let previousInternalByteRange = lastInternalByteRange(before: typeOffset, in: ranges) else {
-                return typeOffset
-            }
+        override func visit(_ node: ClassDeclSyntax) -> SyntaxVisitorContinueKind {
+            collectViolations(decl: node, token: node.classKeyword)
+            declScope.push(.yes)
+            return node.modifiers.containsPrivateOrFileprivate() ? .skipChildren : .visitChildren
+        }
 
-            // the "internal" token correspond to the type if there're only
-            // attributeBuiltin (`final` for example) tokens between them
-            let length = typeOffset - previousInternalByteRange.location
-            let range = ByteRange(location: previousInternalByteRange.location, length: length)
-            let internalDoesntBelongToType = Set(file.syntaxMap.kinds(inByteRange: range)) != [.attributeBuiltin]
+        override func visitPost(_ node: ClassDeclSyntax) {
+            declScope.pop()
+        }
 
-            return internalDoesntBelongToType ? typeOffset : nil
+        override func visit(_ node: ClosureExprSyntax) -> SyntaxVisitorContinueKind {
+            .skipChildren
         }
-    }
 
-    func validate(file: SwiftLintFile) -> [StyleViolation] {
-        let implicitAndExplicitInternalElements = internalTypeElements(in: file.structureDictionary)
+        override func visit(_ node: CodeBlockSyntax) -> SyntaxVisitorContinueKind {
+            .skipChildren
+        }
 
-        guard implicitAndExplicitInternalElements.isNotEmpty else {
-            return []
+        override func visit(_ node: ExtensionDeclSyntax) -> SyntaxVisitorContinueKind {
+            declScope.push(node.modifiers.accessLevelModifier != nil ? .no : .yes)
+            return node.modifiers.containsPrivateOrFileprivate() ? .skipChildren : .visitChildren
         }
 
-        let explicitInternalRanges = findAllExplicitInternalTokens(in: file)
+        override func visitPost(_ node: ExtensionDeclSyntax) {
+            declScope.pop()
+        }
 
-        let violations = offsetOfElements(from: implicitAndExplicitInternalElements, in: file,
-                                          thatAreNotInRanges: explicitInternalRanges)
+        override func visit(_ node: EnumDeclSyntax) -> SyntaxVisitorContinueKind {
+            collectViolations(decl: node, token: node.enumKeyword)
+            declScope.push(.yes)
+            return node.modifiers.containsPrivateOrFileprivate() ? .skipChildren : .visitChildren
+        }
 
-        return violations.map {
-            StyleViolation(ruleDescription: Self.description,
-                           severity: configuration.severity,
-                           location: Location(file: file, byteOffset: $0))
+        override func visitPost(_ node: EnumDeclSyntax) {
+            declScope.pop()
         }
-    }
 
-    private func lastInternalByteRange(before typeOffset: ByteCount, in ranges: [ByteRange]) -> ByteRange? {
-        let firstPartition = ranges.prefix(while: { typeOffset > $0.location })
-        return firstPartition.last
-    }
+        override func visitPost(_ node: FunctionDeclSyntax) {
+            collectViolations(decl: node, token: node.staticOrClassKeyword ?? node.funcKeyword)
+        }
 
-    private func internalTypeElements(in parent: SourceKittenElement) -> [SourceKittenElement] {
-        return parent.substructure.flatMap { element -> [SourceKittenElement] in
-            guard let elementKind = element.declarationKind,
-                  elementKind != .varLocal, elementKind != .varParameter else {
-                return []
-            }
+        override func visitPost(_ node: InitializerDeclSyntax) {
+            collectViolations(decl: node, token: node.initKeyword)
+        }
 
-            let isDeinit = elementKind == .functionMethodInstance && element.name == "deinit"
-            guard !isDeinit else {
-                return []
-            }
+        override func visitPost(_ node: ProtocolDeclSyntax) {
+            collectViolations(decl: node, token: node.protocolKeyword)
+        }
 
-            let isPrivate = element.accessibility?.isPrivate ?? false
-            let internalTypeElementsInSubstructure = elementKind.childsAreExemptFromACL || isPrivate ? [] :
-                internalTypeElements(in: element)
+        override func visit(_ node: StructDeclSyntax) -> SyntaxVisitorContinueKind {
+            collectViolations(decl: node, token: node.structKeyword)
+            declScope.push(.yes)
+            return node.modifiers.containsPrivateOrFileprivate() ? .skipChildren : .visitChildren
+        }
 
-            var isInExtension = false
-            if let kind = parent.declarationKind {
-                isInExtension = SwiftDeclarationKind.extensionKinds.contains(kind)
-            }
+        override func visitPost(_ node: StructDeclSyntax) {
+            declScope.pop()
+        }
 
-            if element.accessibility == .internal || (element.accessibility == nil && isInExtension) {
-                return internalTypeElementsInSubstructure + [element]
-            }
+        override func visitPost(_ node: SubscriptDeclSyntax) {
+            collectViolations(decl: node, token: node.staticOrClassKeyword ?? node.subscriptKeyword)
+        }
+
+        override func visitPost(_ node: TypeAliasDeclSyntax) {
+            collectViolations(decl: node, token: node.typealiasKeyword)
+        }
 
-            return internalTypeElementsInSubstructure
+        override func visitPost(_ node: VariableDeclSyntax) {
+            collectViolations(decl: node, token: node.staticOrClassKeyword ?? node.bindingSpecifier)
+        }
+
+        private func collectViolations(decl: some WithModifiersSyntax, token: TokenSyntax) {
+            let aclModifiers = decl.modifiers.filter { $0.asAccessLevelModifier != nil }
+            if declScope.peek() != .no, aclModifiers.isEmpty || aclModifiers.allSatisfy({ $0.detail != nil }) {
+                violations.append(token.positionAfterSkippingLeadingTrivia)
+            }
         }
     }
 }
 
-private extension SwiftDeclarationKind {
-    var childsAreExemptFromACL: Bool {
-        switch self {
-        case .associatedtype, .enumcase, .enumelement, .functionAccessorAddress,
-             .functionAccessorDidset, .functionAccessorGetter, .functionAccessorMutableaddress,
-             .functionAccessorSetter, .functionAccessorWillset, .genericTypeParam, .module,
-             .precedenceGroup, .varLocal, .varParameter, .varClass,
-             .varGlobal, .varInstance, .varStatic, .typealias, .functionAccessorModify, .functionAccessorRead,
-             .functionConstructor, .functionDestructor, .functionFree, .functionMethodClass,
-             .functionMethodInstance, .functionMethodStatic, .functionOperator, .functionOperatorInfix,
-             .functionOperatorPostfix, .functionOperatorPrefix, .functionSubscript, .protocol, .opaqueType:
-            return true
-        case .actor, .class, .enum, .extension, .extensionClass, .extensionEnum,
-             .extensionProtocol, .extensionStruct, .struct:
-            return false
-        }
+private extension WithModifiersSyntax {
+    var staticOrClassKeyword: TokenSyntax? {
+        modifiers.first { [.keyword(.static), .keyword(.class)].contains($0.name.tokenKind) }?.name
     }
 }