Skip to content

Commit

Permalink
#2435 - Make modifier_order rule rely on an explicit set of rules (#2458
Browse files Browse the repository at this point in the history 
)

* #2435 - Adjust modifier_order rule to require explicit modifier order specified to conclude a violation

* #2435 - Move modifier order rule examples to a separate file

* #2435 - Add modifier interference tests

* #2435 - Fix whitespaces

* Minor edits

* Add changelog entry
  • Loading branch information
@abdulowork @jpsim
abdulowork authored and jpsim committed Nov 28, 2018
1 parent b66446c commit efa6817
Show file tree
Hide file tree
Showing 6 changed files with 302 additions and 178 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@
[Cihat Gündüz](https://github.com/Dschee)
[#2306](https://github.com/realm/SwiftLint/issues/2306)

* Ignore unspecified modifiers in `modifier_order`.
[Timofey Solonin](https://github.com/biboran)
[#2435](https://github.com/realm/SwiftLint/issues/2435)

#### Bug Fixes

* Fix false positive in `nimble_operator` rule.
Expand Down
252 changes: 75 additions & 177 deletions Source/SwiftLintFramework/Rules/Style/ModifierOrderRule.swift
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,21 @@
import SourceKittenFramework

private typealias ModifierKeyword = String
private typealias ModifierDescription = (ModifierKeyword, SwiftDeclarationAttributeKind.ModifierGroup)

public struct ModifierOrderRule: ASTRule, OptInRule, ConfigurationProviderRule {
public var configuration = ModifierOrderConfiguration(preferredModifierOrder: [.override, .acl])
public var configuration = ModifierOrderConfiguration(
preferredModifierOrder: [
.override,
.acl,
.setterACL,
.dynamic,
.mutators,
.lazy,
.final,
.required,
.convenience,
.typeMethods,
.owned
]
)

public init() {}

Expand All @@ -14,163 +25,8 @@ public struct ModifierOrderRule: ASTRule, OptInRule, ConfigurationProviderRule {
description: "Modifier order should be consistent.",
kind: .style,
minSwiftVersion: .fourDotOne ,
nonTriggeringExamples: [
"public class Foo { \n" +
" public convenience required init() {} \n" +
"}",
"public class Foo { \n" +
" public static let bar = 42 \n" +
"}",
"public class Foo { \n" +
" public static var bar: Int { \n" +
" return 42" +
" }" +
"}",
"public class Foo { \n" +
" public class var bar: Int { \n" +
" return 42 \n" +
" } \n" +
"}",
"public class Bar { \n" +
" public class var foo: String { \n" +
" return \"foo\" \n" +
" } \n" +
"} \n" +
"public class Foo: Bar { \n" +
" override public final class var foo: String { \n" +
" return \"bar\" \n" +
" } \n" +
"}",
"open class Bar { \n" +
" public var foo: Int? { \n" +
" return 42 \n" +
" } \n" +
"} \n" +
"open class Foo: Bar { \n" +
" override public var foo: Int? { \n" +
" return 43 \n" +
" } \n" +
"}",
"open class Bar { \n" +
" open class func foo() -> Int { \n" +
" return 42 \n" +
" } \n" +
"} \n" +
"class Foo: Bar { \n" +
" override open class func foo() -> Int { \n" +
" return 43 \n" +
" } \n" +
"}",
"protocol Foo: class {} \n" +
"class Bar { \n" +
" public private(set) weak var foo: Foo? \n" +
"} \n",
"@objc \n" +
"public final class Foo: NSObject {} \n",
"@objcMembers \n" +
"public final class Foo: NSObject {} \n",
"@objc \n" +
"override public private(set) weak var foo: Bar? \n",
"@objc \n" +
"public final class Foo: NSObject {} \n",
"@objc \n" +
"open final class Foo: NSObject { \n" +
" open weak var weakBar: NSString? = nil \n" +
"}",
"public final class Foo {}",
"class Bar { \n" +
" func bar() {} \n" +
"}",
"internal class Foo: Bar { \n" +
" override internal func bar() {} \n" +
"}",
"public struct Foo { \n" +
" internal weak var weakBar: NSObject? = nil \n" +
"}",
"class Foo { \n" +
" internal lazy var bar: String = \"foo\" \n" +
"}"
],
triggeringExamples: [
"class Foo { \n" +
" convenience required public init() {} \n" +
"}",
"public class Foo { \n" +
" static public let bar = 42 \n" +
"}",
"public class Foo { \n" +
" static public var bar: Int { \n" +
" return 42 \n" +
" } \n" +
"} \n",
"public class Foo { \n" +
" class public var bar: Int { \n" +
" return 42 \n" +
" } \n" +
"}",
"public class RootFoo { \n" +
" class public var foo: String { \n" +
" return \"foo\" \n" +
" } \n" +
"} \n" +
"public class Foo: RootFoo { \n" +
" override final class public var foo: String { \n" +
" return \"bar\" \n" +
" } \n" +
"}",
"open class Bar { \n" +
" public var foo: Int? { \n" +
" return 42 \n" +
" } \n" +
"} \n" +
"open class Foo: Bar { \n" +
" public override var foo: Int? { \n" +
" return 43 \n" +
" } \n" +
"}",
"protocol Foo: class {} \n" +
"class Bar { \n" +
" private(set) public weak var foo: Foo? \n" +
"} \n",
"open class Bar { \n" +
" open class func foo() -> Int { \n" +
" return 42 \n" +
" } \n" +
"} \n" +
"class Foo: Bar { \n" +
" class open override func foo() -> Int { \n" +
" return 43 \n" +
" } \n" +
"}",
"open class Bar { \n" +
" open class func foo() -> Int { \n" +
" return 42 \n" +
" } \n" +
"} \n" +
"class Foo: Bar { \n" +
" open override class func foo() -> Int { \n" +
" return 43 \n" +
" } \n" +
"}",
"@objc \n" +
"final public class Foo: NSObject {}",
"@objcMembers \n" +
"final public class Foo: NSObject {}",
"@objc \n" +
"final open class Foo: NSObject { \n" +
" weak open var weakBar: NSString? = nil \n" +
"}",
"final public class Foo {} \n",
"internal class Foo: Bar { \n" +
" internal override func bar() {} \n" +
"}",
"public struct Foo { \n" +
" weak internal var weakBar: NSObjetc? = nil \n" +
"}",
"class Foo { \n" +
" lazy internal var bar: String = \"foo\" \n" +
"}"
]
nonTriggeringExamples: ModifierOrderRuleExamples.nonTriggeringExamples,
triggeringExamples: ModifierOrderRuleExamples.triggeringExamples
)

public func validate(file: File,
Expand All @@ -181,24 +37,55 @@ public struct ModifierOrderRule: ASTRule, OptInRule, ConfigurationProviderRule {
return []
}

let preferredOrder = [.atPrefixed] + configuration.preferredModifierOrder
let declaredDescriptions = dictionary.modifierDescriptions
let preferredDescriptions = preferredOrder.reduce(into: [ModifierDescription]()) { descriptions, group in
if let description = declaredDescriptions.first(where: { $0.1 == group }) {
descriptions.append(description)
}
let violatableModifiers = self.violatableModifiers(declaredModifiers: dictionary.modifierDescriptions)
let prioritizedModifiers = self.prioritizedModifiers(violatableModifiers: violatableModifiers)
let sortedByPriorityModifiers = prioritizedModifiers.sorted(
by: { lhs, rhs in lhs.priority < rhs.priority }
).map { $0.modifier }

let violatingModifiers = zip(
sortedByPriorityModifiers,
violatableModifiers
).filter { sortedModifier, unsortedModifier in
return sortedModifier != unsortedModifier
}

for (index, preferredDescription) in preferredDescriptions.enumerated()
where preferredDescription.1 != declaredDescriptions[index].1 {
let reason = "\(preferredDescription.0) modifier should be before \(declaredDescriptions[index].0)."
return [StyleViolation(ruleDescription: type(of: self).description,
severity: configuration.severityConfiguration.severity,
location: Location(file: file, byteOffset: offset),
reason: reason)]
if let first = violatingModifiers.first {
let preferredModifier = first.0
let declaredModifier = first.1
let reason = "\(preferredModifier.keyword) modifier should be before \(declaredModifier.keyword)."
return [
StyleViolation(
ruleDescription: type(of: self).description,
severity: configuration.severityConfiguration.severity,
location: Location(file: file, byteOffset: offset),
reason: reason
)
]
} else {
return []
}
}

return []
private func violatableModifiers(declaredModifiers: [ModifierDescription]) -> [ModifierDescription] {
let preferredModifierGroups = ([.atPrefixed] + configuration.preferredModifierOrder)
return declaredModifiers.filter { preferredModifierGroups.contains($0.group) }
}

private func prioritizedModifiers(
violatableModifiers: [ModifierDescription]
) -> [(priority: Int, modifier: ModifierDescription)] {
let prioritizedPreferredModifierGroups = ([.atPrefixed] + configuration.preferredModifierOrder).enumerated()
return violatableModifiers.reduce(
[(priority: Int, modifier: ModifierDescription)]()
) { prioritizedModifiers, modifier in
guard let priority = prioritizedPreferredModifierGroups.first(
where: { _, group in modifier.group == group }
)?.offset else {
return prioritizedModifiers
}
return prioritizedModifiers + [(priority: priority, modifier: modifier)]
}
}
}

Expand All @@ -216,9 +103,15 @@ private extension Dictionary where Key == String, Value == SourceKitRepresentabl
.compactMap {
if let attribute = $0.attribute,
let modifierGroup = SwiftDeclarationAttributeKind.ModifierGroup(rawAttribute: attribute) {
return ModifierDescription(attribute.lastComponentAfter("."), modifierGroup)
return ModifierDescription(
keyword: attribute.lastComponentAfter("."),
group: modifierGroup
)
} else if let kind = $0.kind {
return ModifierDescription(kind.lastComponentAfter("."), .typeMethods)
return ModifierDescription(
keyword: kind.lastComponentAfter("."),
group: .typeMethods
)
}
return nil
}
Expand All @@ -240,3 +133,8 @@ private extension String {
return components(separatedBy: charachter).last ?? ""
}
}

private struct ModifierDescription: Equatable {
let keyword: String
let group: SwiftDeclarationAttributeKind.ModifierGroup
}
Loading

0 comments on commit efa6817

Please sign in to comment.