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

[Fix #210] Change enforced style to conditionals for Style/AndOr #224

Merged
merged 1 commit into from Apr 25, 2020

Conversation

koic
Copy link
Member

@koic koic commented Mar 29, 2020

Resolves #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.


Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

@koic koic force-pushed the change_enforced_style_to_conditionals branch 2 times, most recently from 6dbae58 to 76f1042 Compare April 3, 2020 04:00
@koic koic force-pushed the change_enforced_style_to_conditionals branch from 76f1042 to 7cbceda Compare April 14, 2020 10:00
…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.
@koic koic force-pushed the change_enforced_style_to_conditionals branch from 7cbceda to 83a32e1 Compare April 25, 2020 10:19
@koic koic merged commit f5cbb50 into rubocop:master Apr 25, 2020
@koic koic deleted the change_enforced_style_to_conditionals branch April 25, 2020 15:28
koic added a commit to koic/rubocop that referenced this pull request May 12, 2020
This PR changes enforced style to `conditionals` for `Style/AndOr` cop.
It forward porting from rubocop/rubocop-rails#224.
bbatsov pushed a commit to rubocop/rubocop that referenced this pull request May 12, 2020
This PR changes enforced style to `conditionals` for `Style/AndOr` cop.
It forward porting from rubocop/rubocop-rails#224.
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 this pull request may close these issues.

Do not replace and with && after a redirect_to or redirect_back
1 participant