Skip to content

Commit

Permalink
No correction for errors.details[:n] << v
Browse files Browse the repository at this point in the history
Fixes for Rails/DeprecatedActiveModelErrorsMethods:
- Fixed a bad autocorrection of `errors.details[:name] << value`.
  There isn't really a correct replacement for this one.
- Did some refactors prompted by rubocop complaints.
- Fixed a misspelling of autocorrectable.
- Added missing correction assertions to test cases.
  • Loading branch information
BrianHawley committed Jul 6, 2022
1 parent 990a786 commit cf2c8a5
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#741](https://github.com/rubocop/rubocop-rails/pull/741): Fix a bad autocorrection for `errors.details[:name] << value` in Rails/DeprecatedActiveModelErrorsMethods. ([@BrianHawley][])
11 changes: 9 additions & 2 deletions lib/rubocop/cop/rails/deprecated_active_model_errors_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class DeprecatedActiveModelErrorsMethods < Base
extend AutoCorrector

MSG = 'Avoid manipulating ActiveModel errors as hash directly.'
AUTOCORECTABLE_METHODS = %i[<< clear keys].freeze
AUTOCORRECTABLE_METHODS = %i[<< clear keys].freeze

MANIPULATIVE_METHODS = Set[
*%i[
Expand Down Expand Up @@ -109,7 +109,7 @@ def on_send(node)
next if node.method?(:keys) && target_rails_version <= 6.0

add_offense(node) do |corrector|
next unless AUTOCORECTABLE_METHODS.include?(node.method_name)
next if skip_autocorrect?(node)

autocorrect(corrector, node)
end
Expand All @@ -118,6 +118,13 @@ def on_send(node)

private

def skip_autocorrect?(node)
receiver = node.receiver.receiver
!AUTOCORRECTABLE_METHODS.include?(node.method_name) || (
receiver&.send_type? && receiver&.method?(:details) && node.method?(:<<)
)
end

def autocorrect(corrector, node)
receiver = node.receiver

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@
user.errors.details[:name] << {}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid manipulating ActiveModel errors as hash directly.
RUBY

expect_no_corrections
end

context 'when assigning' do
Expand All @@ -131,6 +133,14 @@ def expect_offense_if_model_file(code, file_path)
end
end

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 modifying errors' do
it 'registers an offense for model file' do
expect_offense_if_model_file(<<~RUBY, file_path)
Expand Down Expand Up @@ -189,6 +199,8 @@ 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

context 'when assigning' do
Expand Down

0 comments on commit cf2c8a5

Please sign in to comment.