[Fix #7611] An unsafe cop cannot have a safe auto-correction

lib/rubocop/config_validator.rb

@@ -24,6 +24,7 @@ def initialize(config)
 
     def validate
       check_cop_config_value(@config)
+      reject_conflicting_safe_settings
 
       # Don't validate RuboCop's own files further. Avoids infinite recursion.
       return if @config.internal?
@@ -168,6 +169,18 @@ def reject_mutually_exclusive_defaults
       raise ValidationError, msg
     end
 
+    def reject_conflicting_safe_settings
+      @config.each do |name, cop_config|
+        next unless cop_config.is_a?(Hash)
+        next unless cop_config['Safe'] == false &&
+                    cop_config['SafeAutoCorrect'] == true
+
+        msg = 'Unsafe cops cannot have a safe auto-correction ' \
+              "(section #{name} in #{smart_loaded_path})"
+        raise ValidationError, msg
+      end
+    end
+
     def check_cop_config_value(hash, parent = nil)
       hash.each do |key, value|
         check_cop_config_value(value, key) if value.is_a?(Hash)

lib/rubocop/cop/autocorrect_logic.rb

@@ -24,15 +24,18 @@ def disable_uncorrectable?
         @options[:disable_uncorrectable] == true
       end
 
+      def safe_autocorrect?
+        cop_config.fetch('Safe', true) &&
+          cop_config.fetch('SafeAutoCorrect', true)
+      end
+
       def autocorrect_enabled?
         # allow turning off autocorrect on a cop by cop basis
         return true unless cop_config
 
         return false if cop_config['AutoCorrect'] == false
 
-        if @options.fetch(:safe_auto_correct, false)
-          return cop_config.fetch('SafeAutoCorrect', true)
-        end
+        return safe_autocorrect? if @options.fetch(:safe_auto_correct, false)
 
         true
       end

manual/cops_lint.md

@@ -1304,7 +1304,7 @@ end
 
 Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
 --- | --- | --- | --- | ---
-Enabled | No | Yes  | 0.78 | -
+Enabled | No | Yes (Unsafe) | 0.78 | -
 
 `Dir[...]` and `Dir.glob(...)` do not make any guarantees about
 the order in which files are returned. The final order is
@@ -1470,7 +1470,7 @@ puts(x + y)
 
 Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
 --- | --- | --- | --- | ---
-Enabled | No | Yes  | 0.41 | -
+Enabled | No | Yes (Unsafe) | 0.41 | -
 
 This cop checks for quotes and commas in %w, e.g. `%w('foo', "bar")`
 

manual/cops_style.md

@@ -754,7 +754,7 @@ end
 
 Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
 --- | --- | --- | --- | ---
-Disabled | No | Yes  | 0.9 | 0.27
+Disabled | No | Yes (Unsafe) | 0.9 | 0.27
 
 This cop enforces the use of consistent method names
 from the Enumerable module.
@@ -2271,7 +2271,7 @@ EnforcedStyle | `annotated` | `annotated`, `template`, `unannotated`
 
 Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
 --- | --- | --- | --- | ---
-Enabled | No | Yes  | 0.36 | 0.79
+Enabled | No | Yes (Unsafe) | 0.36 | 0.79
 
 This cop is designed to help you transition from mutable string literals
 to frozen string literals.
@@ -2813,7 +2813,7 @@ end
 
 Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
 --- | --- | --- | --- | ---
-Enabled | No | Yes  | 0.48 | -
+Enabled | No | Yes (Unsafe) | 0.48 | -
 
 This cop check for usages of not (`not` or `!`) called on a method
 when an inverse of that method can be used instead.
@@ -4992,7 +4992,7 @@ puts Regexp.last_match(1)
 
 Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
 --- | --- | --- | --- | ---
-Enabled | No | Yes  | 0.41 | 0.70
+Enabled | No | Yes (Unsafe) | 0.41 | 0.70
 
 This cop (by default) checks for uses of methods Hash#has_key? and
 Hash#has_value? where it enforces Hash#key? and Hash#value?
@@ -6452,7 +6452,7 @@ warn('hello')
 
 Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
 --- | --- | --- | --- | ---
-Disabled | No | Yes  | 0.52 | 0.75
+Disabled | No | Yes (Unsafe) | 0.52 | 0.75
 
 This cop checks for the use of strings as keys in hashes. The use of
 symbols is preferred instead.
@@ -7429,7 +7429,7 @@ WordRegex | `(?-mix:\A(?:\p{Word}|\p{Word}-\p{Word}|\n|\t)+\z)` |
 
 Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
 --- | --- | --- | --- | ---
-Enabled | No | Yes  | 0.49 | 0.75
+Enabled | No | Yes (Unsafe) | 0.49 | 0.75
 
 This cop can either enforce or forbid Yoda conditions,
 i.e. comparison operations where the order of expression is reversed.
@@ -7504,7 +7504,7 @@ EnforcedStyle | `forbid_for_all_comparison_operators` | `forbid_for_all_comparis
 
 Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
 --- | --- | --- | --- | ---
-Enabled | No | Yes  | 0.37 | 0.39
+Enabled | No | Yes (Unsafe) | 0.37 | 0.39
 
 This cop checks for numeric comparisons that can be replaced
 by a predicate method, such as receiver.length == 0,

spec/rubocop/config_spec.rb

@@ -389,6 +389,41 @@
         end
       end
     end
+
+    describe 'conflicting Safe settings' do
+      context 'when the configuration includes an unsafe cop that is ' \
+              'explicitly declared to have a safe auto-correction' do
+        before do
+          create_file(configuration_path, <<~YAML)
+            Style/PreferredHashMethods:
+              Safe: false
+              SafeAutoCorrect: true
+          YAML
+        end
+
+        it 'raises validation error' do
+          expect { configuration.validate }
+            .to raise_error(
+              RuboCop::ValidationError,
+              /Unsafe cops cannot have a safe auto-correction/
+            )
+        end
+      end
+
+      context 'when the configuration includes an unsafe cop without ' \
+              'a declaration of its auto-correction' do
+        before do
+          create_file(configuration_path, <<~YAML)
+            Style/PreferredHashMethods:
+              Safe: false
+          YAML
+        end
+
+        it 'does not raise validation error' do
+          expect { configuration.validate }.not_to raise_error
+        end
+      end
+    end
   end
 
   describe '#make_excludes_absolute' do

spec/rubocop/cop/cop_spec.rb

@@ -304,4 +304,29 @@
       end
     end
   end
+
+  describe '#safe_autocorrect?' do
+    subject { cop.safe_autocorrect? }
+
+    let(:config) { RuboCop::Config.new('Cop/Cop' => cop_config) }
+    let(:cop) { described_class.new(config) }
+
+    context 'when cop is declared unsafe' do
+      let(:cop_config) { { 'Safe' => false } }
+
+      it { is_expected.to be(false) }
+    end
+
+    context 'when auto-correction of the cop is declared unsafe' do
+      let(:cop_config) { { 'SafeAutoCorrect' => false } }
+
+      it { is_expected.to be(false) }
+    end
+
+    context 'when safety is undeclared' do
+      let(:cop_config) { {} }
+
+      it { is_expected.to be(true) }
+    end
+  end
 end

tasks/cops_documentation.rake

@@ -16,7 +16,7 @@ task generate_cops_documentation: :yard_for_generate_documentation do
 
   def cops_body(config, cop, description, examples_objects, pars)
     content = h2(cop.cop_name)
-    content << properties(config, cop)
+    content << properties(cop.new(config))
     content << "#{description}\n"
     content << examples(examples_objects) if examples_objects.count.positive?
     content << configurations(pars)
@@ -32,24 +32,23 @@ task generate_cops_documentation: :yard_for_generate_documentation do
   end
 
   # rubocop:disable Metrics/MethodLength
-  def properties(config, cop)
+  def properties(cop_instance)
     header = [
       'Enabled by default', 'Safe', 'Supports autocorrection', 'VersionAdded',
       'VersionChanged'
     ]
-    config = config.for_cop(cop)
-    safe_auto_correct = config.fetch('SafeAutoCorrect', true)
-    autocorrect = if cop.new.support_autocorrect?
-                    "Yes #{'(Unsafe)' unless safe_auto_correct}"
+    autocorrect = if cop_instance.support_autocorrect?
+                    "Yes #{'(Unsafe)' unless cop_instance.safe_autocorrect?}"
                   else
                     'No'
                   end
+    cop_config = cop_instance.cop_config
     content = [[
-      config.fetch('Enabled') ? 'Enabled' : 'Disabled',
-      config.fetch('Safe', true) ? 'Yes' : 'No',
+      cop_config.fetch('Enabled') ? 'Enabled' : 'Disabled',
+      cop_config.fetch('Safe', true) ? 'Yes' : 'No',
       autocorrect,
-      config.fetch('VersionAdded', '-'),
-      config.fetch('VersionChanged', '-')
+      cop_config.fetch('VersionAdded', '-'),
+      cop_config.fetch('VersionChanged', '-')
     ]]
     to_table(header, content) + "\n"
   end