diff --git a/CHANGELOG.md b/CHANGELOG.md index 53577245e..caae0e1b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -141,6 +141,7 @@ - `redundant_discardable_let` - `redundant_nil_coalescing` - `redundant_objc_attribute` + - `redundant_set_access_control` - `redundant_string_enum_value` - `required_deinit` - `return_arrow_whitespace` diff --git a/Source/SwiftLintFramework/Extensions/Dictionary+SwiftLint.swift b/Source/SwiftLintFramework/Extensions/Dictionary+SwiftLint.swift index 6b17682fa..de186ad93 100644 --- a/Source/SwiftLintFramework/Extensions/Dictionary+SwiftLint.swift +++ b/Source/SwiftLintFramework/Extensions/Dictionary+SwiftLint.swift @@ -227,31 +227,6 @@ extension SourceKittenDictionary { } } - /// Traversing all substuctures of the dictionary hierarchically, calling `traverseBlock` on each node. - /// Traversing using depth first strategy, so deepest substructures will be passed to `traverseBlock` first. - /// - /// - parameter traverseBlock: block that will be called for each substructure and its parent. - /// - /// - returns: The list of substructure dictionaries with updated values from the traverse block. - func traverseWithParentDepthFirst(traverseBlock: (SourceKittenDictionary, SourceKittenDictionary) -> [T]?) - -> [T] { - var result: [T] = [] - traverseWithParentDepthFirst(collectingValuesInto: &result, traverseBlock: traverseBlock) - return result - } - - private func traverseWithParentDepthFirst( - collectingValuesInto array: inout [T], - traverseBlock: (SourceKittenDictionary, SourceKittenDictionary) -> [T]?) { - substructure.forEach { subDict in - subDict.traverseWithParentDepthFirst(collectingValuesInto: &array, traverseBlock: traverseBlock) - - if let collectedValues = traverseBlock(self, subDict) { - array += collectedValues - } - } - } - /// Traversing all entities of the dictionary hierarchically, calling `traverseBlock` on each node. /// Traversing using depth first strategy, so deepest substructures will be passed to `traverseBlock` first. /// diff --git a/Source/SwiftLintFramework/Rules/Idiomatic/RedundantSetAccessControlRule.swift b/Source/SwiftLintFramework/Rules/Idiomatic/RedundantSetAccessControlRule.swift index ae2bd2729..48255f7de 100644 --- a/Source/SwiftLintFramework/Rules/Idiomatic/RedundantSetAccessControlRule.swift +++ b/Source/SwiftLintFramework/Rules/Idiomatic/RedundantSetAccessControlRule.swift @@ -1,6 +1,6 @@ -import SourceKittenFramework +import SwiftSyntax -public struct RedundantSetAccessControlRule: ConfigurationProviderRule { +public struct RedundantSetAccessControlRule: ConfigurationProviderRule, SwiftSyntaxRule { public var configuration = SeverityConfiguration(.warning) public init() {} @@ -20,6 +20,16 @@ public struct RedundantSetAccessControlRule: ConfigurationProviderRule { private final class A { private(set) var value: Int } + """), + Example(""" + fileprivate class A { + public fileprivate(set) var value: Int + } + """, excludeFromDocumentation: true), + Example(""" + extension Color { + public internal(set) static var someColor = Color.anotherColor + } """) ], triggeringExamples: [ @@ -38,6 +48,11 @@ public struct RedundantSetAccessControlRule: ConfigurationProviderRule { } """), Example(""" + internal class A { + ↓internal(set) var value: Int + } + """), + Example(""" fileprivate class A { ↓fileprivate(set) var value: Int } @@ -45,80 +60,94 @@ public struct RedundantSetAccessControlRule: ConfigurationProviderRule { ] ) - public func validate(file: SwiftLintFile) -> [StyleViolation] { - return file.structureDictionary.traverseWithParentDepthFirst { parent, subDict in - guard let kind = subDict.declarationKind else { return nil } - return validate(file: file, kind: kind, dictionary: subDict, parentDictionary: parent) - } - } - - private func validate(file: SwiftLintFile, kind: SwiftDeclarationKind, - dictionary: SourceKittenDictionary, - parentDictionary: SourceKittenDictionary?) -> [StyleViolation] { - let aclAttributes: Set = [.private, .fileprivate, .internal, .public, .open] - let explicitACL = dictionary.swiftAttributes.compactMap { dict -> SwiftDeclarationAttributeKind? in - guard let attribute = dict.attribute.flatMap(SwiftDeclarationAttributeKind.init), - aclAttributes.contains(attribute) else { - return nil - } - - return attribute - }.first - - let acl = dictionary.accessibility - let resolvedAccessibility: AccessControlLevel? = explicitACL?.acl ?? { - let parentACL = parentDictionary?.accessibility - - if acl == .internal, let parentACL = parentACL, parentACL == .fileprivate { - return .fileprivate - } else { - return acl - } - }() - - guard SwiftDeclarationKind.variableKinds.contains(kind), - resolvedAccessibility?.rawValue == dictionary.setterAccessibility else { - return [] - } - - let explicitSetACL = dictionary.swiftAttributes.first { dict in - return dict.attribute?.hasPrefix("source.decl.attribute.setter_access") ?? false - } - - guard let offset = explicitSetACL?.offset else { - return [] - } - - // if it's an inferred `private`, it means the variable is actually inside a fileprivate structure - if dictionary.accessibility == .private, - explicitACL == nil, - dictionary.setterAccessibility.flatMap(AccessControlLevel.init(identifier:)) == .private { - return [] - } - - return [ - StyleViolation(ruleDescription: Self.description, - severity: configuration.severity, - location: Location(file: file, byteOffset: offset)) - ] + public func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor { + Visitor(viewMode: .sourceAccurate) } } -private extension SwiftDeclarationAttributeKind { - var acl: AccessControlLevel? { - switch self { - case .private: - return .private - case .fileprivate: - return .fileprivate - case .internal: - return .internal - case .public: - return .public - case .open: - return .open - default: - return nil +private extension RedundantSetAccessControlRule { + final class Visitor: ViolationsSyntaxVisitor { + override var skippableDeclarations: [DeclSyntaxProtocol.Type] { + [FunctionDeclSyntax.self] + } + + override func visitPost(_ node: VariableDeclSyntax) { + guard let modifiers = node.modifiers, let setAccessor = modifiers.setAccessor else { + return + } + + let uniqueModifiers = Set(modifiers.map(\.name.tokenKind)) + if uniqueModifiers.count != modifiers.count { + violations.append(modifiers.positionAfterSkippingLeadingTrivia) + return + } + + if setAccessor.name.tokenKind == .fileprivateKeyword, + modifiers.getAccessor == nil, + let closestDeclModifiers = node.closestDecl()?.modifiers { + let closestDeclIsFilePrivate = closestDeclModifiers.contains { + $0.name.tokenKind == .fileprivateKeyword + } + + if closestDeclIsFilePrivate { + violations.append(modifiers.positionAfterSkippingLeadingTrivia) + return + } + } + + if setAccessor.name.tokenKind == .internalKeyword, + modifiers.getAccessor == nil, + let closesDecl = node.closestDecl(), + let closestDeclModifiers = closesDecl.modifiers { + let closestDeclIsInternal = closestDeclModifiers.isEmpty || closestDeclModifiers.contains { + $0.name.tokenKind == .internalKeyword + } + + if closestDeclIsInternal { + violations.append(modifiers.positionAfterSkippingLeadingTrivia) + return + } + } } } } + +private extension SyntaxProtocol { + func closestDecl() -> DeclSyntax? { + if let decl = self.parent?.as(DeclSyntax.self) { + return decl + } + + return parent?.closestDecl() + } +} + +private extension DeclSyntax { + var modifiers: ModifierListSyntax? { + if let decl = self.as(ClassDeclSyntax.self) { + return decl.modifiers ?? ModifierListSyntax([]) + } else if let decl = self.as(ActorDeclSyntax.self) { + return decl.modifiers ?? ModifierListSyntax([]) + } else if let decl = self.as(StructDeclSyntax.self) { + return decl.modifiers ?? ModifierListSyntax([]) + } else if let decl = self.as(ProtocolDeclSyntax.self) { + return decl.modifiers ?? ModifierListSyntax([]) + } else if let decl = self.as(ExtensionDeclSyntax.self) { + return decl.modifiers ?? ModifierListSyntax([]) + } else if let decl = self.as(EnumDeclSyntax.self) { + return decl.modifiers ?? ModifierListSyntax([]) + } + + return nil + } +} + +private extension ModifierListSyntax { + var setAccessor: DeclModifierSyntax? { + first { $0.detail?.detail.tokenKind == .contextualKeyword("set") } + } + + var getAccessor: DeclModifierSyntax? { + first { $0.detail == nil } + } +}