[Fix rubocop#163] Detect array indexes for Performance/Detect

For example, `[1, 2, 3].detect{ ... }[0]` will now be caught and corrected by the cop.

CHANGELOG.md

@@ -7,6 +7,10 @@
 * [#164](https://github.com/rubocop-hq/rubocop-performance/pull/164): Fix an error for `Performance/CollectionLiteralInLoop` when a method from `Enumerable` is called with no receiver. ([@eugeneius][])
 * [#165](https://github.com/rubocop-hq/rubocop-performance/issues/165): Fix a false positive for `Performance/Sum` when using initial value argument is a variable. ([@koic][])
 
+### Changes
+
+* [#163](https://github.com/rubocop-hq/rubocop-performance/pull/163): Change `Performance/Detect` to also detect offenses when index 0 or -1 is used instead (ie. `detect{ ... }[0]`). ([@dvandersluis][])
+
 ## 1.8.0 (2020-09-04)
 
 ### New features
@@ -162,3 +166,4 @@
 [@dischorde]: https://github.com/dischorde
 [@siegfault]: https://github.com/siegfault
 [@fatkodima]: https://github.com/fatkodima
+[@dvandersluis]: https://github.com/dvandersluis

docs/modules/ROOT/pages/cops_performance.adoc

@@ -577,8 +577,8 @@ str.sub!(/suffix$/, '')
 | 1.8
 |===
 
-This cop is used to identify usages of
-`select.first`, `select.last`, `find_all.first`, `find_all.last`, `filter.first`, and `filter.last`
+This cop is used to identify usages of `first`, `last`, `[0]` or `[-1]`
+chained to `select`, `find_all`, or `find_all`
 and change them to use `detect` instead.
 
 `ActiveRecord` compatibility:
@@ -597,6 +597,8 @@ considered unsafe.
 [].find_all { |item| true }.last
 [].filter { |item| true }.first
 [].filter { |item| true }.last
+[].filter { |item| true }[0]
+[].filter { |item| true }[-1]
 
 # good
 [].detect { |item| true }

lib/rubocop/cop/performance/detect.rb

@@ -3,8 +3,8 @@
 module RuboCop
   module Cop
     module Performance
-      # This cop is used to identify usages of
-      # `select.first`, `select.last`, `find_all.first`, `find_all.last`, `filter.first`, and `filter.last`
+      # This cop is used to identify usages of `first`, `last`, `[0]` or `[-1]`
+      # chained to `select`, `find_all`, or `find_all`
       # and change them to use `detect` instead.
       #
       # @example
@@ -15,6 +15,8 @@ module Performance
       #   [].find_all { |item| true }.last
       #   [].filter { |item| true }.first
       #   [].filter { |item| true }.last
+      #   [].filter { |item| true }[0]
+      #   [].filter { |item| true }[-1]
       #
       #   # good
       #   [].detect { |item| true }
@@ -27,27 +29,40 @@ module Performance
       class Detect < Base
         extend AutoCorrector
 
+        CANDIDATE_METHODS = Set[:select, :find_all, :filter].freeze
+
         MSG = 'Use `%<prefer>s` instead of ' \
               '`%<first_method>s.%<second_method>s`.'
         REVERSE_MSG = 'Use `reverse.%<prefer>s` instead of ' \
                       '`%<first_method>s.%<second_method>s`.'
+        INDEX_MSG = 'Use `%<prefer>s` instead of ' \
+              '`%<first_method>s[%<index>i]`.'
+        INDEX_REVERSE_MSG = 'Use `reverse.%<prefer>s` instead of ' \
+              '`%<first_method>s[%<index>i]`.'
 
         def_node_matcher :detect_candidate?, <<~PATTERN
           {
-            (send $(block (send _ {:select :find_all :filter}) ...) ${:first :last} $...)
-            (send $(send _ {:select :find_all :filter} ...) ${:first :last} $...)
+            (send $(block (send _ %CANDIDATE_METHODS) ...) ${:first :last} $...)
+            (send $(block (send _ %CANDIDATE_METHODS) ...) $:[] (int ${0 -1}))
+            (send $(send _ %CANDIDATE_METHODS ...) ${:first :last} $...)
+            (send $(send _ %CANDIDATE_METHODS ...) $:[] (int ${0 -1}))
           }
         PATTERN
 
         def on_send(node)
           detect_candidate?(node) do |receiver, second_method, args|
+            if second_method == :[]
+              index = args
+              args = {}
+            end
+
             return unless args.empty?
             return unless receiver
 
             receiver, _args, body = *receiver if receiver.block_type?
             return if accept_first_call?(receiver, body)
 
-            register_offense(node, receiver, second_method)
+            register_offense(node, receiver, second_method, index)
           end
         end
 
@@ -62,28 +77,31 @@ def accept_first_call?(receiver, body)
           lazy?(caller)
         end
 
-        def register_offense(node, receiver, second_method)
+        def register_offense(node, receiver, second_method, index)
           _caller, first_method, _args = *receiver
           range = receiver.loc.selector.join(node.loc.selector)
 
-          message = second_method == :last ? REVERSE_MSG : MSG
+          message = message_for_method(second_method, index)
           formatted_message = format(message, prefer: preferred_method,
                                               first_method: first_method,
-                                              second_method: second_method)
+                                              second_method: second_method,
+                                              index: index)
 
           add_offense(range, message: formatted_message) do |corrector|
-            autocorrect(corrector, node)
+            autocorrect(corrector, node, replacement(second_method, index))
           end
         end
 
-        def autocorrect(corrector, node)
-          receiver, first_method = *node
+        def replacement(method, index)
+          if method == :last || method == :[] && index == -1
+            "reverse.#{preferred_method}"
+          else
+            preferred_method
+          end
+        end
 
-          replacement = if first_method == :last
-                          "reverse.#{preferred_method}"
-                        else
-                          preferred_method
-                        end
+        def autocorrect(corrector, node, replacement)
+          receiver, _first_method = *node
 
           first_range = receiver.source_range.end.join(node.loc.selector)
 
@@ -93,6 +111,17 @@ def autocorrect(corrector, node)
           corrector.replace(receiver.loc.selector, replacement)
         end
 
+        def message_for_method(method, index)
+          case method
+          when :[]
+            index == -1 ? INDEX_REVERSE_MSG : INDEX_MSG
+          when :last
+            REVERSE_MSG
+          else
+            MSG
+          end
+        end
+
         def preferred_method
           config.for_cop('Style/CollectionMethods')['PreferredMethods']['detect'] || 'detect'
         end

spec/rubocop/cop/performance/detect_spec.rb

@@ -101,6 +101,28 @@
       RUBY
     end
 
+    it "registers an offense with #{method} short syntax and [0]" do
+      expect_offense(<<~RUBY, method: method)
+        [1, 2, 3].#{method}(&:even?)[0]
+                  ^{method}^^^^^^^^^^^^ Use `detect` instead of `#{method}[0]`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        [1, 2, 3].detect(&:even?)
+      RUBY
+    end
+
+    it "registers an offense with #{method} short syntax and [0]" do
+      expect_offense(<<~RUBY, method: method)
+        [1, 2, 3].#{method}(&:even?)[-1]
+                  ^{method}^^^^^^^^^^^^^ Use `reverse.detect` instead of `#{method}[-1]`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        [1, 2, 3].reverse.detect(&:even?)
+      RUBY
+    end
+
     it "registers an offense when #{method} is called on `lazy` without receiver" do
       expect_offense(<<~RUBY, method: method)
         lazy.#{method}(&:even?).first
@@ -112,6 +134,28 @@
       RUBY
     end
 
+    it "registers an offense and corrects when [0] is called on #{method}" do
+      expect_offense(<<~RUBY, method: method)
+        [1, 2, 3].#{method} { |i| i % 2 == 0 }[0]
+                  ^{method}^^^^^^^^^^^^^^^^^^^^^^ Use `detect` instead of `#{method}[0]`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        [1, 2, 3].detect { |i| i % 2 == 0 }
+      RUBY
+    end
+
+    it "registers an offense and corrects when [-1] is called on #{method}" do
+      expect_offense(<<~RUBY, method: method)
+        [1, 2, 3].#{method} { |i| i % 2 == 0 }[-1]
+                  ^{method}^^^^^^^^^^^^^^^^^^^^^^^ Use `reverse.detect` instead of `#{method}[-1]`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        [1, 2, 3].reverse.detect { |i| i % 2 == 0 }
+      RUBY
+    end
+
     it "does not register an offense when #{method} is used without first or last" do
       expect_no_offenses("[1, 2, 3].#{method} { |i| i % 2 == 0 }")
     end