From efb16af8eacffc18c5e34e5b5d639a36745c24f7 Mon Sep 17 00:00:00 2001 From: Maxim Krizhanovsky Date: Fri, 28 Sep 2018 13:15:54 +0300 Subject: [PATCH] [Fix #5980] Implement safe and safe-auto-correct options --- CHANGELOG.md | 4 ++++ config/default.yml | 4 ++-- lib/rubocop/cop/autocorrect_logic.rb | 8 +++++++- lib/rubocop/cop/registry.rb | 13 +++++++++++-- lib/rubocop/cop/team.rb | 5 +++-- lib/rubocop/options.rb | 7 +++++++ manual/cops_performance.md | 3 +-- manual/cops_style.md | 3 +-- spec/rubocop/cop/registry_spec.rb | 10 +++++++++- spec/rubocop/options_spec.rb | 2 ++ 10 files changed, 47 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e20dc939f83..77935e15d2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master (unreleased) +### New features + +* [#5980](https://github.com/rubocop-hq/rubocop/issues/5980): Add --safe and --safe-auto-correct options. ([@Darhazer][]) + ### Bug fixes * [#6330](https://github.com/rubocop-hq/rubocop/issues/6330): Fix an error for `Rails/ReversibleMigration` when using variable assignment. ([@koic][], [@scottmatthewman][]) diff --git a/config/default.yml b/config/default.yml index 55e558891f6..1c77ed2e704 100644 --- a/config/default.yml +++ b/config/default.yml @@ -2152,7 +2152,7 @@ Performance/TimesMap: Enabled: true VersionAdded: 0.36 VersionChanged: 0.50 - SafeAutocorrect: false # see https://github.com/rubocop-hq/rubocop/issues/4658 + SafeAutoCorrect: false # see https://github.com/rubocop-hq/rubocop/issues/4658 Performance/UnfreezeString: Description: 'Use unary plus to get an unfrozen string literal.' @@ -3640,7 +3640,7 @@ Style/NumericPredicate: # This will change to a new method call which isn't guaranteed to be on the # object. Switching these methods has to be done with knowledge of the types # of the variables which rubocop doesn't have. - SafeAutocorrect: false + SafeAutoCorrect: false AutoCorrect: false Enabled: true VersionAdded: 0.42 diff --git a/lib/rubocop/cop/autocorrect_logic.rb b/lib/rubocop/cop/autocorrect_logic.rb index b4f6653cd13..c5b75acf3ae 100644 --- a/lib/rubocop/cop/autocorrect_logic.rb +++ b/lib/rubocop/cop/autocorrect_logic.rb @@ -20,7 +20,13 @@ def autocorrect_enabled? # allow turning off autocorrect on a cop by cop basis return true unless cop_config - cop_config['AutoCorrect'] != false + return false if cop_config['AutoCorrect'] == false + + if @options.fetch(:safe_auto_correct, false) + return cop_config.fetch('SafeAutoCorrect', true) + end + + true end end end diff --git a/lib/rubocop/cop/registry.rb b/lib/rubocop/cop/registry.rb index d2c8ed7d70d..9bdc2ebc85c 100644 --- a/lib/rubocop/cop/registry.rb +++ b/lib/rubocop/cop/registry.rb @@ -117,9 +117,18 @@ def length @registry.size end - def enabled(config, only) + def enabled(config, only, only_safe = false) select do |cop| - config.for_cop(cop).fetch('Enabled') || only.include?(cop.cop_name) + only.include?(cop.cop_name) || enabled?(cop, config, only_safe) + end + end + + def enabled?(cop, config, only_safe) + cfg = config.for_cop(cop) + if only_safe + cfg.fetch('Enabled') && cfg.fetch('Safe', true) + else + cfg.fetch('Enabled') end end diff --git a/lib/rubocop/cop/team.rb b/lib/rubocop/cop/team.rb index 4175ff19803..08fc9da2d3a 100644 --- a/lib/rubocop/cop/team.rb +++ b/lib/rubocop/cop/team.rb @@ -45,8 +45,9 @@ def inspect_file(processed_source) end def cops - only_options = @options.fetch(:only, []) - @cops ||= @cop_classes.enabled(@config, only_options).map do |cop_class| + only = @options.fetch(:only, []) + safe = @options.fetch(:safe, false) + @cops ||= @cop_classes.enabled(@config, only, safe).map do |cop_class| cop_class.new(@config, @options) end end diff --git a/lib/rubocop/options.rb b/lib/rubocop/options.rb index a51a432b0c4..5be6112cd8a 100644 --- a/lib/rubocop/options.rb +++ b/lib/rubocop/options.rb @@ -156,6 +156,8 @@ def add_boolean_flags(opts) option(opts, '-R', '--rails') option(opts, '-a', '--auto-correct') + option(opts, '--safe') + option(opts, '--[no-]color') option(opts, '-v', '--version') @@ -173,6 +175,9 @@ def add_aliases(opts) @options[:only] << 'Layout' @options[:auto_correct] = true end + option(opts, '--safe-auto-correct') do + @options[:auto_correct] = true + end end def add_list_options(opts) @@ -410,8 +415,10 @@ module OptionsHelp extra_details: 'Display extra details in offense messages.', rails: 'Run extra Rails cops.', lint: 'Run only lint cops.', + safe: 'Run only safe cops.', list_target_files: 'List all files RuboCop will inspect.', auto_correct: 'Auto-correct offenses.', + safe_auto_correct: 'Run auto-correct only when it\'s safe.', fix_layout: 'Run only layout cops, with auto-correct on.', color: 'Force color output on or off.', version: 'Display version.', diff --git a/manual/cops_performance.md b/manual/cops_performance.md index a71083f5f2e..de95f713bf4 100644 --- a/manual/cops_performance.md +++ b/manual/cops_performance.md @@ -857,7 +857,7 @@ This cop identifies places where `gsub` can be replaced by Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged --- | --- | --- | --- | --- -Enabled | Yes | Yes | 0.36 | 0.5 +Enabled | Yes | Yes (Unsafe) | 0.36 | 0.5 This cop checks for .times.map calls. In most cases such calls can be replaced @@ -882,7 +882,6 @@ end Name | Default value | Configurable values --- | --- | --- AutoCorrect | `false` | Boolean -SafeAutocorrect | `false` | Boolean ## Performance/UnfreezeString diff --git a/manual/cops_style.md b/manual/cops_style.md index a6e334be531..5779e8250f1 100644 --- a/manual/cops_style.md +++ b/manual/cops_style.md @@ -4005,7 +4005,7 @@ Strict | `false` | Boolean Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged --- | --- | --- | --- | --- -Enabled | No | Yes | 0.42 | 0.59 +Enabled | No | Yes (Unsafe) | 0.42 | 0.59 This cop checks for usage of comparison operators (`==`, `>`, `<`) to test numbers as zero, positive, or negative. @@ -4057,7 +4057,6 @@ bar.baz > 0 Name | Default value | Configurable values --- | --- | --- -SafeAutocorrect | `false` | Boolean AutoCorrect | `false` | Boolean EnforcedStyle | `predicate` | `predicate`, `comparison` IgnoredMethods | `[]` | Array diff --git a/spec/rubocop/cop/registry_spec.rb b/spec/rubocop/cop/registry_spec.rb index 21acc167d3c..d5f53bcef95 100644 --- a/spec/rubocop/cop/registry_spec.rb +++ b/spec/rubocop/cop/registry_spec.rb @@ -138,7 +138,10 @@ class Foo < Cop context '#enabled' do let(:config) do - RuboCop::Config.new('Test/IndentArray' => { 'Enabled' => false }) + RuboCop::Config.new( + 'Test/IndentArray' => { 'Enabled' => false }, + 'RSpec/Foo' => { 'Safe' => false } + ) end it 'selects cops which are enabled in the config' do @@ -148,6 +151,11 @@ class Foo < Cop it 'overrides config if :only includes the cop' do expect(registry.enabled(config, ['Test/IndentArray'])).to eql(cops) end + + it 'selects only safe cops if :safe passed' do + enabled_cops = registry.enabled(config, [], true) + expect(enabled_cops).not_to include(RuboCop::Cop::RSpec::Foo) + end end it 'exposes a list of cop names' do diff --git a/spec/rubocop/options_spec.rb b/spec/rubocop/options_spec.rb index 751cedc349f..fd38089ee37 100644 --- a/spec/rubocop/options_spec.rb +++ b/spec/rubocop/options_spec.rb @@ -101,6 +101,7 @@ def abs(path) -S, --display-style-guide Display style guide URLs in offense messages. -R, --rails Run extra Rails cops. -a, --auto-correct Auto-correct offenses. + --safe Run only safe cops. --[no-]color Force color output on or off. -v, --version Display version. -V, --verbose-version Display verbose version. @@ -108,6 +109,7 @@ def abs(path) parallel. -l, --lint Run only lint cops. -x, --fix-layout Run only layout cops, with auto-correct on. + --safe-auto-correct Run auto-correct only when it's safe. -s, --stdin FILE Pipe source from STDIN, using FILE in offense reports. This is useful for editor integration. OUTPUT