From 71a284049f80d2b8edcdcc6ceb16bfbde739a29a Mon Sep 17 00:00:00 2001
From: Timofey Solonin <tim-solonin2@yandex.ru>
Date: Wed, 14 Nov 2018 11:57:12 +0300
Subject: [PATCH] #2435 - Adjust modifier_order rule to require explicit
 modifier order specified to conclude a violation

---
 .../Rules/Style/ModifierOrderRule.swift       | 106 ++++++++++++++----
 .../ModifierOrderTests.swift                  |   2 +-
 2 files changed, 87 insertions(+), 21 deletions(-)

diff --git a/Source/SwiftLintFramework/Rules/Style/ModifierOrderRule.swift b/Source/SwiftLintFramework/Rules/Style/ModifierOrderRule.swift
index 83a3a622047..b7d10f6747f 100644
--- a/Source/SwiftLintFramework/Rules/Style/ModifierOrderRule.swift
+++ b/Source/SwiftLintFramework/Rules/Style/ModifierOrderRule.swift
@@ -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() {}
 
@@ -181,24 +192,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)]
+        }
     }
 }
 
@@ -216,9 +258,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
             }
@@ -240,3 +288,21 @@ private extension String {
         return components(separatedBy: charachter).last ?? ""
     }
 }
+
+private final class ModifierDescription: Equatable {
+    let keyword: String
+    let group: SwiftDeclarationAttributeKind.ModifierGroup
+
+    init(
+        keyword: String,
+        group: SwiftDeclarationAttributeKind.ModifierGroup
+    ) {
+        self.keyword = keyword
+        self.group = group
+    }
+
+    static func == (lhs: ModifierDescription, rhs: ModifierDescription) -> Bool {
+        return lhs.keyword == rhs.keyword
+            && lhs.group == rhs.group
+    }
+}
diff --git a/Tests/SwiftLintFrameworkTests/ModifierOrderTests.swift b/Tests/SwiftLintFrameworkTests/ModifierOrderTests.swift
index 95ab3b01293..fedaeb23b47 100644
--- a/Tests/SwiftLintFrameworkTests/ModifierOrderTests.swift
+++ b/Tests/SwiftLintFrameworkTests/ModifierOrderTests.swift
@@ -142,7 +142,7 @@ class ModifierOrderTests: XCTestCase {
         )
 
         verifyRule(descriptionOverride,
-                   ruleConfiguration: ["preferred_modifier_order": ["override", "acl", "final"]])
+                   ruleConfiguration: ["preferred_modifier_order": ["override", "acl", "owned", "final"]])
     }
 
     func testViolationMessage() {