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/InverseMethods produces behaviourally different code #4643

Closed
Valvyn opened this issue Aug 2, 2017 · 3 comments · Fixed by #4651
Closed

Style/InverseMethods produces behaviourally different code #4643

Valvyn opened this issue Aug 2, 2017 · 3 comments · Fixed by #4651
Labels

Comments

@Valvyn
Copy link

Valvyn commented Aug 2, 2017

Expected behavior

Rubocop -a should produce code that has the same behaviour as the previous code.

This is the original code:

x = y.reject { |key, _value| !(key =~ /c\d/) }

This is what a possible result could look like:

x = y.select { |key, _value| key =~ /c\d/ }

Actual behavior

Rubocop -a produces code that has different behaviour than the previous code.

The following code:

x = y.reject { |key, _value| !(key =~ /c\d/) }

Results in this which does not have the same behaviour.

x = y.reject { |key, _value| key =~ /c\d/ }

Steps to reproduce the problem

run the following code with this rubocop command:

rubocop -a --only Style/InverseMethods
x = y.reject { |key, _value| !(key =~ /c\d/) }

RuboCop version

0.49.1 (using Parser 2.4.0.0, running on ruby 2.2.4 x86_64-linux)
@Valvyn Valvyn changed the title Style/InverseMethods produces Style/InverseMethods produces behaviourally different code Aug 2, 2017
@mikegee
Copy link
Contributor

mikegee commented Aug 2, 2017

I confirmed the incorrect behavior. It is important to note the 3 corrections that occur here:

Offenses:

test/test.rb:1:5: C: [Corrected] Style/InverseMethods: Use reject instead of inverting select.
x = y.select { |key, _value| key !~ /c\d/ }
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
test/test.rb:1:5: C: [Corrected] Style/InverseMethods: Use select instead of inverting reject.
x = y.reject { |key, _value| !(key =~ /c\d/) }
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
test/test.rb:1:30: C: [Corrected] Style/InverseMethods: Use !~ instead of inverting =~.
x = y.reject { |key, _value| !(key =~ /c\d/) }
                             ^^^^^^^^^^^^^^^

1 file inspected, 3 offenses detected, 3 offenses corrected

@ghost
Copy link

ghost commented Aug 2, 2017

It seems as if an additional check would be required since the situation is not as simple as a .select versus .reject, when combined with regexes, parens, and !. Other code examples out in the wild may behave correctly; I assume that this behaviour change is only possible in regex-styles like the above, right?

@Drenmi Drenmi added the bug label Aug 5, 2017
@rrosenblum
Copy link
Contributor

It looks like the issue here has to do with the body of the block being wrapped in a not. x = y.reject { |key, _value| key !~ /c\d/ } will properly corrects to x = y.select { |key, _value| key =~ /c\d/ }. I agree that this is a bug that should be fixed. I just added a test case for this. I will try to come up for a fix for this in the next couple of days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants