Update Rails/Pluck to be aware of numblocks

[sammiya]

Fixed Rails/Pluck to catch map { _1[:foo] }.

---

Before submitting the PR make sure the following are checked:

• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
• Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.
• The PR relates to only one subject with a clear title
  and description in grammatically correct, complete sentences.
• If this is a new cop, consider making a corresponding update to the Rails Style Guide.

[koic]

I'm worried about the duplication of implementation. Can you integrate it as follows?

diff --git a/lib/rubocop/cop/rails/pluck.rb b/lib/rubocop/cop/rails/pluck.rb
index 332e5ac94..a18b46032 100644
--- a/lib/rubocop/cop/rails/pluck.rb
+++ b/lib/rubocop/cop/rails/pluck.rb
@@ -18,28 +18,35 @@ module RuboCop
       #   Post.published.pluck(:title)
       #   [{ a: :b, c: :d }].pluck(:a)
       class Pluck < Base
         extend AutoCorrector
         extend TargetRailsVersion

-        MSG = 'Prefer `pluck(:%<value>s)` over `%<method>s { |%<argument>s| %<element>s[:%<value>s] }`.'
+        MSG = 'Prefer `pluck(:%<value>s)` over `%<current>s`.'

         minimum_target_rails_version 5.0

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

         def on_block(node)
           pluck_candidate?(node) do |method, argument, element, value|
-            next unless argument == element
+            match = if node.block_type?
+                      argument.children.first.source.to_sym == element
+                    else # numblock
+                      argument == 1 && element == :_1
+                    end
+            next unless match

-            message = message(method, argument, element, value)
+            message = message(value, node)

             add_offense(offense_range(node), message: message) do |corrector|
               corrector.replace(offense_range(node), "pluck(:#{value})")
             end
           end
         end
+        alias on_numblock on_block

         private

@@ -47,8 +54,10 @@ module RuboCop
           node.send_node.loc.selector.join(node.loc.end)
         end

-        def message(method, argument, element, value)
-          format(MSG, method: method, argument: argument, element: element, value: value)
+        def message(value, node)
+          current = node.send_node.loc.selector.join(node.loc.end).source
+
+          format(MSG, value: value, current: current)
         end
       end
     end

[koic]

It's a bit redundant and can be remove.

[sammiya]

@koic Thank you for the elegant suggestion!
I was concerned about code duplication too, but couldn't think of the way to handle block and numblock in the same code.

I've incorporated your code and made the following minor tweaks, so please let me know if anything you don't like:

• stopped capturing unused method name
• called the offense_range method in the message method

diff --git a/lib/rubocop/cop/rails/pluck.rb b/lib/rubocop/cop/rails/pluck.rb
index 9218a135f..ffb42fb1f 100644
--- a/lib/rubocop/cop/rails/pluck.rb
+++ b/lib/rubocop/cop/rails/pluck.rb
@@ -26,11 +26,11 @@ module RuboCop
         minimum_target_rails_version 5.0

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

         def on_block(node)
-          pluck_candidate?(node) do |method, argument, element, value|
+          pluck_candidate?(node) do |argument, element, value|
             match = if node.block_type?
                       argument.children.first.source.to_sym == element
                     else # numblock
@@ -54,7 +54,7 @@ module RuboCop
         end

         def message(value, node)
-          current = node.send_node.loc.selector.join(node.loc.end).source
+          current = offense_range(node).source

           format(MSG, value: value, current: current)
         end

[koic]

Can you remove it because the default configuration of this cop does not change.

[sammiya]

Sure, done.

[koic]

I've incorporated your code and made the following minor tweaks

Yeah, the adjustment looks good!

[koic]

Thanks!