Skip to content

Commit

Permalink
[Fix rubocop#603] Auto-correct with multiple validation attributes
Browse files Browse the repository at this point in the history
  • Loading branch information
pirj committed Dec 31, 2021
1 parent 636e58c commit 75583d4
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#609](https://github.com/rubocop/rubocop-rails/pull/609): Fix autocorrection of multiple attributes for `Rails/RedundantPresenceValidationOnBelongsTo` cop. ([@pirj][])
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class RedundantPresenceValidationOnBelongsTo < Base
# @example source that matches - by a foreign key
# validates :user_id, presence: true
def_node_matcher :presence_validation?, <<~PATTERN
$(
(
send nil? :validates
(sym $_)+
$(hash <$(pair (sym :presence) {true hash}) ...>)
Expand Down Expand Up @@ -152,27 +152,44 @@ class RedundantPresenceValidationOnBelongsTo < Base
PATTERN

def on_send(node)
validation, all_keys, options, presence = presence_validation?(node)
return unless validation
presence_validation?(node) do |all_keys, options, presence|
keys = non_optional_belongs_to(node.parent, all_keys)
return if keys.none?

keys = all_keys.select do |key|
belongs_to = belongs_to_for(node.parent, key)
belongs_to && !optional?(belongs_to)
add_offense_and_correct(node, all_keys, keys, options, presence)
end
return if keys.none?
end

private

def add_offense_and_correct(node, all_keys, keys, options, presence)
add_offense(presence, message: message_for(keys)) do |corrector|
remove_presence_validation(corrector, node, options, presence)
if options.children.one? # `presence: true` is the only option
if keys == all_keys
remove_validation(corrector, node)
else
remove_keys_from_validation(corrector, node, keys)
end
elsif keys == all_keys
remove_presence_option(corrector, presence)
else
extract_validation_for_keys(corrector, node, keys, options)
end
end
end

private

def message_for(keys)
display_keys = keys.map { |key| "`#{key}`" }.join('/')
format(MSG, association: display_keys)
end

def non_optional_belongs_to(node, keys)
keys.select do |key|
belongs_to = belongs_to_for(node, key)
belongs_to && !optional?(belongs_to)
end
end

def belongs_to_for(model_class_node, key)
if key.to_s.end_with?('_id')
normalized_key = key.to_s.delete_suffix('_id').to_sym
Expand All @@ -182,17 +199,48 @@ def belongs_to_for(model_class_node, key)
end
end

def remove_presence_validation(corrector, node, options, presence)
if options.children.one?
corrector.remove(range_by_whole_lines(node.source_range, include_final_newline: true))
else
range = range_with_surrounding_comma(
range_with_surrounding_space(range: presence.source_range, side: :left),
:left
def remove_validation(corrector, node)
corrector.remove(validation_range(node))
end

def remove_keys_from_validation(corrector, node, keys)
keys.each do |key|
key_node = node.arguments.find { |arg| arg.value == key }
key_range = range_with_surrounding_space(
range: range_with_surrounding_comma(key_node.source_range, :right),
side: :right
)
corrector.remove(range)
corrector.remove(key_range)
end
end

def remove_presence_option(corrector, presence)
range = range_with_surrounding_comma(
range_with_surrounding_space(range: presence.source_range, side: :left),
:left
)
corrector.remove(range)
end

def extract_validation_for_keys(corrector, node, keys, options)
indentation = ' ' * node.source_range.column
options_without_presence = options.children.reject { |pair| pair.key.value == :presence }
source = [
indentation,
'validates ',
keys.map(&:inspect).join(', '),
', ',
options_without_presence.map(&:source).join(', '),
"\n"
].join

remove_keys_from_validation(corrector, node, keys)
corrector.insert_after(validation_range(node), source)
end

def validation_range(node)
range_by_whole_lines(node.source_range, include_final_newline: true)
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,64 @@
RUBY
end

it 'registers an offense for multiple attributes when not all are associations' do
expect_offense(<<~RUBY)
belongs_to :user
validates :user, :name, presence: true
^^^^^^^^^^^^^^ Remove explicit presence validation for `user`.
RUBY

expect_correction(<<~RUBY)
belongs_to :user
validates :name, presence: true
RUBY
end

it 'registers an offense for a secondary attribute' do
expect_offense(<<~RUBY)
belongs_to :user
validates :name, :user, presence: true
^^^^^^^^^^^^^^ Remove explicit presence validation for `user`.
RUBY

expect_correction(<<~RUBY)
belongs_to :user
validates :name, presence: true
RUBY
end

it 'registers an offense for multiple attributes and options' do
expect_offense(<<~RUBY)
belongs_to :user
validates :user, :name, presence: true, uniqueness: true
^^^^^^^^^^^^^^ Remove explicit presence validation for `user`.
RUBY

expect_correction(<<~RUBY)
belongs_to :user
validates :name, presence: true, uniqueness: true
validates :user, uniqueness: true
RUBY
end

it 'preserves indentation for the extracted validation line' do
expect_offense(<<~RUBY)
class Profile
belongs_to :user
validates :user, :name, presence: true, uniqueness: true
^^^^^^^^^^^^^^ Remove explicit presence validation for `user`.
end
RUBY

expect_correction(<<~RUBY)
class Profile
belongs_to :user
validates :name, presence: true, uniqueness: true
validates :user, uniqueness: true
end
RUBY
end

it 'registers an offense for presence with a message' do
expect_offense(<<~RUBY)
belongs_to :user
Expand Down

0 comments on commit 75583d4

Please sign in to comment.