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

Add EnforcedStyle for Style/DoubleNegation cop #7985

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -13,6 +13,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 @@ -2699,6 +2699,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