Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make SortedImportsRule auto-correctable #1922

Merged
merged 7 commits into from
Oct 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,17 @@
* 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)
[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

* Correct equality tests for `Configuration` values. They previously didn't
Expand Down
64 changes: 63 additions & 1 deletion Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -10544,6 +10544,37 @@ import labc
import Ldef
```

```swift
import BBB
// comment
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
```

</details>
<details>
<summary>Triggering Examples</summary>
Expand All @@ -10555,6 +10586,37 @@ import ↓BBB
import CCC
```

```swift
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
```

</details>


Expand Down
192 changes: 173 additions & 19 deletions Source/SwiftLintFramework/Rules/SortedImportsRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,52 @@
import Foundation
import SourceKittenFramework

public struct SortedImportsRule: ConfigurationProviderRule, OptInRule {
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 importModule() -> Substring {
return content[importModuleRange()]
}

fileprivate func importModuleRange() -> Range<String.Index> {
let rangeOfImport = content.range(of: "import")
precondition(rangeOfImport != nil)
let moduleStart = content.rangeOfCharacter(from: CharacterSet.whitespaces.inverted, options: [],
range: rangeOfImport!.upperBound..<content.endIndex)
return moduleStart!.lowerBound..<content.endIndex
}

// Case insensitive comparison of contents of the line
// after the word `import`
fileprivate static func <= (lhs: Line, rhs: Line) -> Bool {
return lhs.importModule().lowercased() <= rhs.importModule().lowercased()
}
}

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 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
}
}
}

public struct SortedImportsRule: CorrectableRule, ConfigurationProviderRule, OptInRule {
public var configuration = SeverityConfiguration(.warning)

public init() {}
Expand All @@ -22,36 +67,145 @@ public struct SortedImportsRule: ConfigurationProviderRule, OptInRule {
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",
"@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 AAA\nimport ZZZ\nimport ↓BBB\nimport CCC",
"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",
"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
"""
]
)

public func validate(file: File) -> [StyleViolation] {
let importRanges = file.match(pattern: "import\\s+\\w+", with: [.keyword, .identifier])
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,
location: location)
}
}

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] = importRanges.flatMap { range in
guard let line = contents.lineAndCharacter(forCharacterOffset: range.location)?.line
else { return nil }
return lines[line - 1]
}

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)
return importLines.grouped()
}

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 {
return violatingOffsets
}
let distance = current.content.distance(from: current.content.startIndex,
to: current.importModuleRange().lowerBound)
return violatingOffsets + [current.range.location + distance]
}
}
}

public func correct(file: File) -> [Correction] {
let groups = importGroups(in: file, filterEnabled: true)

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)
}

let modulePairs = zip(modulesAndOffsets, modulesAndOffsets.dropFirst())
let violatingOffsets = modulePairs.flatMap { previous, current in
return current < previous ? current.1 : nil
guard !corrections.isEmpty else {
return []
}

return violatingOffsets.map {
StyleViolation(ruleDescription: type(of: self).description,
severity: configuration.severity,
location: Location(file: file, characterOffset: $0))
let correctedContents = NSMutableString(string: file.contents)
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())

return corrections
}
}
1 change: 0 additions & 1 deletion Source/swiftlint/Commands/RulesCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
// Copyright © 2015 Realm. All rights reserved.
//

// swiftlint:disable sorted_imports
import Commandant
#if os(Linux)
import Glibc
Expand Down