Skip to content

Commit

Permalink
Add EnforcedStyle for Style/DoubleNegation cop
Browse files Browse the repository at this point in the history
This PR adds `EnforcedStyle` option for `Style/DoubleNegation` cop,
and `EnforcedStyle: allowed_in_boolean_context` by default based on #7833 survey.

```ruby
# @example EnforcedStyle: allowed_in_boolean_context (default)
#   # good
#   def foo?
#     !!return_value
#   end
#
# @example EnforcedStyle: always
#   # bad
#   def foo?
#     !!return_value
#   end
```

Ruby will return the last evaluated value.

This PR only supports simple contexts, such as the end of line value of
method, and when using `return`.
A complex contexts should also be supported, such as when branching with
`if` and others in the future.
  • Loading branch information
koic authored and bbatsov committed May 17, 2020
1 parent fdbbda2 commit 18e726b
Show file tree
Hide file tree
Showing 5 changed files with 157 additions and 20 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@
* [#7962](https://github.com/rubocop-hq/rubocop/issues/7962): Fix a false positive for `Lint/ParenthesesAsGroupedExpression` when heredoc has a space between the same string as the method name and `(`. ([@koic][])
* [#7967](https://github.com/rubocop-hq/rubocop/pull/7967): `Style/SlicingWithRange` cop now supports any expression as its first index. ([@zverok][])
* [#7972](https://github.com/rubocop-hq/rubocop/issues/7972): Fix an incorrect autocrrect for `Style/HashSyntax` when using a return value uses `return`. ([@koic][])
* [#7985](https://github.com/rubocop-hq/rubocop/pull/7985): Add `EnforcedStyle` for `Style/DoubleNegation` cop and allow double nagation in contexts that use boolean as a return value. ([@koic][])

### Changes

Expand Down
5 changes: 5 additions & 0 deletions config/default.yml
Expand Up @@ -2706,6 +2706,11 @@ Style/DoubleNegation:
StyleGuide: '#no-bang-bang'
Enabled: true
VersionAdded: '0.19'
VersionChanged: '0.84'
EnforcedStyle: allowed_in_returns
SupportedStyles:
- allowed_in_returns
- forbidden

Style/EachForSimpleLoop:
Description: >-
Expand Down
45 changes: 41 additions & 4 deletions lib/rubocop/cop/style/double_negation.rb
Expand Up @@ -3,32 +3,69 @@
module RuboCop
module Cop
module Style
# This cop checks for uses of double negation (!!) to convert something
# to a boolean value. As this is both cryptic and usually redundant, it
# should be avoided.
# This cop checks for uses of double negation (`!!`) to convert something to a boolean value.
#
# @example
# When using `EnforcedStyle: allowed_in_returns`, allow double nagation in contexts
# that use boolean as a return value. When using `EnforcedStyle: forbidden`, double nagation
# should be forbidden always.
#
# @example
# # bad
# !!something
#
# # good
# !something.nil?
#
# @example EnforcedStyle: allowed_in_returns (default)
# # good
# def foo?
# !!return_value
# end
#
# @example EnforcedStyle: forbidden
# # bad
# def foo?
# !!return_value
# end
#
# Please, note that when something is a boolean value
# !!something and !something.nil? are not the same thing.
# As you're unlikely to write code that can accept values of any type
# this is rarely a problem in practice.
class DoubleNegation < Cop
include ConfigurableEnforcedStyle

MSG = 'Avoid the use of double negation (`!!`).'

def_node_matcher :double_negative?, '(send (send _ :!) :!)'

def on_send(node)
return unless double_negative?(node) && node.prefix_bang?
return if style == :allowed_in_returns && allowed_in_returns?(node)

add_offense(node, location: :selector)
end

private

def allowed_in_returns?(node)
node.parent&.return_type? || end_of_method_definition?(node)
end

def end_of_method_definition?(node)
return false unless (def_node = find_def_node_from_ascendant(node))

last_child = def_node.child_nodes.last

last_child.last_line == node.last_line
end

def find_def_node_from_ascendant(node)
return unless (parent = node.parent)
return parent if parent.def_type? || parent.defs_type?

find_def_node_from_ascendant(node.parent)
end
end
end
end
Expand Down
32 changes: 28 additions & 4 deletions manual/cops_style.md
Expand Up @@ -1588,11 +1588,13 @@ end

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | No | 0.19 | -
Enabled | Yes | No | 0.19 | 0.84

This cop checks for uses of double negation (!!) to convert something
to a boolean value. As this is both cryptic and usually redundant, it
should be avoided.
This cop checks for uses of double negation (`!!`) to convert something to a boolean value.

When using `EnforcedStyle: allowed_in_returns`, allow double nagation in contexts
that use boolean as a return value. When using `EnforcedStyle: forbidden`, double nagation
should be forbidden always.

Please, note that when something is a boolean value
!!something and !something.nil? are not the same thing.
Expand All @@ -1608,6 +1610,28 @@ this is rarely a problem in practice.
# good
!something.nil?
```
#### EnforcedStyle: allowed_in_returns (default)

```ruby
# good
def foo?
!!return_value
end
```
#### EnforcedStyle: forbidden

```ruby
# bad
def foo?
!!return_value
end
```

### Configurable attributes

Name | Default value | Configurable values
--- | --- | ---
EnforcedStyle | `allowed_in_returns` | `allowed_in_returns`, `forbidden`

### References

Expand Down
94 changes: 82 additions & 12 deletions spec/rubocop/cop/style/double_negation_spec.rb
@@ -1,20 +1,90 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Style::DoubleNegation do
subject(:cop) { described_class.new }

it 'registers an offense for !!' do
expect_offense(<<~RUBY)
!!test.something
^ Avoid the use of double negation (`!!`).
RUBY
RSpec.describe RuboCop::Cop::Style::DoubleNegation, :config do
subject(:cop) { described_class.new(config) }

let(:cop_config) do
{ 'EnforcedStyle' => enforced_style }
end

shared_examples 'common' do
it 'does not register an offense for `!!` when not a return location' do
expect_offense(<<~RUBY)
def foo?
foo
!!test.something
^ Avoid the use of double negation (`!!`).
bar
end
RUBY
end

it 'registers an offense for `!!`' do
expect_offense(<<~RUBY)
!!test.something
^ Avoid the use of double negation (`!!`).
RUBY
end

it 'does not register an offense for !' do
expect_no_offenses('!test.something')
end

it 'does not register an offense for `not not`' do
expect_no_offenses('not not test.something')
end
end

it 'does not register an offense for !' do
expect_no_offenses('!test.something')
context 'when `EnforcedStyle: allowed_in_returns`' do
let(:enforced_style) { 'allowed_in_returns' }

include_examples 'common'

it 'does not register an offense for `!!` when return location' do
expect_no_offenses(<<~RUBY)
def foo?
bar
!!baz.do_something
end
RUBY
end

it 'does not register an offense for `!!` when using `return` keyword' do
expect_no_offenses(<<~RUBY)
def foo?
return !!bar.do_something if condition
baz
!!qux
end
RUBY
end
end

it 'does not register an offense for not not' do
expect_no_offenses('not not test.something')
context 'when `EnforcedStyle: forbidden`' do
let(:enforced_style) { 'forbidden' }

include_examples 'common'

it 'registers an offense for `!!` when return location' do
expect_offense(<<~RUBY)
def foo?
bar
!!baz.do_something
^ Avoid the use of double negation (`!!`).
end
RUBY
end

it 'does not register an offense for `!!` when using `return` keyword' do
expect_offense(<<~RUBY)
def foo?
return !!bar.do_something if condition
^ Avoid the use of double negation (`!!`).
baz
!!bar
^ Avoid the use of double negation (`!!`).
end
RUBY
end
end
end

0 comments on commit 18e726b

Please sign in to comment.