rubocop-performance icon indicating copy to clipboard operation
rubocop-performance copied to clipboard

Performance/RegexpMatch not safe for variables that may be numeric

Open formigarafa opened this issue 2 years ago • 2 comments

Numerics can be used with =~ operator but not be used as argument for #match? The code transformation applied can break the code.


For example if it takes a file with this content:

def has_an_o?(arg)
  if arg =~ /o/i
    true
  else
    false
  end
end

has_an_o?("one") # => true
has_an_o?(1) # => false

The file contents is turned into this

def has_an_o?(arg)
  if /o/i.match?(arg)
    true
  else
    false
  end
end

has_an_o?("one") # => true
has_an_o?(1) # => :2:in `match?': no implicit conversion of Integer into String (TypeError)

Which breaks as seen above.

RuboCop version

1.50.2 (using Parser 3.2.2.1, rubocop-ast 1.28.1, running on ruby 3.0.6) [x86_64-darwin22]

  • rubocop-performance 1.14.3
  • rubocop-rails 2.15.2

formigarafa avatar May 04 '23 02:05 formigarafa

Note, the behavior of Object#=~ has changed in Ruby 3.2:

$ ruby -ve '42 =~ /regexp/'
ruby 3.1.4p223 (2023-03-30 revision 957bb7cb81) [x86_64-darwin19]
-e:1: warning: deprecated Object#=~ is called on Integer; it always returns nil

$ ruby -ve '42 =~ /regexp/'
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-darwin19]
-e:1:in `<main>': undefined method `=~' for 42:Integer (NoMethodError)

koic avatar May 06 '23 00:05 koic

Ah, that's really good to know. I will get to adjust that before get to version 3.2.

My config is set to 3.0 for now, though. I am using:

AllCops:
  TargetRubyVersion: 3.0

Would there be a way to mark a cop conditionally safe by TargetRubyVersion or is this a bad idea somehow? I do understand it will be better getting the code shape to be ready for future versions, but I believe as me most developers may be disappointed by the tool when we are trying to get things right and the tool works against it.

BTW, I am asking if it is a good or bad idea so I dig further and help making a PR. I took a quick look at the code and it will take me a while to get there. I am just trying to save my time if it may be a silly idea.

formigarafa avatar May 08 '23 22:05 formigarafa