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

New cop: Style/RedundantUnless #11410

Closed
fatkodima opened this issue Jan 9, 2023 · 2 comments · Fixed by #11432
Closed

New cop: Style/RedundantUnless #11410

fatkodima opened this issue Jan 9, 2023 · 2 comments · Fixed by #11432

Comments

@fatkodima
Copy link
Contributor

It is a known fact, that unless adds cognitive overhead. Personally, I always try to avoid it if if looks better/simpler.
This cop will detect places where unless is completely unneeded and can be replaced by if.

I was thinking, that there is a similar cop already, but was not able to find it.

For example,

# bad
foo unless !bar
foo unless x != y
foo unless x >= 10
foo unless x.positive?

# good
foo if bar
foo if x == y
foo if x < 10
foo if x.negative?

I saw at least many offenses in the rubocop itself. Here are only a few examples:

return unless blank_lines != wanted_blank_lines

return unless node.loc.operator.line != rhs.first_line

return unless count > max_params

Related: #11399

There is a kinda related cop already (Style/NegatedUnless), but it only checks for simple negations with !.

@vlad-pisanov
Copy link

This is great, although I'm sure about the name. It's not that the unless is redundant (as in "duplicated"), it's more that it's confusing / unnatural. Style/UnnaturalUnless? Style/ClearCondition? idk, can't think of a better name 🤷‍♂️

@si-lens
Copy link
Contributor

si-lens commented Jan 11, 2023

I like the idea. unless is a pain in the ass frequently, especially for non-native English speakers.
However, I agree with @vlad-pisanov that the cop name could be better.

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.

3 participants