Reenable cache #1530

jpsim · 2017-05-18T22:02:47Z

Significantly extended from #1516 /cc @victorpimentel @marcelofabri

…files with a weak algorithm.

applying minor edits. Remove `configurationDescription` from breaking changes in changelog since additions aren't breaking changes.

rather than use hardcoded, duplicate string literals

rather than just when using one of many code paths in one of many initializers

to provide more complete descriptions for cache invalidation purposes.

by respecting the value of ignoreCache rather than always passing 'true'

to make the cache play nicely with nested configurations

…Line

and make cacheURL an internal member of Configuration

SwiftLintBot · 2017-05-18T22:18:13Z

	1 Warning
⚠️	Big PR

	12 Messages
📖	Linting WordPress with this PR took 9.41s vs 12.37s on master (23% faster)
📖	Linting Alamofire with this PR took 2.27s vs 2.39s on master (5% faster)
📖	Linting Firefox with this PR took 9.62s vs 10.15s on master (5% faster)
📖	Linting Kickstarter with this PR took 12.64s vs 13.74s on master (8% faster)
📖	Linting Moya with this PR took 0.31s vs 0.33s on master (6% faster)
📖	Linting Nimble with this PR took 1.25s vs 1.33s on master (6% faster)
📖	Linting Aerial with this PR took 0.35s vs 0.36s on master (2% faster)
📖	Linting Realm with this PR took 1.95s vs 1.99s on master (2% faster)
📖	Linting SourceKitten with this PR took 0.83s vs 0.86s on master (3% faster)
📖	Linting Sourcery with this PR took 2.02s vs 2.09s on master (3% faster)
📖	Linting Swift with this PR took 8.94s vs 9.65s on master (7% faster)
📖	Linting Quick with this PR took 0.42s vs 0.43s on master (2% faster)

Generated by 🚫 danger

codecov-io · 2017-05-18T22:22:32Z

Codecov Report

Merging #1530 into master will increase coverage by 0.78%.
The diff coverage is 78.37%.

@@            Coverage Diff             @@
##           master    #1530      +/-   ##
==========================================
+ Coverage   82.78%   83.57%   +0.78%     
==========================================
  Files         187      186       -1     
  Lines        9342     9547     +205     
==========================================
+ Hits         7734     7979     +245     
+ Misses       1608     1568      -40

Impacted Files	Coverage Δ
...Framework/Extensions/NSFileManager+SwiftLint.swift	`70.58% <0%> (-9.42%)`	⬇️
...s/SwiftLintFrameworkTests/ConfigurationTests.swift	`81.73% <0%> (-0.8%)`	⬇️
Source/swiftlint/Commands/LintCommand.swift	`0% <0%> (ø)`	⬆️
...iftlint/Extensions/Configuration+CommandLine.swift	`0% <0%> (ø)`	⬆️
...iftLintFramework/Protocols/RuleConfiguration.swift	`50% <0%> (-50%)`	⬇️
...SwiftLintFramework/Rules/PrivateUnitTestRule.swift	`87.5% <100%> (+0.46%)`	⬆️
Source/SwiftLintFramework/Rules/CustomRules.swift	`98.11% <100%> (+0.15%)`	⬆️
Source/SwiftLintFramework/Protocols/Rule.swift	`100% <100%> (+12.5%)`	⬆️
...s/RuleConfigurations/AttributesConfiguration.swift	`65.21% <100%> (+17.39%)`	⬆️
...k/Rules/RuleConfigurations/NameConfiguration.swift	`97.77% <100%> (+13.33%)`	⬆️
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b8bde9...cea6f43. Read the comment docs.

norio-nomura · 2017-05-19T01:11:52Z

Source/SwiftLintFramework/Models/Configuration.swift

+            "root": rootPath ?? FileManager.default.currentDirectoryPath,
+            "rules": cacheRulesDescriptions
+        ]
+        if let jsonData = try? JSONSerialization.data(withJSONObject: dict),


AFAIK, the json string created by JSONSerialization is non-deterministic in dictionary key order on Linux.
Should we consider something about it?

None of this relies on key order remaining stable.

Oh, yes it does of course. The cache.json file doesn't rely on ordering, but since this is embedded as a string, that does matter.

What do you mean though that the order is non-deterministic?

configuration.cacheDescription is used for key to getting cache in cache(violations:forFile:configuration:) and violations(forFile:configuration:).
cacheDescription is jsonstring serialized by JSONSerialization.
So, getting cache depends on non-deterministic string keys.

Yup. The fact that the result is always constant with macOS is just such an implementation, it is not guaranteed as a specification.

