From 708e73b0c8ee4f39ae40bbc91cd378f7a9a13c56 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Sat, 16 Sep 2017 22:42:13 +0100 Subject: [PATCH 1/7] Make sorted_imports corractable --- Rules.md | 2 +- .../Rules/SortedImportsRule.swift | 124 ++++++++++++++++-- 2 files changed, 117 insertions(+), 9 deletions(-) diff --git a/Rules.md b/Rules.md index 88c17fbc29..22643a76d5 100644 --- a/Rules.md +++ b/Rules.md @@ -10518,7 +10518,7 @@ class TotoTests { } Identifier | Enabled by default | Supports autocorrection | Kind --- | --- | --- | --- -`sorted_imports` | Disabled | No | style +`sorted_imports` | Disabled | Yes | style Imports should be sorted. diff --git a/Source/SwiftLintFramework/Rules/SortedImportsRule.swift b/Source/SwiftLintFramework/Rules/SortedImportsRule.swift index b442d7a53f..e6f66f68ec 100644 --- a/Source/SwiftLintFramework/Rules/SortedImportsRule.swift +++ b/Source/SwiftLintFramework/Rules/SortedImportsRule.swift @@ -9,7 +9,46 @@ import Foundation import SourceKittenFramework -public struct SortedImportsRule: ConfigurationProviderRule, OptInRule { +private extension Line { + var contentRange: NSRange { + return NSRange(location: range.location, length: content.characters.count) + } + + // `Line` in this rule always contains word import + // This method returns contents of line that are after import + private func afterImport() -> String { + guard let range = content.range(of: "import ") else { return "" } + return String(content.characters.suffix(from: range.upperBound)) + } + + // Case insensitive comparission of contents of the line + // after the word `import` + static func <= (lhs: Line, rhs: Line) -> Bool { + return lhs.afterImport().lowercased() <= rhs.afterImport().lowercased() + } +} + +private extension Array where Element == Line { + // Groups lines, so that lines that are one after the other + // will end up in same group. + func grouped() -> [[Line]] { + return self.reduce([[Line]]()) { result, line in + if let last = result.last?.last { + var copy = result + if last.index == line.index - 1 { + copy[copy.count - 1].append(line) + } else { + copy.append([line]) + } + return copy + } else { + return [[line]] + } + } + } +} + +public struct SortedImportsRule: CorrectableRule, ConfigurationProviderRule, OptInRule { public var configuration = SeverityConfiguration(.warning) public init() {} @@ -26,13 +65,31 @@ public struct SortedImportsRule: ConfigurationProviderRule, OptInRule { ], triggeringExamples: [ "import AAA\nimport ZZZ\nimport ↓BBB\nimport CCC" + ], + corrections: [ + "import AAA\nimport ZZZ\nimport ↓BBB\nimport CCC": "import AAA\nimport BBB\nimport CCC\nimport ZZZ", + "import BBB // comment\nimport ↓AAA": "import AAA\nimport BBB // comment", + "import BBB\n// comment\nimport CCC\nimport ↓AAA": "import BBB\n// comment\nimport AAA\nimport CCC", + "@testable import CCC\nimport AAA": "import AAA\n@testable import CCC" ] ) public func validate(file: File) -> [StyleViolation] { - let importRanges = file.match(pattern: "import\\s+\\w+", with: [.keyword, .identifier]) - let contents = file.contents.bridge() + let violatingOffsets = findViolationOffsets(file: file) + + return violatingOffsets.map { + StyleViolation(ruleDescription: type(of: self).description, + severity: configuration.severity, + location: Location(file: file, characterOffset: $0)) + } + } + private static let importPattern = "import\\s+\\w+" + + private func findViolationOffsets(file: File) -> [Int] { + let importRanges = file.match(pattern: type(of: self).importPattern, + with: [.keyword, .identifier]) + let contents = file.contents.bridge() let importLength = 6 let modulesAndOffsets: [(String, Int)] = importRanges.map { range in let moduleRange = NSRange(location: range.location + importLength, @@ -44,14 +101,65 @@ public struct SortedImportsRule: ConfigurationProviderRule, OptInRule { } let modulePairs = zip(modulesAndOffsets, modulesAndOffsets.dropFirst()) - let violatingOffsets = modulePairs.flatMap { previous, current in + return modulePairs.flatMap { previous, current in return current < previous ? current.1 : nil } + } - return violatingOffsets.map { - StyleViolation(ruleDescription: type(of: self).description, - severity: configuration.severity, - location: Location(file: file, characterOffset: $0)) + private func violatingOffsets(in groups: [[Line]]) -> [Int] { + var violatingOffsets = [Int]() + groups.forEach { group in + let pairs = zip(group, group.dropFirst()) + pairs.forEach { previous, current in + let isOrderedCorrectly = previous <= current + if !isOrderedCorrectly { + violatingOffsets.append(current.range.location + 7) + } + } + } + return violatingOffsets + } + + public func correct(file: File) -> [Correction] { + let importRanges = file.match(pattern: type(of: self).importPattern, with: [.keyword, .identifier]) + let enabledImportRanges = file.ruleEnabled(violatingRanges: importRanges, for: self) + guard enabledImportRanges.count > 1 else { + return [] } + + let contents = file.contents.bridge() + let lines = contents.lines() + let importLines: [Line] = enabledImportRanges.flatMap { range in + guard let line = contents.lineAndCharacter(forCharacterOffset: range.location)?.line + else { return nil } + return lines[line - 1] + } + + var groups = importLines.grouped() + let description = type(of: self).description + let corrections = self.violatingOffsets(in: groups).map { index -> Correction in + let location = Location(file: file, characterOffset: index) + return Correction(ruleDescription: description, location: location) + } + + groups.enumerated().forEach { index, group in + groups[index] = group.sorted { previous, current in + previous <= current + } + } + + let correctedContents = NSMutableString(string: file.contents) + groups.forEach { group in + if let first = group.first?.contentRange { + let result = group.map { $0.content }.joined(separator: "\n") + let union = group.dropFirst().reduce(first, { result, line in + return NSUnionRange(result, line.contentRange) + }) + correctedContents.replaceCharacters(in: union, with: result) + } + } + file.write(correctedContents.bridge()) + + return corrections } } From cf7d1111669c0f1b531a0e7e610274f65995c1c6 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Sun, 17 Sep 2017 10:43:46 +0100 Subject: [PATCH 2/7] Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0571c56323..650bfb8fb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,10 @@ * Add `catch` to the statements checked by the `control_statement` rule. [JP Simard](https://github.com/jpsim) +* Make `sorted_imports` correctable. + [Samuel Susla](https://github.com/sammy-sc) + [#1822](https://github.com/realm/SwiftLint/issues/1822) + ##### Bug Fixes * Correct equality tests for `Configuration` values. They previously didn't From 14036dca389b287edd39a58700d45fc9c6bcdc3a Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Thu, 21 Sep 2017 18:13:46 +0100 Subject: [PATCH 3/7] Make `sorted_imports` rule group imports into groups --- Rules.md | 14 +++++ .../Rules/SortedImportsRule.swift | 61 +++++++------------ 2 files changed, 35 insertions(+), 40 deletions(-) diff --git a/Rules.md b/Rules.md index 22643a76d5..070230f962 100644 --- a/Rules.md +++ b/Rules.md @@ -10544,6 +10544,13 @@ import labc import Ldef ``` +```swift +import BBB +// comment +import AAA +import CCC +``` +
Triggering Examples @@ -10555,6 +10562,13 @@ import ↓BBB import CCC ``` +```swift +import BBB +// comment +import CCC +import ↓AAA +``` +
diff --git a/Source/SwiftLintFramework/Rules/SortedImportsRule.swift b/Source/SwiftLintFramework/Rules/SortedImportsRule.swift index e6f66f68ec..b47c1af41c 100644 --- a/Source/SwiftLintFramework/Rules/SortedImportsRule.swift +++ b/Source/SwiftLintFramework/Rules/SortedImportsRule.swift @@ -61,10 +61,12 @@ public struct SortedImportsRule: CorrectableRule, ConfigurationProviderRule, Opt nonTriggeringExamples: [ "import AAA\nimport BBB\nimport CCC\nimport DDD", "import Alamofire\nimport API", - "import labc\nimport Ldef" + "import labc\nimport Ldef", + "import BBB\n// comment\nimport AAA\nimport CCC" ], triggeringExamples: [ - "import AAA\nimport ZZZ\nimport ↓BBB\nimport CCC" + "import AAA\nimport ZZZ\nimport ↓BBB\nimport CCC", + "import BBB\n// comment\nimport CCC\nimport ↓AAA" ], corrections: [ "import AAA\nimport ZZZ\nimport ↓BBB\nimport CCC": "import AAA\nimport BBB\nimport CCC\nimport ZZZ", @@ -75,35 +77,28 @@ public struct SortedImportsRule: CorrectableRule, ConfigurationProviderRule, Opt ) public func validate(file: File) -> [StyleViolation] { - let violatingOffsets = findViolationOffsets(file: file) - - return violatingOffsets.map { - StyleViolation(ruleDescription: type(of: self).description, - severity: configuration.severity, - location: Location(file: file, characterOffset: $0)) + let groups = self.importGroups(in: file) + return self.violatingOffsets(in: groups).map { index -> StyleViolation in + let location = Location(file: file, characterOffset: index) + return StyleViolation(ruleDescription: type(of: self).description, + severity: configuration.severity, + location: location) } } - private static let importPattern = "import\\s+\\w+" + private func importGroups(in file: File) -> [[Line]] { + let importRanges = file.match(pattern: "import\\s+\\w+", with: [.keyword, .identifier]) + let enabledImportRanges = file.ruleEnabled(violatingRanges: importRanges, for: self) - private func findViolationOffsets(file: File) -> [Int] { - let importRanges = file.match(pattern: type(of: self).importPattern, - with: [.keyword, .identifier]) let contents = file.contents.bridge() - let importLength = 6 - let modulesAndOffsets: [(String, Int)] = importRanges.map { range in - let moduleRange = NSRange(location: range.location + importLength, - length: range.length - importLength) - let moduleName = contents.substring(with: moduleRange) - .trimmingCharacters(in: .whitespacesAndNewlines).lowercased() - let offset = NSMaxRange(range) - moduleName.bridge().length - return (moduleName, offset) + let lines = contents.lines() + let importLines: [Line] = enabledImportRanges.flatMap { range in + guard let line = contents.lineAndCharacter(forCharacterOffset: range.location)?.line + else { return nil } + return lines[line - 1] } - let modulePairs = zip(modulesAndOffsets, modulesAndOffsets.dropFirst()) - return modulePairs.flatMap { previous, current in - return current < previous ? current.1 : nil - } + return importLines.grouped() } private func violatingOffsets(in groups: [[Line]]) -> [Int] { @@ -121,25 +116,11 @@ public struct SortedImportsRule: CorrectableRule, ConfigurationProviderRule, Opt } public func correct(file: File) -> [Correction] { - let importRanges = file.match(pattern: type(of: self).importPattern, with: [.keyword, .identifier]) - let enabledImportRanges = file.ruleEnabled(violatingRanges: importRanges, for: self) - guard enabledImportRanges.count > 1 else { - return [] - } - - let contents = file.contents.bridge() - let lines = contents.lines() - let importLines: [Line] = enabledImportRanges.flatMap { range in - guard let line = contents.lineAndCharacter(forCharacterOffset: range.location)?.line - else { return nil } - return lines[line - 1] - } + var groups = self.importGroups(in: file) - var groups = importLines.grouped() - let description = type(of: self).description let corrections = self.violatingOffsets(in: groups).map { index -> Correction in let location = Location(file: file, characterOffset: index) - return Correction(ruleDescription: description, location: location) + return Correction(ruleDescription: type(of: self).description, location: location) } groups.enumerated().forEach { index, group in From ac53589f790cafaf8b350eb34461274fb54701eb Mon Sep 17 00:00:00 2001 From: JP Simard Date: Mon, 23 Oct 2017 11:56:28 -0700 Subject: [PATCH 4/7] Fix issues with SortedImportsRule * Fix superfluous disable command producing false positives * Fix import formatting assumptions: * Exactly once space after `import` * No support for `@testable import` * Reduce indentation * Omit types when they can be inferred * Omit explicit references to `self.` * Fix typos in comments * Make constrained extensions more generic * Use explicit ACLs for all declarations --- .../Rules/SortedImportsRule.swift | 104 ++++++++++-------- 1 file changed, 56 insertions(+), 48 deletions(-) diff --git a/Source/SwiftLintFramework/Rules/SortedImportsRule.swift b/Source/SwiftLintFramework/Rules/SortedImportsRule.swift index b47c1af41c..0f1414229d 100644 --- a/Source/SwiftLintFramework/Rules/SortedImportsRule.swift +++ b/Source/SwiftLintFramework/Rules/SortedImportsRule.swift @@ -9,41 +9,47 @@ import Foundation import SourceKittenFramework -private extension Line { - var contentRange: NSRange { - return NSRange(location: range.location, length: content.characters.count) +extension Line { + fileprivate var contentRange: NSRange { + return NSRange(location: range.location, length: content.bridge().length) } // `Line` in this rule always contains word import // This method returns contents of line that are after import - private func afterImport() -> String { - guard let range = content.range(of: "import ") else { return "" } - return String(content.characters.suffix(from: range.upperBound)) + private func importModule() -> Substring { + return content[importModuleRange()] } - // Case insensitive comparission of contents of the line + fileprivate func importModuleRange() -> Range { + let rangeOfImport = content.range(of: "import") + precondition(rangeOfImport != nil) + let moduleStart = content.rangeOfCharacter(from: CharacterSet.whitespaces.inverted, options: [], + range: rangeOfImport!.upperBound.. Bool { - return lhs.afterImport().lowercased() <= rhs.afterImport().lowercased() + fileprivate static func <= (lhs: Line, rhs: Line) -> Bool { + return lhs.importModule().lowercased() <= rhs.importModule().lowercased() } } -private extension Array where Element == Line { +private extension Sequence where Element == Line { // Groups lines, so that lines that are one after the other // will end up in same group. func grouped() -> [[Line]] { - return self.reduce([[Line]]()) { result, line in - if let last = result.last?.last { - var copy = result - if last.index == line.index - 1 { - copy[copy.count - 1].append(line) - } else { - copy.append([line]) - } - return copy - } else { + return reduce([[]]) { result, line in + guard let last = result.last?.last else { return [[line]] } + var copy = result + if last.index == line.index - 1 { + copy[copy.count - 1].append(line) + } else { + copy.append([line]) + } + return copy } } } @@ -77,8 +83,8 @@ public struct SortedImportsRule: CorrectableRule, ConfigurationProviderRule, Opt ) public func validate(file: File) -> [StyleViolation] { - let groups = self.importGroups(in: file) - return self.violatingOffsets(in: groups).map { index -> StyleViolation in + let groups = importGroups(in: file, filterEnabled: false) + return violatingOffsets(inGroups: groups).map { index -> StyleViolation in let location = Location(file: file, characterOffset: index) return StyleViolation(ruleDescription: type(of: self).description, severity: configuration.severity, @@ -86,13 +92,15 @@ public struct SortedImportsRule: CorrectableRule, ConfigurationProviderRule, Opt } } - private func importGroups(in file: File) -> [[Line]] { - let importRanges = file.match(pattern: "import\\s+\\w+", with: [.keyword, .identifier]) - let enabledImportRanges = file.ruleEnabled(violatingRanges: importRanges, for: self) + private func importGroups(in file: File, filterEnabled: Bool) -> [[Line]] { + var importRanges = file.match(pattern: "import\\s+\\w+", with: [.keyword, .identifier]) + if filterEnabled { + importRanges = file.ruleEnabled(violatingRanges: importRanges, for: self) + } let contents = file.contents.bridge() let lines = contents.lines() - let importLines: [Line] = enabledImportRanges.flatMap { range in + let importLines: [Line] = importRanges.flatMap { range in guard let line = contents.lineAndCharacter(forCharacterOffset: range.location)?.line else { return nil } return lines[line - 1] @@ -101,43 +109,43 @@ public struct SortedImportsRule: CorrectableRule, ConfigurationProviderRule, Opt return importLines.grouped() } - private func violatingOffsets(in groups: [[Line]]) -> [Int] { - var violatingOffsets = [Int]() - groups.forEach { group in - let pairs = zip(group, group.dropFirst()) - pairs.forEach { previous, current in + private func violatingOffsets(inGroups groups: [[Line]]) -> [Int] { + return groups.flatMap { group in + return zip(group, group.dropFirst()).reduce([]) { violatingOffsets, groupPair in + let (previous, current) = groupPair let isOrderedCorrectly = previous <= current - if !isOrderedCorrectly { - violatingOffsets.append(current.range.location + 7) + if isOrderedCorrectly { + return violatingOffsets } + let distance = current.content.distance(from: current.content.startIndex, + to: current.importModuleRange().lowerBound) + return violatingOffsets + [current.range.location + distance] } } - return violatingOffsets } public func correct(file: File) -> [Correction] { - var groups = self.importGroups(in: file) + let groups = importGroups(in: file, filterEnabled: true) - let corrections = self.violatingOffsets(in: groups).map { index -> Correction in - let location = Location(file: file, characterOffset: index) + let corrections = violatingOffsets(inGroups: groups).map { characterOffset -> Correction in + let location = Location(file: file, characterOffset: characterOffset) return Correction(ruleDescription: type(of: self).description, location: location) } - groups.enumerated().forEach { index, group in - groups[index] = group.sorted { previous, current in - previous <= current - } + guard !corrections.isEmpty else { + return [] } let correctedContents = NSMutableString(string: file.contents) - groups.forEach { group in - if let first = group.first?.contentRange { - let result = group.map { $0.content }.joined(separator: "\n") - let union = group.dropFirst().reduce(first, { result, line in - return NSUnionRange(result, line.contentRange) - }) - correctedContents.replaceCharacters(in: union, with: result) + for group in groups.map({ $0.sorted(by: <=) }) { + guard let first = group.first?.contentRange else { + continue + } + let result = group.map { $0.content }.joined(separator: "\n") + let union = group.dropFirst().reduce(first) { result, line in + return NSUnionRange(result, line.contentRange) } + correctedContents.replaceCharacters(in: union, with: result) } file.write(correctedContents.bridge()) From 170606d3d98243d3bdf36a8a319608be6a5e1ff1 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Mon, 23 Oct 2017 14:35:53 -0700 Subject: [PATCH 5/7] Add more examples to SortedImportsRule --- Rules.md | 50 ++++++++++++++- .../Rules/SortedImportsRule.swift | 63 ++++++++++++++++++- 2 files changed, 109 insertions(+), 4 deletions(-) diff --git a/Rules.md b/Rules.md index 070230f962..5a6df50c6f 100644 --- a/Rules.md +++ b/Rules.md @@ -10551,6 +10551,30 @@ import AAA import CCC ``` +```swift +@testable import AAA +import CCC +``` + +```swift +import AAA +@testable import CCC +``` + +```swift +import EEE.A +import FFF.B +#if os(Linux) +import DDD.A +import EEE.B +#else +import CCC +import DDD.B +#endif +import AAA +import BBB +``` +
Triggering Examples @@ -10563,12 +10587,36 @@ import CCC ``` ```swift -import BBB +import DDD // comment import CCC import ↓AAA ``` +```swift +@testable import CCC +import ↓AAA +``` + +```swift +import CCC +@testable import ↓AAA +``` + +```swift +import FFF.B +import ↓EEE.A +#if os(Linux) +import DDD.A +import EEE.B +#else +import DDD.B +import ↓CCC +#endif +import AAA +import BBB +``` +
diff --git a/Source/SwiftLintFramework/Rules/SortedImportsRule.swift b/Source/SwiftLintFramework/Rules/SortedImportsRule.swift index 0f1414229d..9d16255bdf 100644 --- a/Source/SwiftLintFramework/Rules/SortedImportsRule.swift +++ b/Source/SwiftLintFramework/Rules/SortedImportsRule.swift @@ -68,17 +68,74 @@ public struct SortedImportsRule: CorrectableRule, ConfigurationProviderRule, Opt "import AAA\nimport BBB\nimport CCC\nimport DDD", "import Alamofire\nimport API", "import labc\nimport Ldef", - "import BBB\n// comment\nimport AAA\nimport CCC" + "import BBB\n// comment\nimport AAA\nimport CCC", + "@testable import AAA\nimport CCC", + "import AAA\n@testable import CCC", + """ + import EEE.A + import FFF.B + #if os(Linux) + import DDD.A + import EEE.B + #else + import CCC + import DDD.B + #endif + import AAA + import BBB + """ ], triggeringExamples: [ "import AAA\nimport ZZZ\nimport ↓BBB\nimport CCC", - "import BBB\n// comment\nimport CCC\nimport ↓AAA" + "import DDD\n// comment\nimport CCC\nimport ↓AAA", + "@testable import CCC\nimport ↓AAA", + "import CCC\n@testable import ↓AAA", + """ + import FFF.B + import ↓EEE.A + #if os(Linux) + import DDD.A + import EEE.B + #else + import DDD.B + import ↓CCC + #endif + import AAA + import BBB + """ ], corrections: [ "import AAA\nimport ZZZ\nimport ↓BBB\nimport CCC": "import AAA\nimport BBB\nimport CCC\nimport ZZZ", "import BBB // comment\nimport ↓AAA": "import AAA\nimport BBB // comment", "import BBB\n// comment\nimport CCC\nimport ↓AAA": "import BBB\n// comment\nimport AAA\nimport CCC", - "@testable import CCC\nimport AAA": "import AAA\n@testable import CCC" + "@testable import CCC\nimport ↓AAA": "import AAA\n@testable import CCC", + "import CCC\n@testable import ↓AAA": "@testable import AAA\nimport CCC", + """ + import FFF.B + import ↓EEE.A + #if os(Linux) + import DDD.A + import EEE.B + #else + import DDD.B + import ↓CCC + #endif + import AAA + import BBB + """: + """ + import EEE.A + import FFF.B + #if os(Linux) + import DDD.A + import EEE.B + #else + import CCC + import DDD.B + #endif + import AAA + import BBB + """ ] ) From e3404cbf56234bd2c3b1b39f29a24bad2592e785 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Mon, 23 Oct 2017 15:12:14 -0700 Subject: [PATCH 6/7] Fix superfluous disable command violation --- Source/swiftlint/Commands/RulesCommand.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Source/swiftlint/Commands/RulesCommand.swift b/Source/swiftlint/Commands/RulesCommand.swift index 9577328819..159bf21287 100644 --- a/Source/swiftlint/Commands/RulesCommand.swift +++ b/Source/swiftlint/Commands/RulesCommand.swift @@ -6,7 +6,6 @@ // Copyright © 2015 Realm. All rights reserved. // -// swiftlint:disable sorted_imports import Commandant #if os(Linux) import Glibc From de67d72d4903bdb9a3102052e73effc655fc787a Mon Sep 17 00:00:00 2001 From: JP Simard Date: Mon, 23 Oct 2017 15:19:37 -0700 Subject: [PATCH 7/7] Add another sorted_imports changelog entry --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 650bfb8fb7..589bfc80e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,13 @@ * Make `sorted_imports` correctable. [Samuel Susla](https://github.com/sammy-sc) + [JP Simard](https://github.com/jpsim) + [#1822](https://github.com/realm/SwiftLint/issues/1822) + +* Make `sorted_imports` only validate within import "groups" of imports on + directly adjacent lines. + [Samuel Susla](https://github.com/sammy-sc) + [JP Simard](https://github.com/jpsim) [#1822](https://github.com/realm/SwiftLint/issues/1822) ##### Bug Fixes