Merge pull request #363 from koic/fix_false_negatives_for_performance…

…_string_replacement Support safe navigation operator for 15 Performance cops
rubocop · Jul 15, 2023 · 9d56a69 · 9d56a69
2 parents 563eb44 + 3b4856e
commit 9d56a69
Show file tree

Hide file tree

Showing 31 changed files with 290 additions and 30 deletions.
diff --git a/changelog/new_support_safe_navigation_operator_for_15_performance_cops.md b/changelog/new_support_safe_navigation_operator_for_15_performance_cops.md
@@ -0,0 +1 @@
+* [#363](https://github.com/rubocop/rubocop-performance/pull/363): Support safe navigation operator for `Performance/ArraySemiInfiniteRangeSlice`, `Performance/DeletePrefix`, `Performance/DeleteSuffix`, `Performance/Detect`, `Performance/EndWith`, `Performance/InefficientHashSearch`, `Performance/MapCompact`, `Performance/RedundantSplitRegexpArgument`, `Performance/ReverseEach`, `Performance/ReverseFirst`, `Performance/SelectMap`, `Performance/Squeeze`, `Performance/StartWith`, `Performance/StringInclude`, and `Performance/StringReplacement` cops. ([@koic][])
diff --git a/lib/rubocop/cop/performance/array_semi_infinite_range_slice.rb b/lib/rubocop/cop/performance/array_semi_infinite_range_slice.rb
@@ -39,7 +39,7 @@ class ArraySemiInfiniteRangeSlice < Base
         RESTRICT_ON_SEND = SLICE_METHODS
 
         def_node_matcher :endless_range_slice?, <<~PATTERN
-          (send $_ $%SLICE_METHODS $#endless_range?)
+          (call $_ $%SLICE_METHODS $#endless_range?)
         PATTERN
 
         def_node_matcher :endless_range?, <<~PATTERN
@@ -59,6 +59,7 @@ def on_send(node)
             end
           end
         end
+        alias on_csend on_send
 
         private
 

diff --git a/lib/rubocop/cop/performance/delete_prefix.rb b/lib/rubocop/cop/performance/delete_prefix.rb
@@ -64,9 +64,10 @@ class DeletePrefix < Base
         }.freeze
 
         def_node_matcher :delete_prefix_candidate?, <<~PATTERN
-          (send $!nil? ${:gsub :gsub! :sub :sub!} (regexp (str $#literal_at_start?) (regopt)) (str $_))
+          (call $!nil? ${:gsub :gsub! :sub :sub!} (regexp (str $#literal_at_start?) (regopt)) (str $_))
         PATTERN
 
+        # rubocop:disable Metrics/AbcSize
         def on_send(node)
           return unless (receiver, bad_method, regexp_str, replace_string = delete_prefix_candidate?(node))
           return unless replace_string.empty?
@@ -80,11 +81,13 @@ def on_send(node)
             regexp_str = interpret_string_escapes(regexp_str)
             string_literal = to_string_literal(regexp_str)
 
-            new_code = "#{receiver.source}.#{good_method}(#{string_literal})"
+            new_code = "#{receiver.source}#{node.loc.dot.source}#{good_method}(#{string_literal})"
 
             corrector.replace(node, new_code)
           end
         end
+        # rubocop:enable Metrics/AbcSize
+        alias on_csend on_send
       end
     end
   end

diff --git a/lib/rubocop/cop/performance/delete_suffix.rb b/lib/rubocop/cop/performance/delete_suffix.rb
@@ -64,9 +64,10 @@ class DeleteSuffix < Base
         }.freeze
 
         def_node_matcher :delete_suffix_candidate?, <<~PATTERN
-          (send $!nil? ${:gsub :gsub! :sub :sub!} (regexp (str $#literal_at_end?) (regopt)) (str $_))
+          (call $!nil? ${:gsub :gsub! :sub :sub!} (regexp (str $#literal_at_end?) (regopt)) (str $_))
         PATTERN
 
+        # rubocop:disable Metrics/AbcSize
         def on_send(node)
           return unless (receiver, bad_method, regexp_str, replace_string = delete_suffix_candidate?(node))
           return unless replace_string.empty?
@@ -80,11 +81,13 @@ def on_send(node)
             regexp_str = interpret_string_escapes(regexp_str)
             string_literal = to_string_literal(regexp_str)
 
-            new_code = "#{receiver.source}.#{good_method}(#{string_literal})"
+            new_code = "#{receiver.source}#{node.loc.dot.source}#{good_method}(#{string_literal})"
 
             corrector.replace(node, new_code)
           end
         end
+        # rubocop:enable Metrics/AbcSize
+        alias on_csend on_send
       end
     end
   end

diff --git a/lib/rubocop/cop/performance/detect.rb b/lib/rubocop/cop/performance/detect.rb
@@ -40,9 +40,9 @@ class Detect < Base
 
         def_node_matcher :detect_candidate?, <<~PATTERN
           {
-            (send $(block (send _ %CANDIDATE_METHODS) ...) ${:first :last} $...)
+            (send $(block (call _ %CANDIDATE_METHODS) ...) ${:first :last} $...)
             (send $(block (send _ %CANDIDATE_METHODS) ...) $:[] (int ${0 -1}))
-            (send $(send _ %CANDIDATE_METHODS ...) ${:first :last} $...)
+            (send $(call _ %CANDIDATE_METHODS ...) ${:first :last} $...)
             (send $(send _ %CANDIDATE_METHODS ...) $:[] (int ${0 -1}))
           }
         PATTERN
@@ -63,6 +63,7 @@ def on_send(node)
             register_offense(node, receiver, second_method, index)
           end
         end
+        alias on_csend on_send
 
         private
 

diff --git a/lib/rubocop/cop/performance/end_with.rb b/lib/rubocop/cop/performance/end_with.rb
@@ -54,7 +54,7 @@ class EndWith < Base
         RESTRICT_ON_SEND = %i[match =~ match?].freeze
 
         def_node_matcher :redundant_regex?, <<~PATTERN
-          {(send $!nil? {:match :=~ :match?} (regexp (str $#literal_at_end?) (regopt)))
+          {(call $!nil? {:match :=~ :match?} (regexp (str $#literal_at_end?) (regopt)))
            (send (regexp (str $#literal_at_end?) (regopt)) {:match :match?} $_)
            (match-with-lvasgn (regexp (str $#literal_at_end?) (regopt)) $_)}
         PATTERN
@@ -66,12 +66,14 @@ def on_send(node)
             receiver, regex_str = regex_str, receiver if receiver.is_a?(String)
             regex_str = drop_end_metacharacter(regex_str)
             regex_str = interpret_string_escapes(regex_str)
+            dot = node.loc.dot ? node.loc.dot.source : '.'
 
-            new_source = "#{receiver.source}.end_with?(#{to_string_literal(regex_str)})"
+            new_source = "#{receiver.source}#{dot}end_with?(#{to_string_literal(regex_str)})"
 
             corrector.replace(node, new_source)
           end
         end
+        alias on_csend on_send
         alias on_match_with_lvasgn on_send
       end
     end

diff --git a/lib/rubocop/cop/performance/inefficient_hash_search.rb b/lib/rubocop/cop/performance/inefficient_hash_search.rb
@@ -45,7 +45,7 @@ class InefficientHashSearch < Base
         RESTRICT_ON_SEND = %i[include?].freeze
 
         def_node_matcher :inefficient_include?, <<~PATTERN
-          (send (send $_ {:keys :values}) :include? _)
+          (send (call $_ {:keys :values}) :include? _)
         PATTERN
 
         def on_send(node)
@@ -56,21 +56,23 @@ def on_send(node)
             add_offense(node, message: message) do |corrector|
               # Replace `keys.include?` or `values.include?` with the appropriate
               # `key?`/`value?` method.
-              corrector.replace(
-                node,
-                "#{autocorrect_hash_expression(node)}.#{autocorrect_method(node)}(#{autocorrect_argument(node)})"
-              )
+              corrector.replace(node, replacement(node))
             end
           end
         end
+        alias on_csend on_send
 
         private
 
         def message(node)
-          "Use `##{autocorrect_method(node)}` instead of `##{current_method(node)}.include?`."
+          "Use `##{correct_method(node)}` instead of `##{current_method(node)}.include?`."
         end
 
-        def autocorrect_method(node)
+        def replacement(node)
+          "#{correct_hash_expression(node)}#{correct_dot(node)}#{correct_method(node)}(#{correct_argument(node)})"
+        end
+
+        def correct_method(node)
           case current_method(node)
           when :keys then use_long_method ? 'has_key?' : 'key?'
           when :values then use_long_method ? 'has_value?' : 'value?'
@@ -86,13 +88,17 @@ def use_long_method
           preferred_config && preferred_config['EnforcedStyle'] == 'long' && preferred_config['Enabled']
         end
 
-        def autocorrect_argument(node)
+        def correct_argument(node)
           node.arguments.first.source
         end
 
-        def autocorrect_hash_expression(node)
+        def correct_hash_expression(node)
           node.receiver.receiver.source
         end
+
+        def correct_dot(node)
+          node.receiver.loc.dot.source
+        end
       end
     end
   end

diff --git a/lib/rubocop/cop/performance/map_compact.rb b/lib/rubocop/cop/performance/map_compact.rb
@@ -40,12 +40,12 @@ class MapCompact < Base
         def_node_matcher :map_compact, <<~PATTERN
           {
             (send
-              $(send _ {:map :collect}
+              $(call _ {:map :collect}
                 (block_pass
                   (sym _))) _)
             (send
               (block
-                $(send _ {:map :collect})
+                $(call _ {:map :collect})
                   (args ...) _) _)
           }
         PATTERN
@@ -61,6 +61,7 @@ def on_send(node)
             remove_compact_method(corrector, map_node, node, node.parent)
           end
         end
+        alias on_csend on_send
 
         private
 

diff --git a/lib/rubocop/cop/performance/redundant_split_regexp_argument.rb b/lib/rubocop/cop/performance/redundant_split_regexp_argument.rb
@@ -21,7 +21,7 @@ class RedundantSplitRegexpArgument < Base
         STR_SPECIAL_CHARS = %w[\n \" \' \\\\ \t \b \f \r].freeze
 
         def_node_matcher :split_call_with_regexp?, <<~PATTERN
-          {(send !nil? :split $regexp)}
+          {(call !nil? :split $regexp)}
         PATTERN
 
         def on_send(node)
@@ -35,6 +35,7 @@ def on_send(node)
             corrector.replace(regexp_node, "\"#{new_argument}\"")
           end
         end
+        alias on_csend on_send
 
         private
 

diff --git a/lib/rubocop/cop/performance/reverse_each.rb b/lib/rubocop/cop/performance/reverse_each.rb
@@ -27,7 +27,7 @@ class ReverseEach < Base
         RESTRICT_ON_SEND = %i[each].freeze
 
         def_node_matcher :reverse_each?, <<~MATCHER
-          (send (send _ :reverse) :each)
+          (send (call _ :reverse) :each)
         MATCHER
 
         def on_send(node)
@@ -41,6 +41,7 @@ def on_send(node)
             end
           end
         end
+        alias on_csend on_send
 
         private
 

diff --git a/lib/rubocop/cop/performance/reverse_first.rb b/lib/rubocop/cop/performance/reverse_first.rb
@@ -24,7 +24,7 @@ class ReverseFirst < Base
         RESTRICT_ON_SEND = %i[first].freeze
 
         def_node_matcher :reverse_first_candidate?, <<~PATTERN
-          (send $(send _ :reverse) :first (int _)?)
+          (send $(call _ :reverse) :first (int _)?)
         PATTERN
 
         def on_send(node)
@@ -39,6 +39,7 @@ def on_send(node)
             end
           end
         end
+        alias on_csend on_send
 
         private
 

diff --git a/lib/rubocop/cop/performance/select_map.rb b/lib/rubocop/cop/performance/select_map.rb
@@ -38,6 +38,7 @@ def on_send(node)
           range = offense_range(node, map_method)
           add_offense(range, message: format(MSG, method_name: node.method_name))
         end
+        alias on_csend on_send
 
         private
 

diff --git a/lib/rubocop/cop/performance/squeeze.rb b/lib/rubocop/cop/performance/squeeze.rb
@@ -27,14 +27,15 @@ class Squeeze < Base
         PREFERRED_METHODS = { gsub: :squeeze, gsub!: :squeeze! }.freeze
 
         def_node_matcher :squeeze_candidate?, <<~PATTERN
-          (send
+          (call
             $!nil? ${:gsub :gsub!}
             (regexp
               (str $#repeating_literal?)
               (regopt))
             (str $_))
         PATTERN
 
+        # rubocop:disable Metrics/AbcSize
         def on_send(node)
           squeeze_candidate?(node) do |receiver, bad_method, regexp_str, replace_str|
             regexp_str = regexp_str[0..-2] # delete '+' from the end
@@ -46,12 +47,14 @@ def on_send(node)
 
             add_offense(node.loc.selector, message: message) do |corrector|
               string_literal = to_string_literal(replace_str)
-              new_code = "#{receiver.source}.#{good_method}(#{string_literal})"
+              new_code = "#{receiver.source}#{node.loc.dot.source}#{good_method}(#{string_literal})"
 
               corrector.replace(node, new_code)
             end
           end
         end
+        # rubocop:enable Metrics/AbcSize
+        alias on_csend on_send
 
         private
 

diff --git a/lib/rubocop/cop/performance/start_with.rb b/lib/rubocop/cop/performance/start_with.rb
@@ -54,7 +54,7 @@ class StartWith < Base
         RESTRICT_ON_SEND = %i[match =~ match?].freeze
 
         def_node_matcher :redundant_regex?, <<~PATTERN
-          {(send $!nil? {:match :=~ :match?} (regexp (str $#literal_at_start?) (regopt)))
+          {(call $!nil? {:match :=~ :match?} (regexp (str $#literal_at_start?) (regopt)))
            (send (regexp (str $#literal_at_start?) (regopt)) {:match :match?} $_)
            (match-with-lvasgn (regexp (str $#literal_at_start?) (regopt)) $_)}
         PATTERN
@@ -66,12 +66,14 @@ def on_send(node)
             receiver, regex_str = regex_str, receiver if receiver.is_a?(String)
             regex_str = drop_start_metacharacter(regex_str)
             regex_str = interpret_string_escapes(regex_str)
+            dot = node.loc.dot ? node.loc.dot.source : '.'
 
-            new_source = "#{receiver.source}.start_with?(#{to_string_literal(regex_str)})"
+            new_source = "#{receiver.source}#{dot}start_with?(#{to_string_literal(regex_str)})"
 
             corrector.replace(node, new_source)
           end
         end
+        alias on_csend on_send
         alias on_match_with_lvasgn on_send
       end
     end

diff --git a/lib/rubocop/cop/performance/string_include.rb b/lib/rubocop/cop/performance/string_include.rb
@@ -26,11 +26,12 @@ class StringInclude < Base
         RESTRICT_ON_SEND = %i[match =~ !~ match?].freeze
 
         def_node_matcher :redundant_regex?, <<~PATTERN
-          {(send $!nil? {:match :=~ :!~ :match?} (regexp (str $#literal?) (regopt)))
+          {(call $!nil? {:match :=~ :!~ :match?} (regexp (str $#literal?) (regopt)))
            (send (regexp (str $#literal?) (regopt)) {:match :match?} $_)
            (match-with-lvasgn (regexp (str $#literal?) (regopt)) $_)}
         PATTERN
 
+        # rubocop:disable Metrics/AbcSize
         def on_send(node)
           return unless (receiver, regex_str = redundant_regex?(node))
 
@@ -40,12 +41,15 @@ def on_send(node)
           add_offense(node, message: message) do |corrector|
             receiver, regex_str = regex_str, receiver if receiver.is_a?(String)
             regex_str = interpret_string_escapes(regex_str)
+            dot = node.loc.dot ? node.loc.dot.source : '.'
 
-            new_source = "#{'!' if negation}#{receiver.source}.include?(#{to_string_literal(regex_str)})"
+            new_source = "#{'!' if negation}#{receiver.source}#{dot}include?(#{to_string_literal(regex_str)})"
 
             corrector.replace(node, new_source)
           end
         end
+        # rubocop:enable Metrics/AbcSize
+        alias on_csend on_send
         alias on_match_with_lvasgn on_send
 
         private

diff --git a/lib/rubocop/cop/performance/string_replacement.rb b/lib/rubocop/cop/performance/string_replacement.rb
@@ -29,7 +29,7 @@ class StringReplacement < Base
         BANG = '!'
 
         def_node_matcher :string_replacement?, <<~PATTERN
-          (send _ {:gsub :gsub!}
+          (call _ {:gsub :gsub!}
                     ${regexp str (send (const nil? :Regexp) {:new :compile} _)}
                     $str)
         PATTERN
@@ -42,6 +42,7 @@ def on_send(node)
             offense(node, first_param, second_param)
           end
         end
+        alias on_csend on_send
 
         private
 

diff --git a/spec/rubocop/cop/performance/array_semi_infinite_range_slice_spec.rb b/spec/rubocop/cop/performance/array_semi_infinite_range_slice_spec.rb
@@ -44,6 +44,20 @@
       RUBY
     end
 
+    it 'registers an offense and corrects when using `slice` with semi-infinite ranges and safe navigation operator' do
+      expect_offense(<<~RUBY)
+        array&.slice(2..)
+        ^^^^^^^^^^^^^^^^^ Use `drop` instead of `slice` with semi-infinite range.
+        array&.slice(..2)
+        ^^^^^^^^^^^^^^^^^ Use `take` instead of `slice` with semi-infinite range.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        array.drop(2)
+        array.take(3)
+      RUBY
+    end
+
     it 'does not register an offense when using `[]` with full range' do
       expect_no_offenses(<<~RUBY)
         array[0..2]