Skip to content

Commit

Permalink
Merge pull request #1073 from marcelofabri/cache
Browse files Browse the repository at this point in the history
Cache implementation
  • Loading branch information
jpsim authored Jan 11, 2017
2 parents 92c5a11 + 5702ef3 commit 4f225a4
Show file tree
Hide file tree
Showing 11 changed files with 407 additions and 29 deletions.
22 changes: 13 additions & 9 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@

##### Enhancements

* Speed up linting by caching linter results across invocations.
[Marcelo Fabri](https://github.com/marcelofabri)
[#868](https://github.com/realm/SwiftLint/issues/868)

* Speed up linting by processing multiple files and rules concurrently.
[JP Simard](https://github.com/jpsim)
[#1077](https://github.com/realm/SwiftLint/issues/1077)

* Make many operations in SwiftLintFramework safe to call in multithreaded
scenarios, including accessing `Linter.styleViolations`.
[JP Simard](https://github.com/jpsim)
[#1077](https://github.com/realm/SwiftLint/issues/1077)

* Permit unsigned and explicitly-sized integer types in `valid_ibinspectable`
[Daniel Duan](https://github.com/dduan)

Expand Down Expand Up @@ -68,15 +81,6 @@
[Marcelo Fabri](https://github.com/marcelofabri)
[#1109](https://github.com/realm/SwiftLint/issues/1109)

* Make many operations in SwiftLintFramework safe to call in multithreaded
scenarios, including accessing `Linter.styleViolations`.
[JP Simard](https://github.com/jpsim)
[#1077](https://github.com/realm/SwiftLint/issues/1077)

* Speed up linting by processing multiple files and rules concurrently.
[JP Simard](https://github.com/jpsim)
[#1077](https://github.com/realm/SwiftLint/issues/1077)

* Add `compiler_protocol_init` rule that flags usage of initializers
declared in protocols used by the compiler such as `ExpressibleByArrayLiteral`
that shouldn't be called directly. Instead, you should use a literal anywhere
Expand Down
20 changes: 18 additions & 2 deletions Dangerfile
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ if has_app_changes
durations = []
start = Time.now
command = '../../.build/release/swiftlint'
if `#{command} help lint`.include? '--no-cache'
command += ' lint --no-cache'
end
File.open("../#{branch}_reports/#{repo_name}.txt", 'w') do |file|
Open3.popen3(command) do |_, stdout, _, _|
file << stdout.read.chomp
Expand Down Expand Up @@ -90,10 +93,23 @@ if has_app_changes
# Generate branch reports
generate_reports(true, 'branch')
# Build master
`git checkout master`
`git pull`
`git fetch`
`git checkout origin/master`
puts 'Building master'
`swift build -c release`
unless $?.success?
# Couldn't build, start fresh
FileUtils.rm_rf %w(Packages .build)
return_value = nil
Open3.popen3('swift build -c release') do |_, stdout, _, wait_thr|
puts stdout.read.chomp
return_value = wait_thr.value
end
unless return_value.success?
fail 'Could not build master'
return
end
end
# Generate master reports
generate_reports(false, 'master')
# Diff and report changes to Danger
Expand Down
14 changes: 11 additions & 3 deletions Source/SwiftLintFramework/Models/Configuration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -47,6 +48,8 @@ public struct Configuration: Equatable {
public let rules: [Rule]
public var rootPath: String? // the root path to search for nested configurations
public var configurationPath: String? // if successfully loaded from a path
public var hash: Int?
public let cachePath: String?

public init?(disabledRules: [String] = [],
optInRules: [String] = [],
Expand All @@ -57,7 +60,8 @@ public struct Configuration: Equatable {
reporter: String = XcodeReporter.identifier,
ruleList: RuleList = masterRuleList,
configuredRules: [Rule]? = nil,
swiftlintVersion: String? = nil) {
swiftlintVersion: String? = nil,
cachePath: String? = nil) {

if let pinnedVersion = swiftlintVersion, pinnedVersion != Version.current.value {
queuedPrintError("Currently running SwiftLint \(Version.current.value) but " +
Expand All @@ -67,6 +71,7 @@ public struct Configuration: Equatable {
self.included = included
self.excluded = excluded
self.reporter = reporter
self.cachePath = cachePath

let configuredRules = configuredRules
?? (try? ruleList.configuredRules(with: [:]))
Expand Down Expand Up @@ -156,7 +161,8 @@ public struct Configuration: Equatable {
XcodeReporter.identifier,
ruleList: ruleList,
configuredRules: configuredRules,
swiftlintVersion: dict[ConfigurationKey.swiftlintVersion.rawValue] as? String)
swiftlintVersion: dict[ConfigurationKey.swiftlintVersion.rawValue] as? String,
cachePath: dict[ConfigurationKey.cachePath.rawValue] as? String)
}

public init(path: String = Configuration.fileName, rootPath: String? = nil,
Expand All @@ -179,6 +185,7 @@ public struct Configuration: Equatable {
}
self.init(dict: dict)!
configurationPath = fullPath
hash = dict.description.hashValue
self.rootPath = rootPath
return
} catch YamlParserError.yamlParsing(let message) {
Expand Down Expand Up @@ -260,7 +267,8 @@ private func defaultStringArray(_ object: Any?) -> [String] {

private func validKeys(ruleList: RuleList) -> [String] {
return [
ConfigurationKey.disabledRules,
ConfigurationKey.cachePath,
.disabledRules,
.enabledRules,
.excluded,
.included,
Expand Down
37 changes: 36 additions & 1 deletion Source/SwiftLintFramework/Models/Linter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ extension Rule {
public struct Linter {
public let file: File
private let rules: [Rule]
private let cache: LinterCache?

public var styleViolations: [StyleViolation] {
return getStyleViolations().0
Expand All @@ -64,6 +65,11 @@ public struct Linter {
}

private func getStyleViolations(benchmark: Bool = false) -> ([StyleViolation], [(id: String, time: Double)]) {

if let cached = cachedStyleViolations(benchmark: benchmark) {
return cached
}

if file.sourcekitdFailed {
queuedPrintError("Most rules will be skipped because sourcekitd has failed.")
}
Expand All @@ -78,6 +84,11 @@ public struct Linter {
deprecatedToValidIdentifier[key] = value
}

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 {
queuedPrintError("'\(deprecatedIdentifier)' rule has been renamed to '\(identifier)' and will be " +
"completely removed in a future release.")
Expand All @@ -86,8 +97,32 @@ public struct Linter {
return (violations, ruleTimes)
}

public init(file: File, configuration: Configuration = Configuration()!) {
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(forFile: file, hash: hash) else {
return nil
}

var ruleTimes = [(id: String, time: Double)]()
if benchmark {
// let's assume that all rules should have the same duration and split the duration among them
let totalTime = -start.timeIntervalSinceNow
let fractionedTime = totalTime / TimeInterval(rules.count)
ruleTimes = rules.flatMap { rule in
let id = type(of: rule).description.identifier
return (id, fractionedTime)
}
}

return (cachedViolations, ruleTimes)
}

public init(file: File, configuration: Configuration = Configuration()!, cache: LinterCache? = nil) {
self.file = file
self.cache = cache
rules = configuration.rules
}

Expand Down
119 changes: 119 additions & 0 deletions Source/SwiftLintFramework/Models/LinterCache.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
//
// LinterCache.swift
// SwiftLint
//
// Created by Marcelo Fabri on 12/27/16.
// Copyright © 2016 Realm. All rights reserved.
//

import Foundation
import SourceKittenFramework

public enum LinterCacheError: Error {
case invalidFormat
case differentVersion
case differentConfiguration
}

public final class LinterCache {
private var cache: [String: Any]
private let lock = NSLock()

public init(currentVersion: Version = .current, configurationHash: Int? = nil) {
cache = [
"version": currentVersion.value,
"files": [:]
]
cache["configuration_hash"] = configurationHash
}

public init(cache: Any, currentVersion: Version = .current, configurationHash: Int? = nil) throws {
guard let dictionary = cache as? [String: Any] else {
throw LinterCacheError.invalidFormat
}

guard let version = dictionary["version"] as? String, version == currentVersion.value else {
throw LinterCacheError.differentVersion
}

if dictionary["configuration_hash"] as? Int != configurationHash {
throw LinterCacheError.differentConfiguration
}

self.cache = dictionary
}

public convenience init(contentsOf url: URL, currentVersion: Version = .current,
configurationHash: Int? = nil) throws {
let data = try Data(contentsOf: url)
let json = try JSONSerialization.jsonObject(with: data, options: [])
try self.init(cache: json, currentVersion: currentVersion,
configurationHash: configurationHash)
}

public func cache(violations: [StyleViolation], forFile file: String, fileHash: Int) {
lock.lock()
var filesCache = (cache["files"] as? [String: Any]) ?? [:]
filesCache[file] = [
"violations": violations.map(dictionary(for:)),
"hash": fileHash
]
cache["files"] = filesCache
lock.unlock()
}

public func violations(forFile file: String, hash: Int) -> [StyleViolation]? {
lock.lock()

guard let filesCache = cache["files"] as? [String: Any],
let entry = filesCache[file] as? [String: Any],
let cacheHash = entry["hash"] as? Int,
cacheHash == hash,
let violations = entry["violations"] as? [[String: Any]] else {
lock.unlock()
return nil
}

lock.unlock()
return violations.flatMap { StyleViolation.from(cache: $0, file: file) }
}

public func save(to url: URL) throws {
lock.lock()
let json = toJSON(cache)
lock.unlock()
try json.write(to: url, atomically: true, encoding: .utf8)
}

private func dictionary(for violation: StyleViolation) -> [String: Any] {
return [
"line": violation.location.line ?? NSNull() as Any,
"character": violation.location.character ?? NSNull() as Any,
"severity": violation.severity.rawValue,
"type": violation.ruleDescription.name,
"rule_id": violation.ruleDescription.identifier,
"reason": violation.reason
]
}
}

extension 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,
let reason = cache["reason"] as? String else {
return nil
}

let line = cache["line"] as? Int
let character = cache["character"] as? Int

let ruleDescription = RuleDescription(identifier: ruleId, name: name, description: reason)
let location = Location(file: file, line: line, character: character)
let violation = StyleViolation(ruleDescription: ruleDescription, severity: severity,
location: location, reason: reason)

return violation
}
}
20 changes: 15 additions & 5 deletions Source/swiftlint/Commands/LintCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ struct LintCommand: CommandProtocol {
var violations = [StyleViolation]()
let configuration = Configuration(options: options)
let reporter = reporterFrom(options: options, configuration: configuration)
let cache = LinterCache.makeCache(options: options, configuration: configuration)
let visitorMutationQueue = DispatchQueue(label: "io.realm.swiftlint.lintVisitorMutation")
return configuration.visitLintableFiles(options: options) { linter in
return configuration.visitLintableFiles(options: options, cache: cache) { linter in
let currentViolations: [StyleViolation]
if options.benchmark {
let start = Date()
Expand Down Expand Up @@ -58,6 +59,9 @@ struct LintCommand: CommandProtocol {
fileBenchmark.save()
ruleBenchmark.save()
}

cache?.save(options: options, configuration: configuration)

return LintCommand.successOrExit(numberOfSeriousViolations: numberOfSeriousViolations,
strictWithViolations: options.strict && !violations.isEmpty)
}
Expand Down Expand Up @@ -113,12 +117,14 @@ struct LintOptions: OptionsProtocol {
let benchmark: Bool
let reporter: String
let quiet: Bool
let cachePath: String
let ignoreCache: Bool

// swiftlint:disable line_length
static func create(_ path: String) -> (_ useSTDIN: Bool) -> (_ configurationFile: String) -> (_ strict: Bool) -> (_ useScriptInputFiles: Bool) -> (_ benchmark: Bool) -> (_ reporter: String) -> (_ quiet: Bool) -> LintOptions {
return { useSTDIN in { configurationFile in { strict in { useScriptInputFiles in { benchmark in { reporter in { quiet in
self.init(path: path, useSTDIN: useSTDIN, configurationFile: configurationFile, strict: strict, useScriptInputFiles: useScriptInputFiles, benchmark: benchmark, reporter: reporter, quiet: quiet)
}}}}}}}
static func create(_ path: String) -> (_ useSTDIN: Bool) -> (_ configurationFile: String) -> (_ strict: Bool) -> (_ useScriptInputFiles: Bool) -> (_ benchmark: Bool) -> (_ reporter: String) -> (_ quiet: Bool) -> (_ cachePath: String) -> (_ ignoreCache: Bool) -> LintOptions {
return { useSTDIN in { configurationFile in { strict in { useScriptInputFiles in { benchmark in { reporter in { quiet in { cachePath in { ignoreCache in
self.init(path: path, useSTDIN: useSTDIN, configurationFile: configurationFile, strict: strict, useScriptInputFiles: useScriptInputFiles, benchmark: benchmark, reporter: reporter, quiet: quiet, cachePath: cachePath, ignoreCache: ignoreCache)
}}}}}}}}}
}

static func evaluate(_ mode: CommandMode) -> Result<LintOptions, CommandantError<CommandantError<()>>> {
Expand All @@ -137,5 +143,9 @@ struct LintOptions: OptionsProtocol {
<*> mode <| Option(key: "reporter", defaultValue: "",
usage: "the reporter used to log errors and warnings")
<*> mode <| quietOption(action: "linting")
<*> mode <| Option(key: "cache-path", defaultValue: "",
usage: "the location of the cache used when linting")
<*> mode <| Option(key: "no-cache", defaultValue: false,
usage: "ignore cache when linting")
}
}
Loading

0 comments on commit 4f225a4

Please sign in to comment.