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

Auto-correct does not cache #5082

Closed
cabello opened this issue Nov 21, 2017 · 2 comments
Closed

Auto-correct does not cache #5082

cabello opened this issue Nov 21, 2017 · 2 comments

Comments

@cabello
Copy link

cabello commented Nov 21, 2017

The first time I run bundle exec rubocop is slower than the sub-sequent ones because of the awesome caching feature.

When using the auto-correct flag -a, it seems to never cache and every single run takes on average the same amount of time.

Expected behavior

Caching should also work when auto-correcting files.

Actual behavior

Every run takes consistently the same amount of time.

Steps to reproduce the problem

➤ time bundle exec rubocop -a
Inspecting 269 files
.............................................................................................................................................................................................................................................................................

269 files inspected, no offenses detected
       10.30 real         9.93 user         0.29 sys
➤ time bundle exec rubocop -a
Inspecting 269 files
.............................................................................................................................................................................................................................................................................

269 files inspected, no offenses detected
       10.18 real         9.80 user         0.29 sys
➤ time bundle exec rubocop -a
Inspecting 269 files
.............................................................................................................................................................................................................................................................................

269 files inspected, no offenses detected
       10.16 real         9.77 user         0.31 sys

Compare to:

➤ time bundle exec rubocop
Inspecting 269 files
.............................................................................................................................................................................................................................................................................

269 files inspected, no offenses detected
       10.99 real        10.54 user         0.40 sys
➤ time bundle exec rubocop
Inspecting 269 files
.............................................................................................................................................................................................................................................................................

269 files inspected, no offenses detected
        1.31 real         1.04 user         0.25 sys

RuboCop version

➤ bundle exec rubocop -V
0.49.1 (using Parser 2.4.0.0, running on ruby 2.3.5 x86_64-darwin16)

Is that a reasonable request? Thanks for the awesome project! ❤️

@pocke
Copy link
Collaborator

pocke commented Nov 22, 2017

It seems good to me. But we must refactor the code for auto-correction to implement this feature.

First, we should add auto_correct to NON_CHANGING.

diff --git a/lib/rubocop/result_cache.rb b/lib/rubocop/result_cache.rb
index bd5bb85e..01c240ef 100644
--- a/lib/rubocop/result_cache.rb
+++ b/lib/rubocop/result_cache.rb
@@ -7,7 +7,7 @@ require 'etc'
 module RuboCop
   # Provides functionality for caching rubocop runs.
   class ResultCache
-    NON_CHANGING = %i[color format formatters out debug fail_level
+    NON_CHANGING = %i[color format formatters out debug fail_level auto_correct
                       cache fail_fast stdin parallel].freeze
 
     # Remove old files so that the cache doesn't grow too big. When the
diff --git a/lib/rubocop/runner.rb b/lib/rubocop/runner.rb
index fc405261..46506c7c 100644
--- a/lib/rubocop/runner.rb
+++ b/lib/rubocop/runner.rb
@@ -175,8 +175,6 @@ module RuboCop
         # cops related to calculating the Max parameters for Metrics cops. We
         # need to do that processing and can not use caching.
         !@options[:auto_gen_config] &&
-        # Auto-correction needs a full run. It can not use cached results.
-        !@options[:auto_correct] &&
         # We can't cache results from code which is piped in to stdin
         !@options[:stdin]
     end

But it does not work well from we just changing them.
For example

$ cat test.rb
p "a"



$ rubocop 
Inspecting 1 file
C

Offenses:

test.rb:1:3: C: Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
p "a"
  ^^^

1 file inspected, 1 offense detected



# It does not execute auto-correction!
$ rubocop -a
Inspecting 1 file
C

Offenses:

test.rb:1:3: C: Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
p "a"
  ^^^

1 file inspected, 1 offense detected

jonas054 added a commit to jonas054/rubocop that referenced this issue May 5, 2018
The old solution was to turn off caching completely when running
with --auto-correct. The reason was that in order to correct a
file, we need more than just the cached result - a full inspection
is necessary.

But for files that have no offenses, there's no reason to let
--auto-correct affect the normal behavior. We can use cached
results in those cases.
@bbatsov bbatsov closed this as completed in 9bdbaba May 5, 2018
@jonas054
Copy link
Collaborator

jonas054 commented May 5, 2018

Thanks, @pocke for the initial work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants