Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Style/NumericPredicate broke $CHILD_STATUS == 0 #3892

Closed
ShockwaveNN opened this issue Jan 11, 2017 · 3 comments
Closed

Style/NumericPredicate broke $CHILD_STATUS == 0 #3892

ShockwaveNN opened this issue Jan 11, 2017 · 3 comments

Comments

@ShockwaveNN
Copy link
Contributor

Steps to reproduce the problem

Create temp.rb file with

require "English"
pid = spawn('exit 0')
Process.wait pid
puts $CHILD_STATUS == 0

Run ruby temp.rb #true
Run rubocop temp.rb

It show warning

temp.rb:3:6: C: Style/NumericPredicate: Use $CHILD_STATUS.zero? instead of $CHILD_STATUS == 0

If you fix it to

require "English"
pid = spawn('exit 0')
Process.wait pid
puts $CHILD_STATUS.zero?

It broke

ruby temp.rb
temp.rb:4:in `<main>': undefined method `zero?' for #<Process::Status: pid 13846 exit 0> (NoMethodError)

RuboCop version

$ rubocop -V
0.46.0 (using Parser 2.3.3.1, running on ruby 2.1.10 x86_64-linux)
@Drenmi
Copy link
Collaborator

Drenmi commented Jan 11, 2017

Yes. This cop has been known to cause some false positives. The reason being we can't know what type the receiver will be at runtime (since we're doing static code analysis. 😀) We make the assumption that if a receiver is compared with 0, that receiver is a duck type of Numeric.

You can disable this for the line affected only using:

if not_a_numeric == 0 # rubocop:disable Style/NumericPredicate

or, if this is causing enough false positives that it's not worth it, you can disable it entirely in your .rubocop.yml file:

Style/NumericPredicate:
  Enabled: false

😊

@mikegee
Copy link
Contributor

mikegee commented Jan 15, 2017

For the particular example you posted, Process::Status#success? is an improvement over either sort of numeric comparison.

Process::Status allows numeric comparison to be compatible with code written a very long time ago. $CHILD_STATUS has been an instance of Process::Status since at least Ruby 1.8.6.

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 17, 2017

Process::Status allows numeric comparison to be compatible with code written a very long time ago. $CHILD_STATUS has been an instance of Process::Status since at least Ruby 1.8.6.
👍 2

We should definitely add a cop for this - it'd be trivial to implement.

Drenmi added a commit to Drenmi/rubocop that referenced this issue Jul 3, 2017
…rison of global variables

This cop would suggest replacing numeric comparison with predicate
methods for global variables, but due to global variables often being
objects that are not entirely polymorphic with `Integer`, this leads to
a large amount of false positives.

This change fixes that.
@bbatsov bbatsov closed this as completed in f808242 Jul 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants