Skip to content

Commit

Permalink
Fix a false positive for Rails/Pluck
Browse files Browse the repository at this point in the history
This PR fixes a false positive for `Rails/Pluck`
when using block argument in `[]` to prevent the following
incompatible autocorrect.

```diff
-x.collect { |a| a[foo...a.to_someghing] }
+x.pluck(foo...a.to_someghing)
```

The `a` is not defined in the autocorrected code.
  • Loading branch information
koic committed Oct 25, 2022
1 parent 1acb097 commit abbf838
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 4 deletions.
1 change: 1 addition & 0 deletions changelog/fix_a_false_positive_for_rails_pluck.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#831](https://github.com/rubocop/rubocop-rails/pull/831): Fix a false positive for `Rails/Pluck` when using block argument in `[]`. ([@koic][])
13 changes: 9 additions & 4 deletions lib/rubocop/cop/rails/pluck.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,16 @@ class Pluck < Base
minimum_target_rails_version 5.0

def_node_matcher :pluck_candidate?, <<~PATTERN
({block numblock} (send _ {:map :collect}) $_argument (send (lvar $_element) :[] $_key))
({block numblock} (send _ {:map :collect}) $_argument (send lvar :[] $_key))
PATTERN

def on_block(node)
pluck_candidate?(node) do |argument, element, key|
pluck_candidate?(node) do |argument, key|
match = if node.block_type?
argument.children.first.source.to_sym == element
block_argument = argument.children.first.source
use_block_argument_in_key?(block_argument, key)
else # numblock
argument == 1 && element == :_1
argument == 1 && use_block_argument_in_key?('_1', key)
end
next unless match

Expand All @@ -50,6 +51,10 @@ def on_block(node)

private

def use_block_argument_in_key?(block_argument, key)
key.each_descendant(:lvar).none? { |lvar| block_argument == lvar.source }
end

def offense_range(node)
node.send_node.loc.selector.join(node.loc.end)
end
Expand Down
16 changes: 16 additions & 0 deletions spec/rubocop/cop/rails/pluck_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@
end
end

context 'when the block argument is used in `[]`' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
x.#{method} { |a| a[foo...a.to_someghing] }
RUBY
end
end

context 'when using Ruby 2.7 or newer', :ruby27 do
context 'when using numbered parameter' do
context "when `#{method}` can be replaced with `pluck`" do
Expand All @@ -64,6 +72,14 @@
RUBY
end
end

context 'when the numblock argument is used in `[]`' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
x.#{method} { _1[foo..._1.to_someghing] }
RUBY
end
end
end
end
end
Expand Down

0 comments on commit abbf838

Please sign in to comment.