Merge pull request #128 from fatkodima/reverse-first-cop

Add new `Performance/ReverseFirst` cop

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### New features
 
+* [#128](https://github.com/rubocop-hq/rubocop-performance/pull/128): Add new `Performance/ReverseFirst` cop. ([@fatkodima][])
 * [#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][])

config/default.yml

@@ -196,6 +196,11 @@ Performance/ReverseEach:
   Enabled: true
   VersionAdded: '0.30'
 
+Performance/ReverseFirst:
+  Description: 'Use `last(n).reverse` instead of `reverse.first(n)`.'
+  Enabled: true
+  VersionAdded: '1.7'
+
 Performance/Size:
   Description: >-
                   Use `size` instead of `count` for counting

docs/modules/ROOT/pages/cops.adoc

@@ -27,6 +27,7 @@
 * 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#performancereversefirst[Performance/ReverseFirst]
 * xref:cops_performance.adoc#performancesize[Performance/Size]
 * xref:cops_performance.adoc#performancesortreverse[Performance/SortReverse]
 * xref:cops_performance.adoc#performancesqueeze[Performance/Squeeze]

docs/modules/ROOT/pages/cops_performance.adoc

@@ -1181,6 +1181,34 @@ change them to use `reverse_each` instead.
 
 * https://github.com/JuanitoFatas/fast-ruby#enumerablereverseeach-vs-enumerablereverse_each-code
 
+== Performance/ReverseFirst
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Enabled
+| Yes
+| Yes
+| 1.7
+| -
+|===
+
+This cop identifies places where `reverse.first(n)` and `reverse.first`
+can be replaced by `last(n).reverse` and `last`.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+array.reverse.first(5)
+array.reverse.first
+
+# good
+array.last(5).reverse
+array.last
+----
+
 == Performance/Size
 
 |===

lib/rubocop/cop/performance/reverse_first.rb

@@ -0,0 +1,78 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Performance
+      # This cop identifies places where `reverse.first(n)` and `reverse.first`
+      # can be replaced by `last(n).reverse` and `last`.
+      #
+      # @example
+      #
+      #   # bad
+      #   array.reverse.first(5)
+      #   array.reverse.first
+      #
+      #   # good
+      #   array.last(5).reverse
+      #   array.last
+      #
+      class ReverseFirst < Cop
+        include RangeHelp
+
+        MSG = 'Use `%<good_method>s` instead of `%<bad_method>s`.'
+
+        def_node_matcher :reverse_first_candidate?, <<~PATTERN
+          (send $(send _ :reverse) :first (int _)?)
+        PATTERN
+
+        def on_send(node)
+          reverse_first_candidate?(node) do |receiver|
+            range = correction_range(receiver, node)
+            message = build_message(node)
+
+            add_offense(node, location: range, message: message)
+          end
+        end
+
+        def autocorrect(node)
+          reverse_first_candidate?(node) do |receiver|
+            range = correction_range(receiver, node)
+            replacement = build_good_method(node)
+
+            lambda do |corrector|
+              corrector.replace(range, replacement)
+            end
+          end
+        end
+
+        private
+
+        def correction_range(receiver, node)
+          range_between(receiver.loc.selector.begin_pos, node.loc.expression.end_pos)
+        end
+
+        def build_message(node)
+          good_method = build_good_method(node)
+          bad_method = build_bad_method(node)
+          format(MSG, good_method: good_method, bad_method: bad_method)
+        end
+
+        def build_good_method(node)
+          if node.arguments?
+            "last(#{node.arguments.first.source}).reverse"
+          else
+            'last'
+          end
+        end
+
+        def build_bad_method(node)
+          if node.arguments?
+            "reverse.first(#{node.arguments.first.source})"
+          else
+            'reverse.first'
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/performance_cops.rb

@@ -27,6 +27,7 @@
 require_relative 'performance/redundant_sort_block'
 require_relative 'performance/regexp_match'
 require_relative 'performance/reverse_each'
+require_relative 'performance/reverse_first'
 require_relative 'performance/size'
 require_relative 'performance/sort_reverse'
 require_relative 'performance/squeeze'

spec/rubocop/cop/performance/reverse_first_spec.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Performance::ReverseFirst do
+  subject(:cop) { described_class.new }
+
+  it 'registers an offense and corrects when using `#reverse.first(5)`' do
+    expect_offense(<<~RUBY)
+      array.reverse.first(5)
+            ^^^^^^^^^^^^^^^^ Use `last(5).reverse` instead of `reverse.first(5)`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      array.last(5).reverse
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `#reverse.first`' do
+    expect_offense(<<~RUBY)
+      array.reverse.first
+            ^^^^^^^^^^^^^ Use `last` instead of `reverse.first`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      array.last
+    RUBY
+  end
+
+  it 'does not register an offense when `#reverse` is not followed by `#first`' do
+    expect_no_offenses(<<~RUBY)
+      array.reverse
+    RUBY
+
+    expect_no_offenses(<<~RUBY)
+      array.reverse.last(5)
+    RUBY
+  end
+end