Make modifier_order rule autocorrectable (realm#2521)

* realm#2353 - Move violating modifiers search to a private function

* realm#2353 - Add offset and length to the ModifierDescription

* realm#2353 - Make modifier_oder rule correctable

* realm#2353 - Add modifier_oder rule correction tests

* realm#2353 - Upadte the changelog

* realm#2353 - Add missing Foundation import

* realm#2353 - Fix linux tests

* Small edits to ModifierOrderRule and changelog entry

CHANGELOG.md

@@ -37,6 +37,10 @@
 * Support glob patterns without the star.  
   [Maksym Grebenets](https://github.com/mgrebenets)
 
+* Make `modifier_order` rule autocorrectable.  
+  [Timofey Solonin](https://github.com/biboran)
+  [#2353](https://github.com/realm/SwiftLint/issues/2353)
+
 #### Bug Fixes
 
 * Fix false positives in `redundant_objc_attribute` for private declarations

Rules.md

@@ -10751,7 +10751,7 @@ public let b: Int
 
 Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version
 --- | --- | --- | --- | --- | ---
-`modifier_order` | Disabled | No | style | No | 4.1.0 
+`modifier_order` | Disabled | Yes | style | No | 4.1.0 
 
 Modifier order should be consistent.
 

Source/SwiftLintFramework/Extensions/File+SwiftLint.swift

@@ -267,6 +267,10 @@ extension File {
         return violatingRanges
     }
 
+    internal func ruleEnabled(violatingRange: NSRange, for rule: Rule) -> NSRange? {
+        return ruleEnabled(violatingRanges: [violatingRange], for: rule).first
+    }
+
     fileprivate func numberOfCommentAndWhitespaceOnlyLines(startLine: Int, endLine: Int) -> Int {
         let commentKinds = SyntaxKind.commentKinds
         return syntaxKindsByLines[startLine...endLine].filter { kinds in

Source/SwiftLintFramework/Rules/Style/ModifierOrderRule.swift

@@ -1,6 +1,7 @@
+import Foundation
 import SourceKittenFramework
 
-public struct ModifierOrderRule: ASTRule, OptInRule, ConfigurationProviderRule {
+public struct ModifierOrderRule: ASTRule, OptInRule, ConfigurationProviderRule, CorrectableRule {
     public var configuration = ModifierOrderConfiguration(
         preferredModifierOrder: [
             .override,
@@ -36,18 +37,7 @@ public struct ModifierOrderRule: ASTRule, OptInRule, ConfigurationProviderRule {
             return []
         }
 
-        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
-        }
+        let violatingModifiers = self.violatingModifiers(dictionary: dictionary)
 
         if let first = violatingModifiers.first {
             let preferredModifier = first.0
@@ -66,6 +56,72 @@ public struct ModifierOrderRule: ASTRule, OptInRule, ConfigurationProviderRule {
         }
     }
 
+    public func correct(file: File) -> [Correction] {
+        return correct(file: file, dictionary: file.structure.dictionary)
+    }
+
+    private func correct(file: File, dictionary: [String: SourceKitRepresentable]) -> [Correction] {
+        return dictionary.substructure.flatMap { subDict -> [Correction] in
+            var corrections = correct(file: file, dictionary: subDict)
+
+            if let kindString = subDict.kind,
+                let kind = KindType(rawValue: kindString) {
+                corrections += correct(file: file, kind: kind, dictionary: subDict)
+            }
+
+            return corrections
+        }
+    }
+
+    private func correct(file: File,
+                         kind: SwiftDeclarationKind,
+                         dictionary: [String: SourceKitRepresentable]) -> [Correction] {
+        guard let offset = dictionary.offset else { return [] }
+        let originalContents = file.contents.bridge()
+        let violatingRanges = violatingModifiers(dictionary: dictionary)
+            .compactMap { preferred, declared -> (NSRange, NSRange)? in
+                guard
+                    let preferredRange = originalContents.byteRangeToNSRange(
+                        start: preferred.offset,
+                        length: preferred.length
+                    ).flatMap({ file.ruleEnabled(violatingRange: $0, for: self) }),
+                    let declaredRange = originalContents.byteRangeToNSRange(
+                        start: declared.offset,
+                        length: declared.length
+                    ).flatMap({ file.ruleEnabled(violatingRange: $0, for: self) }) else {
+                    return nil
+                }
+                return (preferredRange, declaredRange)
+            }
+
+        let corrections: [Correction]
+        if violatingRanges.isEmpty {
+            corrections = []
+        } else {
+            var correctedContents = originalContents
+
+            violatingRanges.reversed().forEach { preferredModifierRange, declaredModifierRange in
+                correctedContents = correctedContents.replacingCharacters(
+                    in: declaredModifierRange,
+                    with: originalContents.substring(with: preferredModifierRange)
+                ).bridge()
+            }
+
+            file.write(correctedContents.bridge())
+
+            corrections = [
+                Correction(
+                    ruleDescription: type(of: self).description,
+                    location: Location(
+                        file: file,
+                        byteOffset: offset
+                    )
+                )
+            ]
+        }
+        return corrections
+    }
+
     private func violatableModifiers(declaredModifiers: [ModifierDescription]) -> [ModifierDescription] {
         let preferredModifierGroups = ([.atPrefixed] + configuration.preferredModifierOrder)
         return declaredModifiers.filter { preferredModifierGroups.contains($0.group) }
@@ -86,6 +142,18 @@ public struct ModifierOrderRule: ASTRule, OptInRule, ConfigurationProviderRule {
             return prioritizedModifiers + [(priority: priority, modifier: modifier)]
         }
     }
+
+    private func violatingModifiers(
+        dictionary: [String: SourceKitRepresentable]
+    ) -> [(preferredModifier: ModifierDescription, declaredModifier: ModifierDescription)] {
+        let violatableModifiers = self.violatableModifiers(declaredModifiers: dictionary.modifierDescriptions)
+        let prioritizedModifiers = self.prioritizedModifiers(violatableModifiers: violatableModifiers)
+        let sortedByPriorityModifiers = prioritizedModifiers
+            .sorted { $0.priority < $1.priority }
+            .map { $0.modifier }
+
+        return zip(sortedByPriorityModifiers, violatableModifiers).filter { $0 != $1 }
+    }
 }
 
 private extension Dictionary where Key == String, Value == SourceKitRepresentable {
@@ -100,16 +168,23 @@ private extension Dictionary where Key == String, Value == SourceKitRepresentabl
                 return rhsOffset < lhsOffset
             }
             .compactMap {
+                guard let offset = $0.offset else { return nil }
                 if let attribute = $0.attribute,
-                   let modifierGroup = SwiftDeclarationAttributeKind.ModifierGroup(rawAttribute: attribute) {
+                   let modifierGroup = SwiftDeclarationAttributeKind.ModifierGroup(rawAttribute: attribute),
+                   let length = $0.length {
                     return ModifierDescription(
                         keyword: attribute.lastComponentAfter("."),
-                        group: modifierGroup
+                        group: modifierGroup,
+                        offset: offset,
+                        length: length
                     )
                 } else if let kind = $0.kind {
+                    let keyword = kind.lastComponentAfter(".")
                     return ModifierDescription(
-                        keyword: kind.lastComponentAfter("."),
-                        group: .typeMethods
+                        keyword: keyword,
+                        group: .typeMethods,
+                        offset: offset,
+                        length: keyword.count
                     )
                 }
                 return nil
@@ -128,12 +203,14 @@ private extension Dictionary where Key == String, Value == SourceKitRepresentabl
 }
 
 private extension String {
-    func lastComponentAfter(_ charachter: String) -> String {
-        return components(separatedBy: charachter).last ?? ""
+    func lastComponentAfter(_ character: String) -> String {
+        return components(separatedBy: character).last ?? ""
     }
 }
 
 private struct ModifierDescription: Equatable {
     let keyword: String
     let group: SwiftDeclarationAttributeKind.ModifierGroup
+    let offset: Int
+    let length: Int
 }

Tests/LinuxMain.swift

@@ -724,6 +724,9 @@ extension ModifierOrderTests {
         ("testRightOrderedModifierGroups", testRightOrderedModifierGroups),
         ("testAtPrefixedGroup", testAtPrefixedGroup),
         ("testNonSpecifiedModifiersDontInterfere", testNonSpecifiedModifiersDontInterfere),
+        ("testCorrectionsAreAppliedCorrectly", testCorrectionsAreAppliedCorrectly),
+        ("testCorrectionsAreNotAppliedToIrrelevantModifier", testCorrectionsAreNotAppliedToIrrelevantModifier),
+        ("testTypeMethodClassCorrection", testTypeMethodClassCorrection),
         ("testViolationMessage", testViolationMessage)
     ]
 }