Merge pull request #73 from koic/fix_false_negative_for_performance_r…

…egexp_match

Fix a false negative for `Performance/RegexpMatch`

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2019-07-20 09:00:25 +0900 using RuboCop version 0.72.0.
+# on 2019-07-27 22:39:00 +0800 using RuboCop version 0.73.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -15,14 +15,14 @@ InternalAffairs/NodeDestructuring:
     - 'lib/rubocop/cop/performance/regexp_match.rb'
     - 'lib/rubocop/cop/performance/string_replacement.rb'
 
-# Offense count: 7
+# Offense count: 8
 Metrics/AbcSize:
   Max: 17
 
 # Offense count: 2
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 149
+  Max: 164
 
 # Offense count: 14
 # Configuration parameters: CountComments, ExcludedMethods.
@@ -44,7 +44,7 @@ RSpec/ContextWording:
     - 'spec/rubocop/cop/performance/string_replacement_spec.rb'
     - 'spec/rubocop/cop/performance/times_map_spec.rb'
 
-# Offense count: 88
+# Offense count: 89
 # Configuration parameters: Max.
 RSpec/ExampleLength:
   Enabled: false

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Bug fixes
 
 * [#67](https://github.com/rubocop-hq/rubocop-performance/issues/67): Fix an error for `Performance/RedundantMerge` when `MaxKeyValuePairs` option is set to `null`. ([@koic][])
+* [#73](https://github.com/rubocop-hq/rubocop-performance/pull/73): Fix a false negative for `Performance/RegexpMatch` when `MatchData` is not detected in `if` branch of guard condition. ([@koic][])
 
 ## 1.4.0 (2019-06-20)
 

lib/rubocop/cop/performance/regexp_match.rb

@@ -177,25 +177,42 @@ def last_match_used?(match_node)
           scope_root = scope_root(match_node)
           body = scope_root ? scope_body(scope_root) : match_node.ancestors.last
 
-          return true if match_node.parent.if_type? &&
-                         match_node.parent.modifier_form?
+          range = range_to_search_for_last_matches(match_node, body, scope_root)
 
-          match_node_pos = match_node.loc.expression.begin_pos
+          find_last_match(body, range, scope_root)
+        end
 
+        def range_to_search_for_last_matches(match_node, body, scope_root)
+          expression = if modifier_form?(match_node)
+                         match_node.parent.if_branch.loc.expression
+                       else
+                         match_node.loc.expression
+                       end
+
+          match_node_pos = expression.begin_pos
           next_match_pos = next_match_pos(body, match_node_pos, scope_root)
-          range = match_node_pos..next_match_pos
 
-          find_last_match(body, range, scope_root)
+          match_node_pos..next_match_pos
         end
 
         def next_match_pos(body, match_node_pos, scope_root)
           node = search_match_nodes(body).find do |match|
-            match.loc.expression.begin_pos > match_node_pos &&
-              scope_root(match) == scope_root
+            begin_pos = if modifier_form?(match)
+                          match.parent.if_branch.loc.expression.begin_pos
+                        else
+                          match.loc.expression.begin_pos
+                        end
+
+            begin_pos > match_node_pos && scope_root(match) == scope_root
           end
+
           node ? node.loc.expression.begin_pos : Float::INFINITY
         end
 
+        def modifier_form?(match_node)
+          match_node.parent.if_type? && match_node.parent.modifier_form?
+        end
+
         def find_last_match(body, range, scope_root)
           last_matches(body).find do |ref|
             ref_pos = ref.loc.expression.begin_pos

spec/rubocop/cop/performance/regexp_match_spec.rb

@@ -38,6 +38,18 @@
       end
     RUBY2
 
+    include_examples 'offense', "#{name} in if condition", <<-RUBY, <<-RUBY2
+      do_something if #{cond}
+    RUBY
+      do_something if #{correction}
+    RUBY2
+
+    include_examples 'offense', "#{name} in unless condition", <<-RUBY, <<-RUBY2
+      do_something unless #{cond}
+    RUBY
+      do_something unless #{correction}
+    RUBY2
+
     include_examples 'offense', "#{name} in elsif condition", <<-RUBY, <<-RUBY2
       if cond
         do_something
@@ -126,6 +138,37 @@ def foo
         RUBY
       end
 
+      it "accepts #{name} in guard condition with " \
+         "#{var} is used in the line after that" do
+        expect_no_offenses(<<-RUBY)
+          def foo
+            return if #{cond}
+
+            do_something(#{var})
+          end
+        RUBY
+      end
+
+      include_examples 'offense',
+                       "#{name} in if guard condition with " \
+                       "#{var} is used in another method", <<-RUBY, <<-RUBY2
+        def foo
+          return if #{cond}
+        end
+
+        def bar
+          do_something(#{var})
+        end
+      RUBY
+        def foo
+          return if #{correction}
+        end
+
+        def bar
+          do_something(#{var})
+        end
+      RUBY2
+
       it "accepts #{name} in method with #{var} in block" do
         expect_no_offenses(<<-RUBY)
           def foo
@@ -389,5 +432,17 @@ def foo
         end
       RUBY
     end
+
+    it 'registers an offense when a regexp used independently ' \
+       'with a regexp used in `if` are mixed' do
+      expect_offense(<<-RUBY)
+        def foo
+          /re/ === re # A regexp used independently is not yet supported.
+
+          do_something if /re/ === re
+                          ^^^^^^^^^^^ Use `match?` instead of `===` when `MatchData` is not used.
+        end
+      RUBY
+    end
   end
 end