Support numbered parameter for RedundantSortBlock, SortReverse, a…

…nd `TimesMap`

This PR support numbered parameter for `Performance/RedundantSortBlock`,
`Performance/SortReverse`, and `Performance/TimesMap` cops.

changelog/new_support_numbered_parameter.md

@@ -0,0 +1 @@
+* [#305](https://github.com/rubocop/rubocop-performance/pull/305): Support numbered parameter for `Performance/RedundantSortBlock`, `Performance/SortReverse`, and `Performance/TimesMap` cops. ([@koic][])

lib/rubocop/cop/mixin/sort_block.rb

@@ -14,6 +14,13 @@ module SortBlock
           $send)
       PATTERN
 
+      def_node_matcher :sort_with_numblock?, <<~PATTERN
+        (numblock
+          $(send _ :sort)
+          $_arg_count
+          $send)
+      PATTERN
+
       def_node_matcher :replaceable_body?, <<~PATTERN
         (send (lvar %1) :<=> (lvar %2))
       PATTERN

lib/rubocop/cop/performance/redundant_sort_block.rb

@@ -16,25 +16,33 @@ class RedundantSortBlock < Base
         include SortBlock
         extend AutoCorrector
 
-        MSG = 'Use `sort` instead of `%<bad_method>s`.'
+        MSG = 'Use `sort` without block.'
 
         def on_block(node)
           return unless (send, var_a, var_b, body = sort_with_block?(node))
 
           replaceable_body?(body, var_a, var_b) do
-            range = sort_range(send, node)
+            register_offense(send, node)
+          end
+        end
+
+        def on_numblock(node)
+          return unless (send, arg_count, body = sort_with_numblock?(node))
+          return unless arg_count == 2
 
-            add_offense(range, message: message(var_a, var_b)) do |corrector|
-              corrector.replace(range, 'sort')
-            end
+          replaceable_body?(body, :_1, :_2) do
+            register_offense(send, node)
           end
         end
 
         private
 
-        def message(var_a, var_b)
-          bad_method = "sort { |#{var_a}, #{var_b}| #{var_a} <=> #{var_b} }"
-          format(MSG, bad_method: bad_method)
+        def register_offense(send, node)
+          range = sort_range(send, node)
+
+          add_offense(range) do |corrector|
+            corrector.replace(range, 'sort')
+          end
         end
       end
     end

lib/rubocop/cop/performance/sort_reverse.rb

@@ -17,27 +17,36 @@ class SortReverse < Base
         include SortBlock
         extend AutoCorrector
 
-        MSG = 'Use `sort.reverse` instead of `%<bad_method>s`.'
+        MSG = 'Use `sort.reverse` instead.'
 
         def on_block(node)
           sort_with_block?(node) do |send, var_a, var_b, body|
             replaceable_body?(body, var_b, var_a) do
-              range = sort_range(send, node)
+              register_offense(send, node)
+            end
+          end
+        end
 
-              add_offense(range, message: message(var_a, var_b)) do |corrector|
-                replacement = 'sort.reverse'
+        def on_numblock(node)
+          sort_with_numblock?(node) do |send, arg_count, body|
+            next unless arg_count == 2
 
-                corrector.replace(range, replacement)
-              end
+            replaceable_body?(body, :_2, :_1) do
+              register_offense(send, node)
             end
           end
         end
 
         private
 
-        def message(var_a, var_b)
-          bad_method = "sort { |#{var_a}, #{var_b}| #{var_b} <=> #{var_a} }"
-          format(MSG, bad_method: bad_method)
+        def register_offense(send, node)
+          range = sort_range(send, node)
+
+          add_offense(range) do |corrector|
+            replacement = 'sort.reverse'
+
+            corrector.replace(range, replacement)
+          end
         end
       end
     end

lib/rubocop/cop/performance/times_map.rb

@@ -43,6 +43,7 @@ def on_send(node)
         def on_block(node)
           check(node)
         end
