Skip to content

Commit 1e83c14

Browse files
committed
[Fix rubocop#787] Make Rails/Pluck aware of all keys
Fixes rubocop#787. This PR makes `Rails/Pluck` aware of all keys.
1 parent 57c2fd4 commit 1e83c14

File tree

3 files changed

+34
-14
lines changed

3 files changed

+34
-14
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#787](https://github.com/rubocop/rubocop-rails/issues/787): Make `Rails/Pluck` aware of all keys. ([@koic][])

lib/rubocop/cop/rails/pluck.rb

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,27 +21,28 @@ class Pluck < Base
2121
extend AutoCorrector
2222
extend TargetRailsVersion
2323

24-
MSG = 'Prefer `pluck(:%<value>s)` over `%<current>s`.'
24+
MSG = 'Prefer `%<replacement>s` over `%<current>s`.'
2525

2626
minimum_target_rails_version 5.0
2727

2828
def_node_matcher :pluck_candidate?, <<~PATTERN
29-
({block numblock} (send _ {:map :collect}) $_argument (send (lvar $_element) :[] (sym $_value)))
29+
({block numblock} (send _ {:map :collect}) $_argument (send (lvar $_element) :[] $_key))
3030
PATTERN
3131

3232
def on_block(node)
33-
pluck_candidate?(node) do |argument, element, value|
33+
pluck_candidate?(node) do |argument, element, key|
3434
match = if node.block_type?
3535
argument.children.first.source.to_sym == element
3636
else # numblock
3737
argument == 1 && element == :_1
3838
end
3939
next unless match
4040

41-
message = message(value, node)
41+
replacement = "pluck(#{key.source})"
42+
message = message(replacement, node)
4243

4344
add_offense(offense_range(node), message: message) do |corrector|
44-
corrector.replace(offense_range(node), "pluck(:#{value})")
45+
corrector.replace(offense_range(node), replacement)
4546
end
4647
end
4748
end
@@ -53,10 +54,10 @@ def offense_range(node)
5354
node.send_node.loc.selector.join(node.loc.end)
5455
end
5556

56-
def message(value, node)
57+
def message(replacement, node)
5758
current = offense_range(node).source
5859

59-
format(MSG, value: value, current: current)
60+
format(MSG, replacement: replacement, current: current)
6061
end
6162
end
6263
end

spec/rubocop/cop/rails/pluck_spec.rb

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
RSpec.describe RuboCop::Cop::Rails::Pluck, :config do
44
%w[map collect].each do |method|
55
context 'when using Rails 5.0 or newer', :rails50 do
6-
context "when `#{method}` can be replaced with `pluck`" do
6+
context "when `#{method}` with symbol literal key can be replaced with `pluck`" do
77
it 'registers an offense' do
88
expect_offense(<<~RUBY, method: method)
99
x.%{method} { |a| a[:foo] }
@@ -16,18 +16,36 @@
1616
end
1717
end
1818

19-
context 'when the block argument is unused' do
20-
it 'does not register an offense' do
21-
expect_no_offenses(<<~RUBY)
22-
x.#{method} { |a| b[:foo] }
19+
context "when `#{method}` with string literal key can be replaced with `pluck`" do
20+
it 'registers an offense' do
21+
expect_offense(<<~RUBY, method: method)
22+
x.%{method} { |a| a['foo'] }
23+
^{method}^^^^^^^^^^^^^^^^^ Prefer `pluck('foo')` over `%{method} { |a| a['foo'] }`.
24+
RUBY
25+
26+
expect_correction(<<~RUBY)
27+
x.pluck('foo')
28+
RUBY
29+
end
30+
end
31+
32+
context "when `#{method}` with method call key can be replaced with `pluck`" do
33+
it 'registers an offense' do
34+
expect_offense(<<~RUBY, method: method)
35+
x.%{method} { |a| a[obj.do_something] }
36+
^{method}^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `pluck(obj.do_something)` over `%{method} { |a| a[obj.do_something] }`.
37+
RUBY
38+
39+
expect_correction(<<~RUBY)
40+
x.pluck(obj.do_something)
2341
RUBY
2442
end
2543
end
2644

27-
context 'when the value is not a symbol' do
45+
context 'when the block argument is unused' do
2846
it 'does not register an offense' do
2947
expect_no_offenses(<<~RUBY)
30-
x.#{method} { |a| a['foo'] }
48+
x.#{method} { |a| b[:foo] }
3149
RUBY
3250
end
3351
end

0 commit comments

Comments
 (0)