Skip to content

Commit

Permalink
[Fix #10613] Allow parallel inspection for autocorrect
Browse files Browse the repository at this point in the history
Allow the `-P`/`--parallel` flag together with the autocorrection
flags `--[safe-]auto-correct`/`-a` `--auto-correct-all`/`-A` and
`--fix-layout`. Also make this the default, so that `--no-parallel`
has to be given for a single core run.

The algorithm for normal parallel inspection is followed, so that a
cache warm-up phase is run first. This phase only does caching of
inspection results. It doesn't update any files. Then it does a single
core run that takes advantage of the cached results and does the
corrections. The speed gain from this approach is probably smaller
than writing files in parallel, but it's a pretty small change that
still outputs correction results in the same way as before.

Also fix the documentation for `--parallel`, which did not mention
`--no-parallel`.
  • Loading branch information
jonas054 authored and bbatsov committed Jul 11, 2022
1 parent ac20d11 commit d8b4235
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 25 deletions.
1 change: 1 addition & 0 deletions changelog/change_allow_parallel_autocorrect.md
@@ -0,0 +1 @@
* [#10613](https://github.com/rubocop/rubocop/issues/10613): Allow autocorrecting with -P/--parallel and make it the default. ([@jonas054][])
4 changes: 2 additions & 2 deletions docs/modules/ROOT/pages/usage/basic_usage.adoc
Expand Up @@ -237,8 +237,8 @@ $ rubocop --only Rails/Blank,Layout/HeredocIndentation,Naming/FileName
| `-o/--out`
| Write output to a file instead of STDOUT.

| `--parallel`
| Use available CPUs to execute inspection in parallel.
| `--[no-]parallel`
| Use available CPUs to execute inspection in parallel. Default is parallel.

| `-r/--require`
| Require Ruby file (see xref:extensions.adoc#loading-extensions[Loading Extensions]).
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cli.rb
Expand Up @@ -12,6 +12,7 @@ class CLI
color debug display_style_guide display_time display_only_fail_level_offenses
display_only_failed except extra_details fail_level fix_layout format
ignore_disable_comments lint only only_guide_cops require safe
autocorrect safe_autocorrect autocorrect_all
].freeze

class Finished < StandardError; end
Expand Down
9 changes: 3 additions & 6 deletions lib/rubocop/options.rb
Expand Up @@ -421,12 +421,9 @@ def disable_parallel_when_invalid_option_combo
end

def invalid_arguments_for_parallel
[('--auto-gen-config' if @options.key?(:auto_gen_config)),
('-F/--fail-fast' if @options.key?(:fail_fast)),
('-x/--fix-layout' if @options.key?(:fix_layout)),
('-a/--autocorrect' if @options.key?(:safe_autocorrect)),
('-A/--autocorrect-all' if @options.key?(:autocorrect_all)),
('--cache false' if @options > { cache: 'false' })].compact
[('--auto-gen-config' if @options.key?(:auto_gen_config)),
('-F/--fail-fast' if @options.key?(:fail_fast)),
('--cache false' if @options > { cache: 'false' })].compact
end

def only_includes_redundant_disable?
Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/runner.rb
Expand Up @@ -63,8 +63,12 @@ def aborting?
# Warms up the RuboCop cache by forking a suitable number of RuboCop
# instances that each inspects its allotted group of files.
def warm_cache(target_files)
saved_options = @options.dup
puts 'Running parallel inspection' if @options[:debug]
%i[autocorrect safe_autocorrect].each { |opt| @options[opt] = false }
Parallel.each(target_files) { |target_file| file_offenses(target_file) }
ensure
@options = saved_options
end

def find_target_files(paths)
Expand Down
12 changes: 8 additions & 4 deletions spec/rubocop/cli_spec.rb
Expand Up @@ -213,16 +213,20 @@ def and_with_args
end
end

# NOTE: Cannot be autocorrected with `parallel`.
context 'when specifying `--debug` and `-a` options`' do
it 'fails with an error message' do
it 'uses parallel inspection when correcting the file' do
create_file('example1.rb', <<~RUBY)
# frozen_string_literal: true
puts 'hello'
puts "hello"
RUBY
expect(cli.run(['--debug', '-a'])).to eq(0)
expect($stdout.string).not_to include('Use parallel by default.')
expect($stdout.string).to include('Use parallel by default.')
expect(File.read('example1.rb')).to eq(<<~RUBY)
# frozen_string_literal: true
puts 'hello'
RUBY
end
end

Expand Down
23 changes: 10 additions & 13 deletions spec/rubocop/options_spec.rb
Expand Up @@ -260,29 +260,26 @@ def abs(path)

context 'combined with an autocorrect argument' do
context 'combined with --fix-layout' do
it 'ignores --parallel' do
it 'allows --parallel' do
options.parse %w[--parallel --fix-layout]
expect($stdout.string).to include('-P/--parallel is being ignored because it is not ' \
'compatible with -x/--fix-layout')
expect(options.instance_variable_get(:@options).keys).not_to include(:parallel)
expect($stdout.string).not_to include('-P/--parallel is being ignored')
expect(options.instance_variable_get(:@options).keys).to include(:parallel)
end
end

context 'combined with --autocorrect' do
it 'ignores --parallel' do
it 'allows --parallel' do
options.parse %w[--parallel --autocorrect]
expect($stdout.string).to include('-P/--parallel is being ignored because it is not ' \
'compatible with -a/--autocorrect')
expect(options.instance_variable_get(:@options).keys).not_to include(:parallel)
expect($stdout.string).not_to include('-P/--parallel is being ignored')
expect(options.instance_variable_get(:@options).keys).to include(:parallel)
end
end

context 'combined with --autocorrect-all' do
it 'ignores --parallel' do
it 'allows --parallel' do
options.parse %w[--parallel --autocorrect-all]
expect($stdout.string).to include('-P/--parallel is being ignored because it is not ' \
'compatible with -A/--autocorrect-all')
expect(options.instance_variable_get(:@options).keys).not_to include(:parallel)
expect($stdout.string).not_to include('-P/--parallel is being ignored')
expect(options.instance_variable_get(:@options).keys).to include(:parallel)
end
end
end
Expand All @@ -309,7 +306,7 @@ def abs(path)
it 'ignores --parallel and lists both incompatible arguments' do
options.parse %w[--parallel --fail-fast --autocorrect]
expect($stdout.string).to include('-P/--parallel is being ignored because it is not ' \
'compatible with -F/--fail-fast, -a/--autocorrect')
'compatible with -F/--fail-fast')
expect(options.instance_variable_get(:@options).keys).not_to include(:parallel)
end
end
Expand Down

0 comments on commit d8b4235

Please sign in to comment.