diff --git a/changelog/fix_deprecated_errors_nil_dereference.md b/changelog/fix_deprecated_errors_nil_dereference.md new file mode 100644 index 0000000000..a9769874af --- /dev/null +++ b/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][]) diff --git a/lib/rubocop/cop/rails/deprecated_active_model_errors_methods.rb b/lib/rubocop/cop/rails/deprecated_active_model_errors_methods.rb index e2c9d79e93..f115174158 100644 --- a/lib/rubocop/cop/rails/deprecated_active_model_errors_methods.rb +++ b/lib/rubocop/cop/rails/deprecated_active_model_errors_methods.rb @@ -121,11 +121,6 @@ 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) @@ -133,11 +128,12 @@ def autocorrect(corrector, node) 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 diff --git a/spec/rubocop/cop/rails/deprecated_active_model_errors_methods_spec.rb b/spec/rubocop/cop/rails/deprecated_active_model_errors_methods_spec.rb index 41e1219594..1ccd7f1e31 100644 --- a/spec/rubocop/cop/rails/deprecated_active_model_errors_methods_spec.rb +++ b/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