From 937130f16484ac44d9ea50acc26f737fdbfc9c1a Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Fri, 24 Jan 2025 13:54:33 +0100 Subject: [PATCH] [Fix #1427] Fix wrong autocorrect for `Rails/UniqBeforePluck` with multi-line expression This introduces a bit of whitespace when the expression spans multiple lines, but that seesm fine --- .../fix_wrong_autocorrect_rails_pluck_uniq.md | 1 + lib/rubocop/cop/rails/uniq_before_pluck.rb | 44 +++++-------------- .../cop/rails/uniq_before_pluck_spec.rb | 23 ++++++++++ 3 files changed, 35 insertions(+), 33 deletions(-) create mode 100644 changelog/fix_wrong_autocorrect_rails_pluck_uniq.md diff --git a/changelog/fix_wrong_autocorrect_rails_pluck_uniq.md b/changelog/fix_wrong_autocorrect_rails_pluck_uniq.md new file mode 100644 index 0000000000..7b93ca9955 --- /dev/null +++ b/changelog/fix_wrong_autocorrect_rails_pluck_uniq.md @@ -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][]) diff --git a/lib/rubocop/cop/rails/uniq_before_pluck.rb b/lib/rubocop/cop/rails/uniq_before_pluck.rb index d11c8569e7..498b8dacd0 100644 --- a/lib/rubocop/cop/rails/uniq_before_pluck.rb +++ b/lib/rubocop/cop/rails/uniq_before_pluck.rb @@ -51,51 +51,29 @@ class UniqBeforePluck < Base MSG = 'Use `distinct` before `pluck`.' RESTRICT_ON_SEND = %i[uniq].freeze - NEWLINE = "\n" - PATTERN = '[!^block (send (send %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 diff --git a/spec/rubocop/cop/rails/uniq_before_pluck_spec.rb b/spec/rubocop/cop/rails/uniq_before_pluck_spec.rb index 7ba2a9616c..031b7b2b5e 100644 --- a/spec/rubocop/cop/rails/uniq_before_pluck_spec.rb +++ b/spec/rubocop/cop/rails/uniq_before_pluck_spec.rb @@ -22,6 +22,7 @@ expect_correction(<<~RUBY) Model.distinct.pluck(:name) + RUBY end @@ -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 @@ -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