Merge pull request #135 from fatkodima/redundant_sort_block-cop

Add new `Performance/RedundantSortBlock` cop

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### New features
 
+* [#132](https://github.com/rubocop-hq/rubocop-performance/issues/132): Add new `Performance/RedundantSortBlock` cop. ([@fatkodima][])
 * [#125](https://github.com/rubocop-hq/rubocop-performance/pull/125): Support `Array()` and `Hash()` methods for `Performance/Size` cop. ([@fatkodima][])
 * [#124](https://github.com/rubocop-hq/rubocop-performance/pull/124): Add new `Performance/Squeeze` cop. ([@fatkodima][])
 * [#129](https://github.com/rubocop-hq/rubocop-performance/pull/129): Add new `Performance/BigDecimalWithNumericArgument` cop. ([@fatkodima][])

config/default.yml

@@ -177,6 +177,11 @@ Performance/RedundantMerge:
   # Max number of key-value pairs to consider an offense
   MaxKeyValuePairs: 2
 
+Performance/RedundantSortBlock:
+  Description: 'Use `sort` instead of `sort { |a, b| a <=> b }`.'
+  Enabled: true
+  VersionAdded: '1.7'
+
 Performance/RegexpMatch:
   Description: >-
                   Use `match?` instead of `Regexp#match`, `String#match`, `Symbol#match`,

docs/modules/ROOT/pages/cops.adoc

@@ -24,6 +24,7 @@
 * xref:cops_performance.adoc#performanceredundantblockcall[Performance/RedundantBlockCall]
 * xref:cops_performance.adoc#performanceredundantmatch[Performance/RedundantMatch]
 * xref:cops_performance.adoc#performanceredundantmerge[Performance/RedundantMerge]
+* xref:cops_performance.adoc#performanceredundantsortblock[Performance/RedundantSortBlock]
 * xref:cops_performance.adoc#performanceregexpmatch[Performance/RegexpMatch]
 * xref:cops_performance.adoc#performancereverseeach[Performance/ReverseEach]
 * xref:cops_performance.adoc#performancesize[Performance/Size]

docs/modules/ROOT/pages/cops_performance.adoc

@@ -1035,6 +1035,32 @@ hash[:b] = 2
 
 * https://github.com/JuanitoFatas/fast-ruby#hashmerge-vs-hash-code
 
+== Performance/RedundantSortBlock
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Enabled
+| Yes
+| Yes
+| 1.7
+| -
+|===
+
+This cop identifies places where `sort { |a, b| a <=> b }`
+can be replaced with `sort`.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+array.sort { |a, b| a <=> b }
+
+# good
+array.sort
+----
+
 == Performance/RegexpMatch
 
 |===

lib/rubocop/cop/mixin/sort_block.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    # Common functionality for cops checking `Enumerable#sort` blocks.
+    module SortBlock
+      extend NodePattern::Macros
+      include RangeHelp
+
+      def_node_matcher :sort_with_block?, <<~PATTERN
+        (block
+          $(send _ :sort)
+          (args (arg $_a) (arg $_b))
+          $send)
+      PATTERN
+
+      def_node_matcher :replaceable_body?, <<~PATTERN
+        (send (lvar %1) :<=> (lvar %2))
+      PATTERN
+
+      private
+
+      def sort_range(send, node)
+        range_between(send.loc.selector.begin_pos, node.loc.end.end_pos)
+      end
+    end
+  end
+end

lib/rubocop/cop/performance/redundant_sort_block.rb

@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Performance
+      # This cop identifies places where `sort { |a, b| a <=> b }`
+      # can be replaced with `sort`.
+      #
+      # @example
+      #   # bad
+      #   array.sort { |a, b| a <=> b }
+      #
+      #   # good
+      #   array.sort
+      #
+      class RedundantSortBlock < Cop
+        include SortBlock
+
+        MSG = 'Use `sort` instead of `%<bad_method>s`.'
+
+        def on_block(node)
+          sort_with_block?(node) do |send, var_a, var_b, body|
+            replaceable_body?(body, var_a, var_b) do
+              range = sort_range(send, node)
+
+              add_offense(
+                node,
+                location: range,
+                message: message(var_a, var_b)
+              )
+            end
+          end
+        end
+
+        def autocorrect(node)
+          sort_with_block?(node) do |send, _var_a, _var_b, _body|
+            lambda do |corrector|
+              range = sort_range(send, node)
+              corrector.replace(range, 'sort')
+            end
+          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)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/performance/sort_reverse.rb

@@ -14,21 +14,10 @@ module Performance
       #   array.sort.reverse
       #
       class SortReverse < Cop
-        include RangeHelp
+        include SortBlock
 
         MSG = 'Use `sort.reverse` instead of `%<bad_method>s`.'
 
-        def_node_matcher :sort_with_block?, <<~PATTERN
-          (block
-            $(send _ :sort)
-            (args (arg $_a) (arg $_b))
-            $send)
-        PATTERN
-
-        def_node_matcher :replaceable_body?, <<~PATTERN
-          (send (lvar %1) :<=> (lvar %2))
-        PATTERN
-
         def on_block(node)
           sort_with_block?(node) do |send, var_a, var_b, body|
             replaceable_body?(body, var_b, var_a) do
@@ -55,10 +44,6 @@ def autocorrect(node)
 
         private
 
-        def sort_range(send, node)
-          range_between(send.loc.selector.begin_pos, node.loc.end.end_pos)
-        end
-
         def message(var_a, var_b)
           bad_method = "sort { |#{var_a}, #{var_b}| #{var_b} <=> #{var_a} }"
           format(MSG, bad_method: bad_method)

lib/rubocop/cop/performance_cops.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require_relative 'mixin/regexp_metacharacter'
+require_relative 'mixin/sort_block'
 
 require_relative 'performance/ancestors_include'
 require_relative 'performance/big_decimal_with_numeric_argument'
@@ -23,6 +24,7 @@
 require_relative 'performance/redundant_block_call'
 require_relative 'performance/redundant_match'
 require_relative 'performance/redundant_merge'
+require_relative 'performance/redundant_sort_block'
 require_relative 'performance/regexp_match'
 require_relative 'performance/reverse_each'
 require_relative 'performance/size'

spec/rubocop/cop/performance/redundant_sort_block_spec.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Performance::RedundantSortBlock do
+  subject(:cop) { described_class.new }
+
+  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 }`.
+    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 { |a, b| b <=> a }
+    RUBY
+  end
+
+  it 'does not register an offense when sorting in direct order by some property' do
+    expect_no_offenses(<<~RUBY)
+      array.sort { |a, b| a.x <=> b.x }
+    RUBY
+  end
+
+  it 'does not register an offense when using `sort`' do
+    expect_no_offenses(<<~RUBY)
+      array.sort
+    RUBY
+  end
+end