But actually, do we use Set here? Or just Dictionary?

Configuration.cacheDescription > Rule.cacheDescription > RuleConfiguration.consoleDescription > https://github.com/realm/SwiftLint/blob/master/Source/SwiftLintFramework/Rules/RuleConfigurations/AttributesConfiguration.swift#L13-L20

🤔

We could revisit those consoleDescription to avoid using Sets. In this particular case, it'd be probably even better for the user if we always sorted the Sets. Or even don't use them at all.

Took a stab at fixing this in 0c09520 & cea6f43.

…ionProvider

jpsim · 2017-05-22T05:43:04Z

Will merge. We should do a prerelease of SwiftLint with this and all the many other changes to make sure it doesn't break things in the wild since there's so much.

marcelofabri · 2017-05-22T06:02:15Z

🎉

jpsim · 2017-05-22T21:34:06Z

0.19.0-rc.1

victorpimentel and others added 16 commits May 18, 2017 13:39

Change LinterCache to use file modification dates instead of hashing …

5ed010c

…files with a weak algorithm.

move changelog entries to appropriate section

547a870

applying minor edits. Remove `configurationDescription` from breaking changes in changelog since additions aren't breaking changes.

update changelog entries to reflect that many changes were made

f1f64ff

simplify FileManager.modificationDate(forFileAtPath:) impl

923f38c

add LinterCache.Key enum

70bcb2c

rather than use hardcoded, duplicate string literals

derive Configuration.cacheDescription in all cases

903b48f

rather than just when using one of many code paths in one of many initializers

add CacheDescriptionProvider protocol

f639ae9

to provide more complete descriptions for cache invalidation purposes.

re-enable cache when running lint command

98ea7ae

by respecting the value of ignoreCache rather than always passing 'true'

don't overwrite cache results from different versions

258a80b

cache each file with its own configuration cache description

bd7169e

to make the cache play nicely with nested configurations

simplify serialized cache structure

690b567

use --cache-path as directory rather than cache file

da91a2e

hoist knowledge of LintOptions.ignoreCache out of LinterCache+Command…

bb2b9a2

…Line

keep a single cache file for all of SwiftLint

97bdac2

move LinterCache+CommandLine to SwiftLintFramework

ca08d65

and make cacheURL an internal member of Configuration

rewrite cache tests

186231e

jpsim requested a review from marcelofabri May 18, 2017 22:03

jpsim mentioned this pull request May 18, 2017

Reenable cache #1516

Closed

norio-nomura reviewed May 19, 2017

View reviewed changes

marcelofabri approved these changes May 19, 2017

View reviewed changes

jpsim added 2 commits May 19, 2017 09:25

ensure deterministic consoleDescription & cacheDescription generation

0c09520

make PrivateUnitTestRule & its configuration conform to CacheDescript…

cea6f43

…ionProvider

marcelofabri approved these changes May 21, 2017

View reviewed changes

jpsim merged commit 572754f into master May 22, 2017

jpsim deleted the jp-reenable-cache-v2 branch May 22, 2017 05:43

marcelofabri mentioned this pull request May 22, 2017

Fix stale cache by using file modification dates instead of hashes #1311

Closed

This was referenced May 22, 2017

Nested Configuration Not Working? #1289

Closed

Investigate cases in which cache is stale #1184

Closed

Support for passing multiple paths to swiftlint lint and autocorrect #1191

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reenable cache #1530

Reenable cache #1530

jpsim commented May 18, 2017

SwiftLintBot commented May 18, 2017 •

edited

Loading

codecov-io commented May 18, 2017 •

edited

Loading

norio-nomura May 19, 2017

jpsim May 19, 2017

jpsim May 19, 2017 •

edited

Loading

jpsim May 19, 2017

norio-nomura May 19, 2017

norio-nomura May 19, 2017

marcelofabri May 19, 2017

norio-nomura May 19, 2017

marcelofabri May 19, 2017

jpsim May 19, 2017

jpsim commented May 22, 2017

marcelofabri commented May 22, 2017

jpsim commented May 22, 2017

Reenable cache #1530

Reenable cache #1530

Conversation

jpsim commented May 18, 2017

SwiftLintBot commented May 18, 2017 • edited Loading

codecov-io commented May 18, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpsim May 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpsim commented May 22, 2017

marcelofabri commented May 22, 2017

jpsim commented May 22, 2017

SwiftLintBot commented May 18, 2017 •

edited

Loading

codecov-io commented May 18, 2017 •

edited

Loading

jpsim May 19, 2017 •

edited

Loading