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

Rails/SaveBang Fails When Used in a Conditional with an Inverse !. #5819

Closed
joshuapinter opened this issue Apr 25, 2018 · 0 comments · Fixed by #5820
Closed

Rails/SaveBang Fails When Used in a Conditional with an Inverse !. #5819

joshuapinter opened this issue Apr 25, 2018 · 0 comments · Fixed by #5820

Comments

@joshuapinter
Copy link
Contributor

joshuapinter commented Apr 25, 2018

The Rails/SaveBang Cop is smart enough to know when the return value is being used in a conditional, except for when you are checking the inverse with a bang prefix. Then it raises an offense.


Expected behavior

Ignore when the return value is used in a conditional, for both normal and inverse.

i.e. I would it expect neither of the following to raise an offense.

normal

if @user.update_attributes( params[ :user ] )

inverse

if !@user.update_attributes( params[ :user ] )

Actual behavior

Normal works fine but inverse does not.

normal

if @user.update_attributes( params[ :user ] )

This raises an offense:

inverse

if !@user.update_attributes( params[ :user ] )

Steps to reproduce the problem

See above.

RuboCop version

Include the output of rubocop -V. Here's an example:

$ bundle exec rubocop -V
0.55.0 (using Parser 2.5.1.0, running on ruby 2.3.6 x86_64-darwin17)
Edouard-chin added a commit to Edouard-chin/rubocop that referenced this issue Apr 25, 2018
- The `conditional?` method was only checking for if/case/or/and but wasn't checking negated if
- This fixes rubocop#5819
bbatsov pushed a commit that referenced this issue Apr 26, 2018
The `conditional?` method was only checking for if/case/or/and but wasn't checking negated if.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant