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
…rubocop#3146)

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.
  • Loading branch information
natalzia-paperless authored and Neodelf committed Oct 15, 2016
1 parent 190e218 commit f159bc4
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 f159bc4

Please sign in to comment.