-
-
Notifications
You must be signed in to change notification settings - Fork 88
Description
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