Skip to content

Commit

Permalink
[Fix #6779] Add new cop Style/NegatedUnless
Browse files Browse the repository at this point in the history
To provide negative condition enforcement on either `if` or `unless`
but not both.
  • Loading branch information
tejasbubane authored and bbatsov committed May 13, 2019
1 parent 1fb484a commit 0d33fb1
Show file tree
Hide file tree
Showing 9 changed files with 377 additions and 17 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
## master (unreleased)

### New features

* Add support for subclassing using `Class.new` to `Lint/InheritException`. ([@houli][])
* [#6779](https://github.com/rubocop-hq/rubocop/issues/6779): Add new cop `Style/NegativeUnless` that checks for unless with negative condition. ([@tejasbubane][])

### Bug fixes

Expand Down
14 changes: 14 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3507,6 +3507,20 @@ Style/NegatedIf:
- prefix
- postfix

Style/NegatedUnless:
Description: 'Favor if over unless for negative conditions.'
StyleGuide: '#if-for-negatives'
Enabled: true
VersionAdded: '0.69'
EnforcedStyle: both
SupportedStyles:
# both: prefix and postfix negated `unless` should both use `if`
# prefix: only use `if` for negated `unless` statements positioned before the body of the statement
# postfix: only use `if` for negated `unless` statements positioned after the body of the statement
- both
- prefix
- postfix

Style/NegatedWhile:
Description: 'Favor until over while for negative conditions.'
StyleGuide: '#until-for-negatives'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,7 @@
require_relative 'rubocop/cop/style/multiple_comparison'
require_relative 'rubocop/cop/style/mutable_constant'
require_relative 'rubocop/cop/style/negated_if'
require_relative 'rubocop/cop/style/negated_unless'
require_relative 'rubocop/cop/style/negated_while'
require_relative 'rubocop/cop/style/nested_modifier'
require_relative 'rubocop/cop/style/nested_parenthesized_calls'
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/negated_if.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class NegatedIf < Cop
include NegativeConditional

def on_if(node)
return if node.elsif? || node.ternary?
return if node.unless? || node.elsif? || node.ternary?
return if correct_style?(node)

check_negative_conditional(node)
Expand Down
89 changes: 89 additions & 0 deletions lib/rubocop/cop/style/negated_unless.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# Checks for uses of unless with a negated condition. Only unless
# without else are considered. There are three different styles:
#
# - both
# - prefix
# - postfix
#
# @example EnforcedStyle: both (default)
# # enforces `if` for `prefix` and `postfix` conditionals
#
# # bad
# unless !foo
# bar
# end
#
# # good
# if foo
# bar
# end
#
# # bad
# bar unless !foo
#
# # good
# bar if foo
#
# @example EnforcedStyle: prefix
# # enforces `if` for just `prefix` conditionals
#
# # bad
# unless !foo
# bar
# end
#
# # good
# if foo
# bar
# end
#
# # good
# bar unless !foo
#
# @example EnforcedStyle: postfix
# # enforces `if` for just `postfix` conditionals
#
# # bad
# bar unless !foo
#
# # good
# bar if foo
#
# # good
# unless !foo
# bar
# end
class NegatedUnless < Cop
include ConfigurableEnforcedStyle
include NegativeConditional

def on_if(node)
return if node.if? || node.elsif? || node.ternary?
return if correct_style?(node)

check_negative_conditional(node)
end

def autocorrect(node)
ConditionCorrector.correct_negative_condition(node)
end

private

def message(node)
format(MSG, inverse: node.inverse_keyword, current: node.keyword)
end

def correct_style?(node)
style == :prefix && node.modifier_form? ||
style == :postfix && !node.modifier_form?
end
end
end
end
end
1 change: 1 addition & 0 deletions manual/cops.md
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ In the following section you find all available cops:
* [Style/MultipleComparison](cops_style.md#stylemultiplecomparison)
* [Style/MutableConstant](cops_style.md#stylemutableconstant)
* [Style/NegatedIf](cops_style.md#stylenegatedif)
* [Style/NegatedUnless](cops_style.md#stylenegatedunless)
* [Style/NegatedWhile](cops_style.md#stylenegatedwhile)
* [Style/NestedModifier](cops_style.md#stylenestedmodifier)
* [Style/NestedParenthesizedCalls](cops_style.md#stylenestedparenthesizedcalls)
Expand Down
81 changes: 81 additions & 0 deletions manual/cops_style.md
Original file line number Diff line number Diff line change
Expand Up @@ -3846,6 +3846,87 @@ EnforcedStyle | `both` | `both`, `prefix`, `postfix`
* [https://github.com/rubocop-hq/ruby-style-guide#unless-for-negatives](https://github.com/rubocop-hq/ruby-style-guide#unless-for-negatives)
## Style/NegatedUnless
Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | Yes | 0.69 | -
Checks for uses of unless with a negated condition. Only unless
without else are considered. There are three different styles:
- both
- prefix
- postfix
### Examples
#### EnforcedStyle: both (default)
```ruby
# enforces `if` for `prefix` and `postfix` conditionals
# bad
unless !foo
bar
end
# good
if foo
bar
end
# bad
bar unless !foo
# good
bar if foo
```
#### EnforcedStyle: prefix
```ruby
# enforces `if` for just `prefix` conditionals
# bad
unless !foo
bar
end
# good
if foo
bar
end
# good
bar unless !foo
```
#### EnforcedStyle: postfix
```ruby
# enforces `if` for just `postfix` conditionals
# bad
bar unless !foo
# good
bar if foo
# good
unless !foo
bar
end
```
### Configurable attributes
Name | Default value | Configurable values
--- | --- | ---
EnforcedStyle | `both` | `both`, `prefix`, `postfix`
### References
* [https://github.com/rubocop-hq/ruby-style-guide#if-for-negatives](https://github.com/rubocop-hq/ruby-style-guide#if-for-negatives)
## Style/NegatedWhile
Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
Expand Down
16 changes: 0 additions & 16 deletions spec/rubocop/cop/style/negated_if_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,6 @@
RUBY
end

it 'registers an offense for unless with exclamation point condition' do
expect_offense(<<~RUBY)
unless !a_condition
^^^^^^^^^^^^^^^^^^^ Favor `if` over `unless` for negative conditions.
some_method
end
some_method unless !a_condition
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor `if` over `unless` for negative conditions.
RUBY
end

it 'registers an offense for if with "not" condition' do
expect_offense(<<~RUBY)
if not a_condition
Expand Down Expand Up @@ -109,11 +98,6 @@
expect(corrected).to eq 'something unless (x.even?)'
end

it 'autocorrects by replacing unless not with if' do
corrected = autocorrect_source('something unless !x.even?')
expect(corrected).to eq 'something if x.even?'
end

it 'autocorrects for prefix' do
corrected = autocorrect_source(<<~RUBY)
if !foo
Expand Down
Loading

0 comments on commit 0d33fb1

Please sign in to comment.