Rubocop: Spurious "zero? instead of" suggestion when comparing string "index('--') == 0"

[PeterHickman]

Expected behaviour

The code if arg.index('--') == 0 is correct and should not raise an issue when checking the return value of the String index() method

Actual behaviour

The following code if arg.index('--') == 0 gives Use arg.index('--').zero? instead of arg.index('--') == 0

The .index method from the String class will return nil if the argument is not in anywhere in the string. This evaluates to nil.zero? which raises NoMethodError: undefined method 'zero?' for nil:NilClass if the suggestion is applied

Steps to reproduce the problem

--- fred.rb

a = 'tom dick harry'
if a.index('tom') == 0
  puts 'Tom is at the head of the string'
else
  puts 'Where is tom?'
end

$ rubocop -f simple fred.rb 
== fred.rb ==
C:  2:  4: Use a.index('tom').zero? instead of a.index('tom') == 0.

1 file inspected, 1 offense detected

RuboCop version

$ rubocop -V
0.49.1 (using Parser 2.4.0.0, running on ruby 2.4.0 x86_64-darwin16)

[Drenmi]

We can unfortunately not know what a will reference at runtime, and any Ruby object can implement the method #index. In general, we need to favour false positives over false negatives, because there are tools to deal with the false positives, but false negatives will go undetected forever.

This is the biggest limitation of doing static analysis on a dynamic language, I'm afraid.

Ruby chose to sprinkle nil return values here and there in it's methods, to create a sort of "duck-or-nil" typing, rather than doing something Java-esque and have "special values" like -1 for #index. Both are non-ideal. The former definitely hurts static analysis more though.

Right now you have a few choices:

• Disable the cop altogether.
• Disable the cop for this instance only, using a # rubocop:disable directive.
• Change the code to also include an explicit #nil? conditional.

[mikegee]

For the particular example provided, String#start_with? is a better choice.

[PeterHickman]

I had a feeling that this was not going to be easy (for some value of easy). Thanks for the start_with? suggestion, I will go with that

[ndbroadbent]

Just ran into this same issue with the flock method on File. It returns either false or 0:

undefined method `zero?' for false:FalseClass (NoMethodError)

RuboCop is actually breaking a native Ruby method by default, so this is definitely a problem that needs to be fixed.

I think .zero? would only be a good default if it was defined on Object, similar to .nil?. You can call false.nil?, nil.nil?, 1.nil?, etc., but you can't do the same for .zero?. It's only defined on numbers, and will crash everywhere else. There's too many cases in Ruby where a return value can either be nil, a boolean, or a number. So I think this cop should probably be removed.

At the very least it should have a special case that handles native Ruby methods like File#flock.

[mikegee]

You make some strong arguments, but I'd prefer to ignore them and recommend you don't compare the return value with 0 at all, and just use something like fail unless f.flock(File::LOCK_EX). (0 is "truthy" in Ruby.)

[ndbroadbent]

Yeah, that would work too. I've been bitten by this in the past, so I'm always careful when using "truthy" values. Always better to stick with true and false whenever possible. RuboCop won't let me use !!, so I've just gone with != false instead of == 0.

[Darhazer]

Since version 0.60, Style/NumericPredicate won't be run with --safe option as it yields false positives by design.