Skip to content

Commit

Permalink
Add check for double negative in NegatedIf and NegatedWhile rules
Browse files Browse the repository at this point in the history
Previously this file only looked at the first operator in the line
With these changes there is now a pattern match to ensure `!!`
isn't flagged by these rules

Add tests for double negation in while and if specs

Update CHANGELOG.md
  • Loading branch information
natalzia-paperless committed May 20, 2016
1 parent 959cc6e commit a4eca96
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 5 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* [#3125](https://github.com/bbatsov/rubocop/issues/3125): Fix `Rails/UniqBeforePluck` to ignore `uniq` with block. ([@tejasbubane][])
* [#3116](https://github.com/bbatsov/rubocop/issues/3116): `Style/SpaceAroundKeyword` allows `&.` method calls after `super` and `yield`. ([@segiddins][])
* [#3131](https://github.com/bbatsov/rubocop/issues/3131): Fix `Style/ZeroLengthPredicate` to ignore `size` and `length` variables. ([@tejasbubane][])
* [#3146](https://github.com/bbatsov/rubocop/pull/3146): Fix `NegatedIf` and `NegatedWhile` to ignore double negations. ([@natalzia-paperless][])

## 0.40.0 (2016-05-09)

Expand Down Expand Up @@ -2168,3 +2169,4 @@
[@tjwp]: https://github.com/tjwp
[@neodelf]: https://github.com/neodelf
[@josh]: https://github.com/josh
[@natalzia-paperless]: https://github.com/natalzia-paperless
10 changes: 6 additions & 4 deletions lib/rubocop/cop/mixin/negative_conditional.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,19 @@ module Cop
# Some common code shared between FavorUnlessOverNegatedIf and
# FavorUntilOverNegatedWhile.
module NegativeConditional
def self.included(mod)
mod.def_node_matcher :single_negative?, '(send !(send _ :!) :!)'
end

def check_negative_conditional(node)
condition, _body, _rest = *node

# Look at last expression of contents if there are parentheses
# around condition.
condition = condition.children.last while condition.type == :begin
return unless condition.type == :send

_object, method = *condition
return unless method == :! && !(node.loc.respond_to?(:else) &&
node.loc.else)
return unless single_negative?(condition) &&
!(node.loc.respond_to?(:else) && node.loc.else)

add_offense(node, :expression)
end
Expand Down
9 changes: 9 additions & 0 deletions spec/rubocop/cop/style/negated_if_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,15 @@
expect(cop.offenses).to be_empty
end

it 'accepts an if where the condition is doubly negated' do
inspect_source(cop,
['if !!condition',
' some_method',
'end',
'some_method if !!condition'])
expect(cop.offenses).to be_empty
end

it 'is not confused by negated elsif' do
inspect_source(cop,
['if test.is_a?(String)',
Expand Down
11 changes: 10 additions & 1 deletion spec/rubocop/cop/style/negated_while_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
expect(cop.offenses.map(&:line)).to eq([1, 4])
end

it 'accepts an while where only part of the condition is negated' do
it 'accepts a while where only part of the condition is negated' do
inspect_source(cop,
['while !a_condition && another_condition',
' some_method',
Expand All @@ -51,6 +51,15 @@
expect(cop.messages).to be_empty
end

it 'accepts a while where the condition is doubly negated' do
inspect_source(cop,
['while !!a_condition',
' some_method',
'end',
'some_method while !!a_condition'])
expect(cop.messages).to be_empty
end

it 'autocorrects by replacing while not with until' do
corrected = autocorrect_source(cop, ['something while !x.even?',
'something while(!x.even?)'])
Expand Down

0 comments on commit a4eca96

Please sign in to comment.