From 18e726b813881f74766b2119e70918efe142538f Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 12 May 2020 00:53:03 +0900 Subject: [PATCH] Add `EnforcedStyle` for `Style/DoubleNegation` cop 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. --- CHANGELOG.md | 1 + config/default.yml | 5 + lib/rubocop/cop/style/double_negation.rb | 45 ++++++++- manual/cops_style.md | 32 ++++++- .../rubocop/cop/style/double_negation_spec.rb | 94 ++++++++++++++++--- 5 files changed, 157 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aeb418f6f5b..ba811273cbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/config/default.yml b/config/default.yml index eb88b25f848..e673ee560c8 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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: >- diff --git a/lib/rubocop/cop/style/double_negation.rb b/lib/rubocop/cop/style/double_negation.rb index 446bfd28628..b6dfbed9b12 100644 --- a/lib/rubocop/cop/style/double_negation.rb +++ b/lib/rubocop/cop/style/double_negation.rb @@ -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 diff --git a/manual/cops_style.md b/manual/cops_style.md index 8a24e211106..f887fb78b86 100644 --- a/manual/cops_style.md +++ b/manual/cops_style.md @@ -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. @@ -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 diff --git a/spec/rubocop/cop/style/double_negation_spec.rb b/spec/rubocop/cop/style/double_negation_spec.rb index e3d5730ebdc..b1d751b82d1 100644 --- a/spec/rubocop/cop/style/double_negation_spec.rb +++ b/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