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

Auto-correct for Style/AndOr does not handle precedence change correctly #8053

Closed
mo-nathan opened this issue May 28, 2020 · 3 comments · Fixed by #9094
Closed

Auto-correct for Style/AndOr does not handle precedence change correctly #8053

mo-nathan opened this issue May 28, 2020 · 3 comments · Fixed by #9094
Labels

Comments

@mo-nathan
Copy link

The auto-correct for Style/AndOr incorrectly replaces expressions like:

a or b and c

with

a || b && c

rather than

(a || b) && c

I wasn't able to find an open issue that relates to this issue. If this behavior is unavoidable for some reason, then I recommend not marking Style/AndOr a "safe" auto-correctable cop.


Expected behavior

I expect it to correctly handle the change in precedence between and and or (which have the same precedence and && and || (&& has higher precedence).

Actual behavior

Running rubocop --safe-auto-correct on this code:

#!/usr/bin/ruby
# frozen_string_literal: true

t1 = (true or false and false)
print("#{t1}\n")

results in:

#!/usr/bin/ruby
# frozen_string_literal: true

t1 = (true || false && false)
print("#{t1}\n")

The original code prints false, but the resulting code prints true.

Steps to reproduce the problem

See above.

RuboCop version

vagrant@vagrant:/vagrant/mushroom-observer$ rubocop -V
0.83.0 (using Parser 2.7.1.2, running on ruby 2.6.6 x86_64-linux)

I originally discovered this using 0.75.1 at work and replicated on a local personal project that is running 0.83.0.

@marcandre
Copy link
Contributor

marcandre commented May 28, 2020

It should be easy to avoid in theory: do the correction without the '(' ')', parse the result and check if the AST is the same or not. If not, add them...
Will be even cleaner after #7868

@stale
Copy link

stale bot commented Nov 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added the stale Issues that haven't been active in a while label Nov 25, 2020
@koic koic added bug and removed stale Issues that haven't been active in a while labels Nov 25, 2020
@mo-nathan
Copy link
Author

Thanks for keeping this alive.

koic added a commit to koic/rubocop that referenced this issue Nov 25, 2020
Fixes rubocop#8053.

This PR fixes an incorrect auto-correct for `Style/AndOr`
when `or` precedes `and`.
bbatsov pushed a commit that referenced this issue Nov 25, 2020
Fixes #8053.

This PR fixes an incorrect auto-correct for `Style/AndOr`
when `or` precedes `and`.
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.

3 participants