Fix a NoMethodError for errors.keys in a model

Fixes for Rails/DeprecatedActiveModelErrorsMethods:
- Fixed a NoMethodError on nil for `errors.keys` in a model.
- Made the range calculation more exact so the replacement is easier.
- Added missing correction assertions to most test cases.
- Added missing test cases.
- Improved test structure for future changes.

Author: BrianHawley

changelog/fix_deprecated_errors_nil_dereference.md

@@ -0,0 +1 @@
+* [#740](https://github.com/rubocop/rubocop-rails/pull/740): Fix a NoMethodError on nil for `errors.keys` in a model in Rails/DeprecatedActiveModelErrorsMethods. ([@BrianHawley][])

lib/rubocop/cop/rails/deprecated_active_model_errors_methods.rb

@@ -121,23 +121,19 @@ def on_send(node)
         def autocorrect(corrector, node)
           receiver = node.receiver
 
-          if receiver.receiver.send_type? && receiver.receiver.method?(:messages)
-            corrector.remove(receiver.receiver.loc.dot)
-            corrector.remove(receiver.receiver.loc.selector)
-          end
-
           range = offense_range(node, receiver)
           replacement = replacement(node, receiver)
 
           corrector.replace(range, replacement)
         end
 
         def offense_range(node, receiver)
-          range_between(receiver.receiver.source_range.end_pos, node.source_range.end_pos)
+          receiver = receiver.receiver while receiver.send_type? && !receiver.method?(:errors) && receiver.receiver
+          range_between(receiver.source_range.end_pos, node.source_range.end_pos)
         end
 
         def replacement(node, receiver)
-          return '.errors.attribute_names' if node.method?(:keys)
+          return '.attribute_names' if node.method?(:keys)
 
           key = receiver.first_argument.source
 

spec/rubocop/cop/rails/deprecated_active_model_errors_methods_spec.rb

@@ -2,7 +2,7 @@
 
 RSpec.describe RuboCop::Cop::Rails::DeprecatedActiveModelErrorsMethods, :config do
   shared_examples 'errors call with explicit receiver' do
-    context 'when modifying errors' do
+    context 'when accessing errors' do
       it 'registers and corrects an offense' do
         expect_offense(<<~RUBY, file_path)
           user.errors[:name] << 'msg'
@@ -20,6 +20,8 @@
             user.errors[:name] = []
             ^^^^^^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
           RUBY
+
+          expect_no_corrections
         end
       end
 
@@ -36,8 +38,8 @@
         end
       end
 
-      context 'when using `keys` method' do
-        context 'Rails >= 6.1', :rails61 do
+      context 'Rails >= 6.1', :rails61 do
+        context 'when using `keys` method' do
           it 'registers and corrects an offense when root receiver is a variable' do
             expect_offense(<<~RUBY, file_path)
               user = create_user
@@ -62,8 +64,10 @@
             RUBY
           end
         end
+      end
 
-        context 'Rails <= 6.0', :rails60 do
+      context 'Rails <= 6.0', :rails60 do
+        context 'when using `keys` method' do
           it 'does not register an offense when root receiver is a variable' do
             expect_no_offenses(<<~RUBY, file_path)
               user = create_user
@@ -98,6 +102,29 @@
             user.errors.messages[:name] = []
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
           RUBY
+
+          expect_no_corrections
+        end
+      end
+
+      context 'when using `clear` method' do
+        it 'registers and corrects an offense' do
+          expect_offense(<<~RUBY, file_path)
+            user.errors.messages[:name].clear
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
+          RUBY
+
+          expect_correction(<<~RUBY)
+            user.errors.delete(:name)
+          RUBY
+        end
+      end
+
+      context 'when calling non-manipulative methods' do
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY, file_path)
+            errors.messages[:name].present?
+          RUBY
         end
       end
     end
@@ -116,6 +143,29 @@
             user.errors.details[:name] = []
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
           RUBY
