Lint/MultipleComparison should not be marked safe

[dgollahon]

I am using a SQL query dsl with code like this:

query.where { date >= range.begin & date < range.end }

rubocop autocorrects this to:

query.where { date >= range.begin & date && range.begin & date < range.end }

which has different semantics.

(see follow-up comment for my original error here^^ 🤦)

---

Expected behavior

This cop seems to make wrong assumptions about the safety of operating on relational operators. Whatever the case may be, this should not change my code to something that could be semantically different (by default).

Actual behavior

It autocorrects to semantically different code as outlined above.

Steps to reproduce the problem

Run rubocop -a on source files with the snippets above.

RuboCop version

1.7.0 (using Parser 3.0.0.0, rubocop-ast 1.3.0, running on ruby 2.6.6 x86_64-darwin19)
  - rubocop-performance 1.9.2
  - rubocop-rspec 2.1.0
  - rubocop-rspec 2.1.0

Side note: that duplicate rubocop-rspec is not a typo, that's what the command outputs. Not sure if there's something wrong there as well or some quirk to my config^

[dgollahon]

Arg, I have screwed up my example 🤦 . It was originally meant to be parenthesized but I screwed up somewhere in working with the DSL and then got very confused by the autocorrect output and opened this issue. Due to operator precedence it should be:

(date >= range.begin) & (date < range.end)

so the lint did highlight something noteworthy but imo it still should not autocorrect like that. a < b < c form of code will generally not work but if it does, it is probably like that for a reason (< does not always mean "less than"). Transforming in the way that this cop does is still not safe despite my confusing/errant code example I chose above.

[koic]

Thank you for your feedback. I've opened #9352.

Side note: that duplicate rubocop-rspec is not a typo, that's what the command outputs. Not sure if there's something wrong there as well or some quirk to my config^

That's interesting. Is it possible that .rubocop.yml contains require: rubocop-rspec twice?

[dgollahon]

Thank you for your feedback. I've opened #9352.

That looks like a good improvement for reducing false positives! It does not appear to mark autocorrection as unsafe, however. It still seems to me like this should not be marked as a "safe" cop. As a very simple example if I write:

1 < 2 < 3 # => NoMethodError: undefined method `<' for true:TrueClass

I (rightly) get an error. If I let rubocop autocorrect it, it outputs the item below which returns true.

1 < 2 && 2 < 3 # => true

Unless "safe" means something different than I think it should mean there are pretty clear changes in semantics here. WDYT?

[dgollahon]

That's interesting. Is it possible that .rubocop.yml contains require: rubocop-rspec twice?

It appears to happen because my rubocop config requires rubocop-rspec but it also uses inherit_gem to use a company-wide config which additionally requires rubocop-rspec. I assume that's not a problem and I suppose the output there was reasonable (?) but it was just something kind of funny I noticed.

[koic]

Unless "safe" means something different than I think it should mean there are pretty clear changes in semantics here. WDYT?

Sure! I update the proposal #9352.

I will also investigate rubocop -V separately. Thank you for the additional information.

[dgollahon]

Thanks @koic that's great. :)