PR feedback

realm · Jan 10, 2017 · 94f48f4 · 94f48f4
1 parent e1e2369
commit 94f48f4
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 31 deletions.
diff --git a/Source/SwiftLintFramework/Models/Configuration.swift b/Source/SwiftLintFramework/Models/Configuration.swift
@@ -26,6 +26,7 @@ extension String {
 private let fileManager = FileManager.default
 
 private enum ConfigurationKey: String {
+    case cachePath = "cache_path"
     case disabledRules = "disabled_rules"
     case enabledRules = "enabled_rules" // deprecated in favor of optInRules
     case excluded = "excluded"
@@ -35,7 +36,6 @@ private enum ConfigurationKey: String {
     case swiftlintVersion = "swiftlint_version"
     case useNestedConfigs = "use_nested_configs" // deprecated
     case warningThreshold = "warning_threshold"
-    case cachePath = "cache_path"
     case whitelistRules = "whitelist_rules"
 }
 

diff --git a/Source/SwiftLintFramework/Models/Linter.swift b/Source/SwiftLintFramework/Models/Linter.swift
@@ -66,7 +66,7 @@ public struct Linter {
 
     private func getStyleViolations(benchmark: Bool = false) -> ([StyleViolation], [(id: String, time: Double)]) {
 
-        if let cached = cachedStyleViolations(benchmark) {
+        if let cached = cachedStyleViolations(benchmark: benchmark) {
             return cached
         }
 
@@ -84,9 +84,9 @@ public struct Linter {
             deprecatedToValidIdentifier[key] = value
         }
 
-        if let cache = cache, let file = file.path {
-            let hash = self.file.contents.hash
-            cache.cacheFile(file, violations: violations, hash: hash)
+        if let cache = cache, let path = file.path {
+            let hash = file.contents.hash
+            cache.cache(violations: violations, forFile: path, fileHash: hash)
         }
 
         for (deprecatedIdentifier, identifier) in deprecatedToValidIdentifier {
@@ -97,12 +97,12 @@ public struct Linter {
         return (violations, ruleTimes)
     }
 
-    private func cachedStyleViolations(_ benchmark: Bool = false) -> ([StyleViolation], [(id: String, time: Double)])? {
+    private func cachedStyleViolations(benchmark: Bool = false) -> ([StyleViolation], [(id: String, time: Double)])? {
         let start: Date! = benchmark ? Date() : nil
         guard let cache = cache,
             let file = file.path,
             case let hash = self.file.contents.hash,
-            let cachedViolations = cache.violations(for: file, hash: hash) else {
+            let cachedViolations = cache.violations(forFile: file, hash: hash) else {
                 return nil
         }
 

diff --git a/Source/SwiftLintFramework/Models/LinterCache.swift b/Source/SwiftLintFramework/Models/LinterCache.swift
@@ -20,10 +20,11 @@ public final class LinterCache {
     private let lock = NSLock()
 
     public init(currentVersion: Version = .current, configurationHash: Int? = nil) {
-        cache = [String: Any]()
-        cache["version"] = currentVersion.value
+        cache = [
+            "version": currentVersion.value,
+            "files": [:]
+        ]
         cache["configuration_hash"] = configurationHash
-        cache["files"] = [:]
     }
 
     public init(cache: Any, currentVersion: Version = .current, configurationHash: Int? = nil) throws {
@@ -50,25 +51,18 @@ public final class LinterCache {
                       configurationHash: configurationHash)
     }
 
-    public func cacheFile(_ file: String, violations: [StyleViolation], hash: Int) {
-        var entry = [String: Any]()
-        var fileViolations = entry["violations"] as? [[String: Any]] ?? []
-
-        for violation in violations {
-            fileViolations.append(dictionaryForViolation(violation))
-        }
-
-        entry["violations"] = fileViolations
-        entry["hash"] = hash
-
+    public func cache(violations: [StyleViolation], forFile file: String, fileHash: Int) {
         lock.lock()
         var filesCache = (cache["files"] as? [String: Any]) ?? [:]
-        filesCache[file] = entry
+        filesCache[file] = [
+            "violations": violations.map(dictionary(for:)),
+            "hash": fileHash
+        ]
         cache["files"] = filesCache
         lock.unlock()
     }
 
-    public func violations(for file: String, hash: Int) -> [StyleViolation]? {
+    public func violations(forFile file: String, hash: Int) -> [StyleViolation]? {
         lock.lock()
 
         guard let filesCache = cache["files"] as? [String: Any],
@@ -81,7 +75,7 @@ public final class LinterCache {
         }
 
         lock.unlock()
-        return violations.flatMap { StyleViolation.fromCache($0, file: file) }
+        return violations.flatMap { StyleViolation.from(cache: $0, file: file) }
     }
 
     public func save(to url: URL) throws {
@@ -91,7 +85,7 @@ public final class LinterCache {
         try json.write(to: url, atomically: true, encoding: .utf8)
     }
 
-    private func dictionaryForViolation(_ violation: StyleViolation) -> [String: Any] {
+    private func dictionary(for violation: StyleViolation) -> [String: Any] {
         return [
             "line": violation.location.line ?? NSNull() as Any,
             "character": violation.location.character ?? NSNull() as Any,
@@ -104,7 +98,7 @@ public final class LinterCache {
 }
 
 extension StyleViolation {
-    fileprivate static func fromCache(_ cache: [String: Any], file: String) -> StyleViolation? {
+    fileprivate static func from(cache: [String: Any], file: String) -> StyleViolation? {
         guard let severity = (cache["severity"] as? String).flatMap({ ViolationSeverity(rawValue: $0) }),
             let name = cache["type"] as? String,
             let ruleId = cache["rule_id"] as? String,

diff --git a/Tests/SwiftLintFrameworkTests/LinterCacheTests.swift b/Tests/SwiftLintFrameworkTests/LinterCacheTests.swift
@@ -63,8 +63,8 @@ class LinterCacheTests: XCTestCase {
                            reason: "Something is wrong.")
         ]
 
-        cache.cacheFile(file, violations: violations, hash: 1)
-        let cachedViolations = cache.violations(for: file, hash: 1)
+        cache.cache(violations: violations, forFile: file, fileHash: 1)
+        let cachedViolations = cache.violations(forFile: file, hash: 1)
 
         XCTAssertNotNil(cachedViolations)
         XCTAssertEqual(cachedViolations!, violations)
@@ -73,16 +73,16 @@ class LinterCacheTests: XCTestCase {
     func testParsesViolationsWithModifiedHash() {
         let cache = LinterCache(currentVersion: Version(value: "0.2.0"))
         let file = "foo.swift"
-        cache.cacheFile(file, violations: [], hash: 1)
-        let cachedViolations = cache.violations(for: file, hash: 2)
+        cache.cache(violations: [], forFile: file, fileHash: 1)
+        let cachedViolations = cache.violations(forFile: file, hash: 2)
 
         XCTAssertNil(cachedViolations)
     }
 
     func testParsesViolationsWithEmptyViolations() {
         let cache = LinterCache(currentVersion: Version(value: "0.2.0"))
         let file = "foo.swift"
-        let cachedViolations = cache.violations(for: file, hash: 2)
+        let cachedViolations = cache.violations(forFile: file, hash: 2)
 
         XCTAssertNil(cachedViolations)
     }