+
+          expect_no_corrections
+        end
+      end
+
+      context 'when using `clear` method' do
+        it 'registers and corrects an offense' do
+          expect_offense(<<~RUBY, file_path)
+            user.errors.details[:name].clear
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
+          RUBY
+
+          expect_correction(<<~RUBY)
+            user.errors.delete(:name)
+          RUBY
+        end
+      end
+
+      context 'when calling non-manipulative methods' do
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY, file_path)
+            errors.details[:name].present?
+          RUBY
         end
       end
     end
@@ -131,12 +181,24 @@ def expect_offense_if_model_file(code, file_path)
       end
     end
 
-    context 'when modifying errors' do
+    def expect_correction_if_model_file(code, file_path)
+      expect_correction(code) if file_path.include?('/models/')
+    end
+
+    def expect_no_corrections_if_model_file(file_path)
+      expect_no_corrections if file_path.include?('/models/')
+    end
+
+    context 'when accessing errors' do
       it 'registers an offense for model file' do
         expect_offense_if_model_file(<<~RUBY, file_path)
           errors[:name] << 'msg'
           ^^^^^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
         RUBY
+
+        expect_correction_if_model_file(<<~RUBY, file_path)
+          errors.add(:name, 'msg')
+        RUBY
       end
 
       context 'when assigning' do
@@ -145,6 +207,46 @@ def expect_offense_if_model_file(code, file_path)
             errors[:name] = []
             ^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
           RUBY
+
+          expect_no_corrections_if_model_file(file_path)
+        end
+      end
+
+      context 'when using `clear` method' do
+        it 'registers and corrects an offense' do
+          expect_offense_if_model_file(<<~RUBY, file_path)
+            errors[:name].clear
+            ^^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
+          RUBY
+
+          expect_correction_if_model_file(<<~RUBY, file_path)
+            errors.delete(:name)
+          RUBY
+        end
+      end
+
+      context 'Rails >= 6.1', :rails61 do
+        context 'when using `keys` method' do
+          it 'registers and corrects an offense when root receiver is a variable' do
+            expect_offense_if_model_file(<<~RUBY, file_path)
+              errors.keys
+              ^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
+            RUBY
+
+            expect_correction_if_model_file(<<~RUBY, file_path)
+              errors.attribute_names
+            RUBY
+          end
+        end
+      end
+
+      context 'Rails <= 6.0', :rails60 do
+        context 'when using `keys` method' do
+          it 'does not register an offense' do
+            expect_no_offenses(<<~RUBY, file_path)
+              errors.keys
+            RUBY
+          end
         end
       end
 
@@ -163,6 +265,10 @@ def expect_offense_if_model_file(code, file_path)
           errors.messages[:name] << 'msg'
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
         RUBY
+
+        expect_correction_if_model_file(<<~RUBY, file_path)
+          errors.add(:name, 'msg')
+        RUBY
       end
 
       context 'when assigning' do
@@ -171,6 +277,29 @@ def expect_offense_if_model_file(code, file_path)
             errors.messages[:name] = []
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
           RUBY
+
+          expect_no_corrections_if_model_file(file_path)
+        end
+      end
+
+      context 'when using `clear` method' do
+        it 'registers and corrects an offense' do
+          expect_offense_if_model_file(<<~RUBY, file_path)
+            errors.messages[:name].clear
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
+          RUBY
+
+          expect_correction_if_model_file(<<~RUBY, file_path)
+            errors.delete(:name)
+          RUBY
+        end
+      end
+
+      context 'when using `keys` method' do
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY, file_path)
+            errors.messages[:name].keys
+          RUBY
         end
       end
 
@@ -197,6 +326,29 @@ def expect_offense_if_model_file(code, file_path)
             errors.details[:name] = []
             ^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
           RUBY
+
+          expect_no_corrections_if_model_file(file_path)
+        end
+      end
+
+      context 'when using `clear` method' do
+        it 'registers and corrects an offense' do
+          expect_offense_if_model_file(<<~RUBY, file_path)
+            errors.details[:name].clear
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
+          RUBY
+
+          expect_correction_if_model_file(<<~RUBY, file_path)
+            errors.delete(:name)
+          RUBY
+        end
+      end
+
+      context 'when using `keys` method' do
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY, file_path)
+            errors.details[:name].keys
+          RUBY
         end
       end