[Fix #11006] Allow multiple elsif for Style/IfWithBooleanLiteralBranches
#11007
Conversation
koic
force-pushed
the
change_allow_multiple_elsif_for_style_if_with_boolean_literal_branches
branch
from
ee749be
to
7953319
Compare
| @@ -8,6 +8,20 @@ module Style | |||
| # The conditions to be checked are comparison methods, predicate methods, and double negative. | |||
| # `nonzero?` method is allowed by default. | |||
| # These are customizable with `AllowedMethods` option. | |||
| # Multiple `elsif`s with booleans in branches are allowed for readability. | |||
There was a problem hiding this comment.
Perhaps it'd be better to mention that this cop targets only ifs with a single else branch. This seems clearer to me.
There was a problem hiding this comment.
Thank you! I've tweaked the text to "This cop targets only ifs with a single elsif or else branch".
koic
force-pushed
the
change_allow_multiple_elsif_for_style_if_with_boolean_literal_branches
branch
from
7953319
to
f688561
Compare
| @@ -8,6 +8,20 @@ module Style | |||
| # The conditions to be checked are comparison methods, predicate methods, and double negative. | |||
| # `nonzero?` method is allowed by default. | |||
| # These are customizable with `AllowedMethods` option. | |||
| # This cop targets only `if`s with a single `elsif` or `else` branch. | |||
| # | |||
| # [source,console] | |||
There was a problem hiding this comment.
Shouldn't it say ruby here instead of console?
| @@ -76,6 +90,12 @@ def on_if(node) | |||
|
|
|||
| private | |||
|
|
|||
| def multiple_elsif?(node) | |||
There was a problem hiding this comment.
We might add this to rubocop-ast as well.
There was a problem hiding this comment.
That's a good idea! I will open a PR to rubocop-ast.
There was a problem hiding this comment.
I've opened rubocop/rubocop-ast#239 to rubocop-ast.
There was a problem hiding this comment.
The above PR has been closed because there are currently no plans to use it outside of this PR. I would like to extract to rubocop-ast when a concrete reuse use case comes up.
…teralBranches` Fixes rubocop#11006. This PR allows multiple `elsif` for `Style/IfWithBooleanLiteralBranches`. I think it's good to allow when multiple `elsif`s are used instead of a new config option. OTOH, if there is only single `elsif`, `elsif` will not exist due to autocorrection. So I think the current behavior can be kept.
koic
force-pushed
the
change_allow_multiple_elsif_for_style_if_with_boolean_literal_branches
branch
from
f688561
to
8122219
Compare
Follow up rubocop/rubocop#11007 (comment). This PR adds `multiple_elsif` to `IfNode`.
Follow up rubocop/rubocop#11007 (comment). This PR adds `multiple_elsif` to `IfNode`.
Follow up rubocop/rubocop#11007 (comment). This PR adds `multiple_elsif` to `IfNode`.
koic
deleted the
change_allow_multiple_elsif_for_style_if_with_boolean_literal_branches
branch
|
I had forgotten about this PR. 🤦 |
Fixes #11006.
This PR allows multiple
elsifforStyle/IfWithBooleanLiteralBranches.I think it's good to allow when multiple
elsifs are used instead of a new config option. OTOH, if there is only singleelsif,elsifwill not exist due to autocorrection. So I think the current behavior can be kept.Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).bundle exec rake default. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.mdif the new code introduces user-observable changes. See changelog entry format for details.