+        alias on_numblock on_block
 
         private
 
@@ -66,7 +67,7 @@ def message(map_or_collect, count)
         end
 
         def_node_matcher :times_map_call, <<~PATTERN
-          {(block $(send (send $!nil? :times) {:map :collect}) ...)
+          {({block numblock} $(send (send $!nil? :times) {:map :collect}) ...)
            $(send (send $!nil? :times) {:map :collect} (block_pass ...))}
         PATTERN
       end

spec/rubocop/cop/performance/redundant_sort_block_spec.rb

@@ -4,7 +4,7 @@
   it 'registers an offense and corrects when sorting in direct order' do
     expect_offense(<<~RUBY)
       array.sort { |a, b| a <=> b }
-            ^^^^^^^^^^^^^^^^^^^^^^^ Use `sort` instead of `sort { |a, b| a <=> b }`.
+            ^^^^^^^^^^^^^^^^^^^^^^^ Use `sort` without block.
     RUBY
 
     expect_correction(<<~RUBY)
@@ -29,4 +29,29 @@
       array.sort
     RUBY
   end
+
+  context 'when using numbered parameter', :ruby27 do
+    it 'registers an offense and corrects when sorting in direct order' do
+      expect_offense(<<~RUBY)
+        array.sort { _1 <=> _2 }
+              ^^^^^^^^^^^^^^^^^^ Use `sort` without block.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        array.sort
+      RUBY
+    end
+
+    it 'does not register an offense when sorting in reverse order' do
+      expect_no_offenses(<<~RUBY)
+        array.sort { _2 <=> _1 }
+      RUBY
+    end
+
+    it 'does not register an offense when sorting in direct order by some property' do
+      expect_no_offenses(<<~RUBY)
+        array.sort { _1.x <=> _2.x }
+      RUBY
+    end
+  end
 end

spec/rubocop/cop/performance/sort_reverse_spec.rb

@@ -9,14 +9,39 @@
   it 'registers an offense and corrects when sorting in reverse order' do
     expect_offense(<<~RUBY)
       array.sort { |a, b| b <=> a }
-            ^^^^^^^^^^^^^^^^^^^^^^^ Use `sort.reverse` instead of `sort { |a, b| b <=> a }`.
+            ^^^^^^^^^^^^^^^^^^^^^^^ Use `sort.reverse` instead.
     RUBY
 
     expect_correction(<<~RUBY)
       array.sort.reverse
     RUBY
   end
 
+  context 'when using numbered parameter', :ruby27 do
+    it 'registers an offense and corrects when sorting in reverse order' do
+      expect_offense(<<~RUBY)
+        array.sort { _2 <=> _1 }
+              ^^^^^^^^^^^^^^^^^^ Use `sort.reverse` instead.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        array.sort.reverse
+      RUBY
+    end
+
+    it 'does not register an offense when sorting in direct order' do
+      expect_no_offenses(<<~RUBY)
+        array.sort { _1 <=> _2 }
+      RUBY
+    end
+
+    it 'does not register an offense when sorting in reverse order by some property' do
+      expect_no_offenses(<<~RUBY)
+        array.sort { _2.x <=> _1.x }
+      RUBY
+    end
+  end
+
   it 'does not register an offense when sorting in direct order' do
     expect_no_offenses(<<~RUBY)
       array.sort { |a, b| a <=> b }

spec/rubocop/cop/performance/times_map_spec.rb

@@ -57,6 +57,19 @@
           RUBY
         end
       end
+
+      context 'when using numbered parameter', :ruby27 do
+        it 'registers an offense and corrects' do
+          expect_offense(<<~RUBY, method: method)
+            4.times.#{method} { _1.to_s }
+            ^^^^^^^^^{method}^^^^^^^^^^^^ Use `Array.new(4)` with a block instead of `.times.#{method}`.
+          RUBY
+
+          expect_correction(<<~RUBY)
+            Array.new(4) { _1.to_s }
+          RUBY
+        end
+      end
     end
   end