Skip to content

Commit

Permalink
[Fix rubocop#1427] Fix wrong autocorrect for Rails/UniqBeforePluck
Browse files Browse the repository at this point in the history
…with multi-line expression

This introduces a bit of whitespace when the expression spans multiple lines, but that seesm fine
  • Loading branch information
Earlopain committed Jan 24, 2025
1 parent c0f086f commit 937130f
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 33 deletions.
1 change: 1 addition & 0 deletions changelog/fix_wrong_autocorrect_rails_pluck_uniq.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1427](https://github.com/rubocop/rubocop-rails/issues/1427): Fix an error for `Rails/UniqBeforePluck` when `pluck` and `unique` are on different lines. ([@earlopain][])
44 changes: 11 additions & 33 deletions lib/rubocop/cop/rails/uniq_before_pluck.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,51 +51,29 @@ class UniqBeforePluck < Base

MSG = 'Use `distinct` before `pluck`.'
RESTRICT_ON_SEND = %i[uniq].freeze
NEWLINE = "\n"
PATTERN = '[!^block (send (send %<type>s :pluck ...) :uniq ...)]'

def_node_matcher :conservative_node_match, format(PATTERN, type: 'const')

def_node_matcher :aggressive_node_match, format(PATTERN, type: '_')
def_node_matcher :uniq_before_pluck, '[!^block $(send $(send _ :pluck ...) :uniq ...)]'

def on_send(node)
uniq = if style == :conservative
conservative_node_match(node)
else
aggressive_node_match(node)
end

return unless uniq
uniq_before_pluck(node) do |uniq_node, pluck_node|
next if style == :conservative && !pluck_node.receiver&.const_type?

add_offense(node.loc.selector) do |corrector|
autocorrect(corrector, node)
add_offense(uniq_node.loc.selector) do |corrector|
autocorrect(corrector, uniq_node, pluck_node)
end
end
end

private

def autocorrect(corrector, node)
method = node.method_name
def autocorrect(corrector, uniq_node, pluck_node)
corrector.remove(range_with_surrounding_space(uniq_node.loc.selector, newlines: false))
corrector.remove(uniq_node.loc.dot) if uniq_node.loc.dot

corrector.remove(dot_method_with_whitespace(method, node))
if (dot = node.receiver.loc.dot)
if (dot = pluck_node.loc.dot)
corrector.insert_before(dot.begin, '.distinct')
else
corrector.insert_before(node.receiver, 'distinct.')
end
end

def dot_method_with_whitespace(method, node)
range_between(dot_method_begin_pos(method, node), node.loc.selector.end_pos)
end

def dot_method_begin_pos(method, node)
lines = node.source.split(NEWLINE)

if lines.last.strip == ".#{method}"
node.source.rindex(NEWLINE)
else
node.loc.dot.begin_pos
corrector.insert_before(pluck_node, 'distinct.')
end
end
end
Expand Down
23 changes: 23 additions & 0 deletions spec/rubocop/cop/rails/uniq_before_pluck_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

expect_correction(<<~RUBY)
Model.distinct.pluck(:name)
RUBY
end

Expand All @@ -34,6 +35,22 @@

expect_correction(<<~RUBY)
Model.distinct.pluck(:name)
RUBY
end

it 'corrects when uniq and pluck are on different lines' do
expect_offense(<<~RUBY)
Model
.pluck(:name)
.uniq
^^^^ Use `distinct` before `pluck`.
RUBY

expect_correction(<<~RUBY)
Model
.distinct.pluck(:name)
RUBY
end

Expand Down Expand Up @@ -85,6 +102,12 @@
instance.assoc.pluck(:name).uniq
RUBY
end

it 'ignores uniq without an receiver' do
expect_no_offenses(<<~RUBY)
pluck(:name).uniq
RUBY
end
end

context 'when the enforced mode is aggressive' do
Expand Down

0 comments on commit 937130f

Please sign in to comment.