False positives for match in RegexpMatch

[dduugg]

The RegexpMatch is a bit too eager in flagging usage of match. Here's a simple example that is flagged when extending Rubocop's own API:

class MyCop < Cop
  KERNEL_PATTERN = RuboCop::NodePattern.new('(const nil? :Kernel)')
  def on_send(node)
    if KERNEL_PATTERN.match(node)
      # do stuff
    end
  end
end

I would suggest making the match_method? logic more selective, and only flag usage by receiver types known to have a regexp matching match method. At risk of getting too into implementation details, rather than:

def_node_matcher :match_method?, <<-PATTERN
  {
    (send _recv :match _)
    (send _recv :match _ (int ...))
  }
PATTERN

RegexpMatch could instead do:

def_node_matcher :match_method?, <<-PATTERN
  {
    (send {regexp str sym} :match _)
    (send {regexp str sym} :match _ (int ...))
  }
PATTERN

I'm happy to open a PR if this is agreeable.

Expected behavior

The snippet above should not trigger a Performance/RegexpMatch violation

Actual behavior

Offenses:

repro.rb:4:8: C: Performance/RegexpMatch: Use match? instead of match when MatchData is not used.
    if KERNEL_PATTERN.match(node)
       ^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected

Steps to reproduce the problem

rubocop file.rb --only Performance/RegexpMatch with the above snippet saved in file.rb

RuboCop version

0.68.0 (using Parser 2.6.0.0, running on ruby 2.4.5 x86_64-darwin17)

[bquorning]

I believe #47 would fix this issue?

[dduugg]

I think that's a different issue. Let me put together a PR with a failing test and a suggested fix, it might take a day or three though.

[koic]

Yes. This is different from #47.
@pocke You are the original author of Performance/RegexpMatch cop. Do you have an opinion on this proposal?

[pocke]

I agree with this change. Thank you!