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 #11399] Add new Style/UnlessMinimizeNegations
cop
#11662
base: master
Are you sure you want to change the base?
Conversation
Fixes rubocop#11399. This PR adds new `Style/UnlessMinimizeNegations` cop. It minimizes the number of negations in an `unless` using De Morgan’s laws. ```ruby # bad (all terms negated) do_something unless !x && !y # good (all terms positive) do_something if x || y # bad (2 negations, 1 positive) do_something unless !x || !y || z # good (2 positives, 1 negation) do_something if x && y && !z ```
module RuboCop | ||
module Cop | ||
module Style | ||
# Minimizes the number of negations in an `unless` using De Morgan’s laws. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why in unless
only? Seems to me this should apply to all conditionals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More often people avoid unless
than use it in difficult conditions.
# I don't think the corrected version is much better. Can be seen as a double negation.
do_something if x && !y && !z
do_something unless !x || y || z
# But such a case is not bad.
do_something if !x || !y || !z
do_something unless x && y && z
For if
I would make a parameter (disabled by default). But, this case is related to other cops. Style/UnlessElse
and Style/UnlessLogicalOperators
with forbid_logical_operators
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I'd be fine to keeping this limited to unless but the rationale needs to be explained clearly in the cop and it's relation to existing cops.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my 2c: I like the idea of conditionally supporting both if
and unless
. Perhaps the cop name could be just Style/MinimizeNegations
with an option to target if/unless/both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll finalize it as soon as I have time.
Fixes #11399.
This PR adds new
Style/UnlessMinimizeNegations
cop.It minimizes the number of negations in an
unless
using De Morgan’s laws.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}.md
if the new code introduces user-observable changes. See changelog entry format for details.