-
-
Notifications
You must be signed in to change notification settings - Fork 263
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
Do not replace and
with &&
after a redirect_to or redirect_back
#210
Comments
Can you describe the incorrect behaviour you're seeing?
|
@andyw8 Thanks for reaching out, because of double rendering errors, as you can find here https://stackoverflow.com/a/1426835/1006863 and a more in depth explanation https://guides.rubyonrails.org/layouts_and_rendering.html#avoiding-double-render-errors |
Yes, I understand the need for it, but I would have expected these to have equivalent behaviour:
|
@andyw8 not really:
from https://guides.rubyonrails.org/layouts_and_rendering.html#avoiding-double-render-errors also check: http://www.virtuouscode.com/2010/08/02/using-and-and-or-in-ruby/ |
…dOr` Resolves rubocop#210. The following idioms exist for `return` and` raise` in Ruby. - `do_something and return` - `do_something || raise` And Rails will also show users the error message using this idiom. > `"redirect_to(...) and return\"` https://github.com/rails/rails/blob/v6.0.2.2/actionpack/lib/abstract_controller/rendering.rb#L10 This is the same for `render :action and return` and others. `Style/AndOr` cop default rule (`EnforcedStyle: always`) does seem to unmatch for these cases. I think these cases need to be accepted. So, this PR changes it to `EnforcedStyle: conditionals` at least in Rails. The enforced style cannot catch all cases, but probably the closest to solving the issue.
I agree with this one. The following idioms exist for
And Rails will also show users the error message using this idiom.
|
…dOr` Resolves rubocop#210. The following idioms exist for `return` and` raise` in Ruby. - `do_something and return` - `do_something || raise` And Rails will also show users the error message using this idiom. > `"redirect_to(...) and return\"` https://github.com/rails/rails/blob/v6.0.2.2/actionpack/lib/abstract_controller/rendering.rb#L10 This is the same for `render :action and return` and others. `Style/AndOr` cop default rule (`EnforcedStyle: always`) does seem to unmatch for these cases. I think these cases need to be accepted. So, this PR changes it to `EnforcedStyle: conditionals` at least in Rails. The enforced style cannot catch all cases, but probably the closest to solving the issue.
…dOr` Resolves rubocop#210. The following idioms exist for `return` and` raise` in Ruby. - `do_something and return` - `do_something || raise` And Rails will also show users the error message using this idiom. > `"redirect_to(...) and return\"` https://github.com/rails/rails/blob/v6.0.2.2/actionpack/lib/abstract_controller/rendering.rb#L10 This is the same for `render :action and return` and others. `Style/AndOr` cop default rule (`EnforcedStyle: always`) does seem to unmatch for these cases. I think these cases need to be accepted. So, this PR changes it to `EnforcedStyle: conditionals` at least in Rails. The enforced style cannot catch all cases, but probably the closest to solving the issue.
…dOr` Resolves rubocop#210. The following idioms exist for `return` and` raise` in Ruby. - `do_something and return` - `do_something || raise` And Rails will also show users the error message using this idiom. > `"redirect_to(...) and return\"` https://github.com/rails/rails/blob/v6.0.2.2/actionpack/lib/abstract_controller/rendering.rb#L10 This is the same for `render :action and return` and others. `Style/AndOr` cop default rule (`EnforcedStyle: always`) does seem to unmatch for these cases. I think these cases need to be accepted. So, this PR changes it to `EnforcedStyle: conditionals` at least in Rails. The enforced style cannot catch all cases, but probably the closest to solving the issue.
…dOr` Resolves rubocop#210. The following idioms exist for `return` and` raise` in Ruby. - `do_something and return` - `do_something || raise` And Rails will also show users the error message using this idiom. > `"redirect_to(...) and return\"` https://github.com/rails/rails/blob/v6.0.2.2/actionpack/lib/abstract_controller/rendering.rb#L10 This is the same for `render :action and return` and others. `Style/AndOr` cop default rule (`EnforcedStyle: always`) does seem to unmatch for these cases. I think these cases need to be accepted. So, this PR changes it to `EnforcedStyle: conditionals` at least in Rails. The enforced style cannot catch all cases, but probably the closest to solving the issue.
[Fix #210] Change enforced style to conditionals for `Style/AndOr`
This PR marks `Style/AndOr` as unsafe auto-correction. cf: rubocop/rubocop-rails#210
This PR marks `Style/AndOr` as unsafe auto-correction. cf: rubocop/rubocop-rails#210
When writing the code:
redirect_to(root_path) and return
it gets autocorrected withredirect_to(root_path) && return
same for
redirect_back
This led to different behaviours, because
&&
andand
are different.RuboCop version
The text was updated successfully, but these errors were encountered: