Use effective ACL in discouraged_default_parameter rule (#6563)

2026-05-07 20:12:49 +00:00 · 2026-03-27 21:25:37 +01:00
parent 29a71dd50d
commit 99ec4923d8
5 changed files with 302 additions and 186 deletions
@@ -17,6 +17,9 @@ struct DiscouragedDefaultParameterRule: Rule {
        nonTriggeringExamples: [
            Example("public func foo(bar: Int = 0) {}"),
            Example("open func foo(bar: Int = 0) {}"),
+            Example("public extension Foo { func foo(bar: Int = 0) {} }"),
+            Example("extension E { public func foo(bar: Int = 0) {} }"),
+            Example("func outer() { func inner(bar: Int = 0) {} }"),
            Example("func foo(bar: Int) {}"),
            Example("private func foo(bar: Int = 0) {}"),
            Example("fileprivate func foo(bar: Int = 0) {}"),
@@ -32,6 +35,8 @@ struct DiscouragedDefaultParameterRule: Rule {
            Example("package func foo(bar: Int ↓= 0) {}"),
            Example("func foo(bar: Int ↓= 0, baz: String ↓= \"\") {}"),
            Example("init(value: Int ↓= 42) {}"),
+            Example("class C { public func foo(bar: Int ↓= 0) {} }"),
+            Example("struct S { public init(value: Int ↓= 42) {} }"),
            Example(
                "private func foo(bar: Int ↓= 0) {}",
                configuration: ["disallowed_access_levels": ["private"]]
@@ -45,47 +50,43 @@ struct DiscouragedDefaultParameterRule: Rule {
 }

 private extension DiscouragedDefaultParameterRule {
-    final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
-        override func visitPost(_ node: FunctionDeclSyntax) {
-            collectViolations(modifiers: node.modifiers, parameterClause: node.signature.parameterClause)
+    final class Visitor: EffectiveAccessControlSyntaxVisitor<ConfigurationType> {
+        init(configuration: ConfigurationType, file: SwiftLintFile) {
+            super.init(configuration: configuration, file: file)
        }

-        override func visitPost(_ node: InitializerDeclSyntax) {
+        override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind {
            collectViolations(modifiers: node.modifiers, parameterClause: node.signature.parameterClause)
+            return .skipChildren
        }

-        override func visitPost(_ node: SubscriptDeclSyntax) {
+        override func visit(_ node: InitializerDeclSyntax) -> SyntaxVisitorContinueKind {
+            collectViolations(modifiers: node.modifiers, parameterClause: node.signature.parameterClause)
+            return .skipChildren
+        }
+
+        override func visit(_ node: SubscriptDeclSyntax) -> SyntaxVisitorContinueKind {
            collectViolations(modifiers: node.modifiers, parameterClause: node.parameterClause)
+            return .skipChildren
        }

-        private func collectViolations(
-            modifiers: DeclModifierListSyntax,
-            parameterClause: FunctionParameterClauseSyntax
-        ) {
-            guard let accessLevel = effectiveAccessLevel(modifiers),
+        private func collectViolations(modifiers: DeclModifierListSyntax,
+                                       parameterClause: FunctionParameterClauseSyntax) {
+            guard !isInLocalAccessControlScope,
+                  case let accessLevel = effectiveAccessControlLevel(for: modifiers),
                  configuration.disallowedAccessLevels.contains(accessLevel) else {
                return
            }
-            let levelName = accessLevel.rawValue
            for param in parameterClause.parameters {
                if let defaultValue = param.defaultValue {
                    violations.append(
-                        ReasonedRuleViolation(
+                        .init(
                            position: defaultValue.positionAfterSkippingLeadingTrivia,
-                            reason: "Default parameter values should not be used in '\(levelName)' functions"
+                            reason: "Default parameter values should not be used in '\(accessLevel)' functions"
                        )
                    )
                }
            }
        }
-
-        private func effectiveAccessLevel(_ modifiers: DeclModifierListSyntax)
-            -> DiscouragedDefaultParameterConfiguration.AccessLevel? {
-            if modifiers.contains(keyword: .private) { return .private }
-            if modifiers.contains(keyword: .fileprivate) { return .fileprivate }
-            if modifiers.contains(keyword: .package) { return .package }
-            if modifiers.contains(keyword: .public) || modifiers.contains(keyword: .open) { return nil }
-            return .internal
-        }
    }
 }
@@ -16,25 +16,22 @@ struct MissingDocsRule: Rule {
 }

 private extension MissingDocsRule {
-    final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
-        private var aclScope = Stack<AccessControlBehavior>()
+    final class Visitor: EffectiveAccessControlSyntaxVisitor<ConfigurationType> {
+        init(configuration: ConfigurationType, file: SwiftLintFile) {
+            super.init(
+                configuration: configuration,
+                file: file,
+                evaluateEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
+            )
+        }

        override func visit(_ node: ActorDeclSyntax) -> SyntaxVisitorContinueKind {
-            defer {
-                aclScope.push(
-                    behavior: .actor(node.modifiers.accessibility),
-                    evalEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
-                )
-            }
            if node.inherits, configuration.excludesInheritedTypes {
+                _ = super.visit(node)
                return .skipChildren
            }
            collectViolation(from: node, on: node.actorKeyword)
-            return .visitChildren
-        }
-
-        override func visitPost(_: ActorDeclSyntax) {
-            aclScope.pop()
+            return super.visit(node)
        }

        override func visitPost(_ node: AssociatedTypeDeclSyntax) {
@@ -42,21 +39,12 @@ private extension MissingDocsRule {
        }

        override func visit(_ node: ClassDeclSyntax) -> SyntaxVisitorContinueKind {
-            defer {
-                aclScope.push(
-                    behavior: .class(node.modifiers.accessibility),
-                    evalEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
-                )
-            }
            if node.inherits, configuration.excludesInheritedTypes {
+                _ = super.visit(node)
                return .skipChildren
            }
            collectViolation(from: node, on: node.classKeyword)
-            return .visitChildren
-        }
-
-        override func visitPost(_: ClassDeclSyntax) {
-            aclScope.pop()
+            return super.visit(node)
        }

        override func visit(_: ClosureExprSyntax) -> SyntaxVisitorContinueKind {
@@ -68,15 +56,14 @@ private extension MissingDocsRule {
        }

        override func visitPost(_ node: EnumCaseDeclSyntax) {
-            guard !node.hasDocComment, case let .enum(enumAcl) = aclScope.peek() else {
+            guard !node.hasDocComment, let enumAccessControlLevel else {
                return
            }
-            let acl = enumAcl ?? .internal
-            if let parameter = configuration.parameters.first(where: { $0.value == acl }) {
+            if let parameter = configuration.parameters.first(where: { $0.value == enumAccessControlLevel }) {
                violations.append(
-                    ReasonedRuleViolation(
+                    .init(
                        position: node.caseKeyword.positionAfterSkippingLeadingTrivia,
-                        reason: "\(acl) declarations should be documented",
+                        reason: "\(enumAccessControlLevel) declarations should be documented",
                        severity: parameter.severity
                    )
                )
@@ -84,36 +71,23 @@ private extension MissingDocsRule {
        }

        override func visit(_ node: EnumDeclSyntax) -> SyntaxVisitorContinueKind {
-            defer {
-                aclScope.push(
-                    behavior: .enum(node.modifiers.accessibility),
-                    evalEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
-                )
-            }
            if node.inherits, configuration.excludesInheritedTypes {
+                _ = super.visit(node)
                return .skipChildren
            }
            collectViolation(from: node, on: node.enumKeyword)
-            return .visitChildren
-        }
-
-        override func visitPost(_: EnumDeclSyntax) {
-            aclScope.pop()
+            return super.visit(node)
        }

        override func visit(_ node: ExtensionDeclSyntax) -> SyntaxVisitorContinueKind {
-            defer { aclScope.push(.extension(node.modifiers.accessibility)) }
            if node.inherits, configuration.excludesInheritedTypes {
+                _ = super.visit(node)
                return .skipChildren
            }
            if !configuration.excludesExtensions {
                collectViolation(from: node, on: node.extensionKeyword)
            }
-            return .visitChildren
-        }
-
-        override func visitPost(_: ExtensionDeclSyntax) {
-            aclScope.pop()
+            return super.visit(node)
        }

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

        override func visit(_ node: ProtocolDeclSyntax) -> SyntaxVisitorContinueKind {
-            defer {
-                aclScope.push(
-                    behavior: .protocol(node.modifiers.accessibility),
-                    evalEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
-                )
-            }
            if node.inherits, configuration.excludesInheritedTypes {
+                _ = super.visit(node)
                return .skipChildren
            }
            collectViolation(from: node, on: node.protocolKeyword)
-            return .visitChildren
-        }
-
-        override func visitPost(_: ProtocolDeclSyntax) {
-            aclScope.pop()
+            return super.visit(node)
        }

        override func visit(_ node: StructDeclSyntax) -> SyntaxVisitorContinueKind {
-            defer {
-                aclScope.push(
-                    behavior: .struct(node.modifiers.accessibility),
-                    evalEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
-                )
-            }
            if node.inherits, configuration.excludesInheritedTypes {
+                _ = super.visit(node)
                return .skipChildren
            }
            collectViolation(from: node, on: node.structKeyword)
-            return .visitChildren
-        }
-
-        override func visitPost(_: StructDeclSyntax) {
-            aclScope.pop()
+            return super.visit(node)
        }

        override func visit(_ node: SubscriptDeclSyntax) -> SyntaxVisitorContinueKind {
@@ -182,10 +138,7 @@ private extension MissingDocsRule {
            if node.modifiers.contains(keyword: .override) || node.hasDocComment {
                return
            }
-            let acl = aclScope.computeAcl(
-                givenExplicitAcl: node.modifiers.accessibility,
-                evalEffectiveAcl: configuration.evaluateEffectiveAccessControlLevel
-            )
+            let acl = effectiveAccessControlLevel(for: node.modifiers)
            if let parameter = configuration.parameters.first(where: { $0.value == acl }) {
                violations.append(
                    ReasonedRuleViolation(
@@ -230,94 +183,3 @@ private extension SyntaxProtocol {
        }
    }
 }
-
-private extension DeclModifierListSyntax {
-    var accessibility: AccessControlLevel? {
-        filter { $0.detail == nil }.compactMap { AccessControlLevel(description: $0.name.text) }.first
-    }
-}
-
-private enum AccessControlBehavior {
-    case `actor`(AccessControlLevel?)
-    case local
-    case `class`(AccessControlLevel?)
-    case `enum`(AccessControlLevel?)
-    case `extension`(AccessControlLevel?)
-    case `protocol`(AccessControlLevel?)
-    case `struct`(AccessControlLevel?)
-
-    var effectiveAcl: AccessControlLevel {
-        explicitAcl ?? .internal
-    }
-
-    var explicitAcl: AccessControlLevel? {
-        switch self {
-        case let .actor(acl): acl
-        case .local: nil
-        case let .class(acl): acl
-        case let .enum(acl): acl
-        case let .extension(acl): acl
-        case let .protocol(acl): acl
-        case let .struct(acl): acl
-        }
-    }
-
-    func sameWith(acl: AccessControlLevel) -> Self {
-        switch self {
-        case .actor: .actor(acl)
-        case .local: .local
-        case .class: .class(acl)
-        case .enum: .enum(acl)
-        case .extension: .extension(acl)
-        case .protocol: .protocol(acl)
-        case .struct: .struct(acl)
-        }
-    }
-}
-
-/// Implementation of Swift's effective ACL logic. Should be moved to a specialized syntax visitor for reuse some time.
-private extension Stack<AccessControlBehavior> {
-    mutating func push(behavior: AccessControlBehavior, evalEffectiveAcl: Bool) {
-        if let parentBehavior = peek() {
-            switch parentBehavior {
-            case .local:
-                push(.local)
-            case .actor, .class, .struct, .enum:
-                if behavior.effectiveAcl <= parentBehavior.effectiveAcl || !evalEffectiveAcl {
-                    push(behavior)
-                } else {
-                    push(behavior.sameWith(acl: parentBehavior.effectiveAcl))
-                }
-            case .extension, .protocol:
-                if behavior.explicitAcl != nil {
-                    push(behavior)
-                } else {
-                    push(behavior.sameWith(acl: parentBehavior.effectiveAcl))
-                }
-            }
-        } else {
-            push(behavior)
-        }
-    }
-
-    func computeAcl(givenExplicitAcl acl: AccessControlLevel?, evalEffectiveAcl: Bool) -> AccessControlLevel {
-        if let parentBehavior = peek() {
-            switch parentBehavior {
-            case .local:
-                .private
-            case .actor, .class, .struct, .enum:
-                if let acl {
-                    acl < parentBehavior.effectiveAcl || !evalEffectiveAcl ? acl : parentBehavior.effectiveAcl
-                } else {
-                    parentBehavior.effectiveAcl >= .internal ? .internal : parentBehavior.effectiveAcl
-                }
-            case .protocol:
-                parentBehavior.effectiveAcl
-            case .extension:
-                acl ?? parentBehavior.effectiveAcl
-            }
-        } else {
-            acl ?? .internal
-        }
-    }
-}
@@ -5,7 +5,7 @@ struct DiscouragedDefaultParameterConfiguration: SeverityBasedRuleConfiguration
    @ConfigurationElement(key: "severity")
    private(set) var severityConfiguration = SeverityConfiguration<Parent>(.warning)
    @ConfigurationElement(key: "disallowed_access_levels")
-    private(set) var disallowedAccessLevels: Set<AccessLevel> = [.internal, .package]
+    private(set) var disallowedAccessLevels: Set<AccessControlLevel> = [.internal, .package]

    @AcceptableByConfigurationElement
    enum AccessLevel: String, Comparable {