Skip to content

Performance/RedundantEqualityComparisonBlock suggest a change that alters the result #217

@jenskdsgn

Description

@jenskdsgn

After upgrading to 1.10.0 with the introduction of the new cop Performance/RedundantEqualityComparisonBlock,
I fixed all the offenses that are related to this cop and noticed failing unit tests because the change did not only changed the style of code but the logic as well.

The underlying problem here is that internally when doing .any?(Integer) for example it does evaluate to Integer === block_argument. This is fine and works as expected, however the operation with === is not commutative, meaning block_argument === Integer can have a different output. Now, in an edge case where you want to test if a certain value is any type of a list of classes, the arguments of === are actually reversed. The cop will suggest to simplify, which then results in different behavior (see following example). I actually did not see that coming and only good unit testing saved me from breaking our code.

Furthermore, I think that the suggested change makes the code shorter but also less intuitive.
Let me know how I can help going further.


Given the example offending code:

value = "test"
[String, NilClass].any? { |klass| value.is_a?(klass) }

Expected behavior

Do not suggest to replace .any? { |klass| value.is_a?(klass) } with .any?(value) because the result is different.

Actual behavior

It prints out an offense and auto-fix will result in different result.

Steps to reproduce the problem

see example

RuboCop version

$ rubocop -V
1.10.0 (using Parser 3.0.0.0, rubocop-ast 1.4.1, running on ruby 2.7.2 x86_64-darwin19)
  - rubocop-performance 1.10.0
  - rubocop-rails 2.9.1
  - rubocop-rake 0.5.1

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions