Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for errors in Rails/DeprecatedActiveModelErrorsMethods #739

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1 @@
* [#739](https://github.com/rubocop/rubocop-rails/pull/739): Fix a bad autocorrection for `errors.details[:name] << value` in Rails/DeprecatedActiveModelErrorsMethods. ([@BrianHawley][])
1 change: 1 addition & 0 deletions changelog/fix_deprecated_errors_missing_methods.md
@@ -0,0 +1 @@
* [#739](https://github.com/rubocop/rubocop-rails/pull/739): Rails/DeprecatedActiveModelErrorsMethods was missing the deprecated `values`, `to_h`, and `to_xml` methods. ([@BrianHawley][])
1 change: 1 addition & 0 deletions changelog/fix_deprecated_errors_nil_dereference.md
@@ -0,0 +1 @@
* [#739](https://github.com/rubocop/rubocop-rails/pull/739): Fix a NoMethodError on nil for `errors.keys` in a model in Rails/DeprecatedActiveModelErrorsMethods. ([@BrianHawley][])
30 changes: 17 additions & 13 deletions lib/rubocop/cop/rails/deprecated_active_model_errors_methods.rb
Expand Up @@ -37,7 +37,8 @@ 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
INCOMPATIBLE_METHODS = %i[keys values to_h to_xml].freeze

MANIPULATIVE_METHODS = Set[
*%i[
Expand All @@ -55,7 +56,7 @@ class DeprecatedActiveModelErrorsMethods < Base
{
#root_manipulation?
#root_assignment?
#errors_keys?
#errors_deprecated?
#messages_details_manipulation?
#messages_details_assignment?
}
Expand All @@ -77,10 +78,10 @@ class DeprecatedActiveModelErrorsMethods < Base
...)
PATTERN

def_node_matcher :errors_keys?, <<~PATTERN
def_node_matcher :errors_deprecated?, <<~PATTERN
(send
(send #receiver_matcher :errors)
:keys)
{:keys :values :to_h :to_xml})
PATTERN

def_node_matcher :messages_details_manipulation?, <<~PATTERN
Expand All @@ -106,10 +107,10 @@ class DeprecatedActiveModelErrorsMethods < Base

def on_send(node)
any_manipulation?(node) do
next if node.method?(:keys) && target_rails_version <= 6.0
next if target_rails_version <= 6.0 && INCOMPATIBLE_METHODS.include?(node.method_name)

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,26 +119,29 @@ 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

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

Expand Down