Skip to content

Commit

Permalink
Add new Style/NegatedIfElseCondition cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Oct 30, 2020
1 parent 5027e4a commit d07ed60
Show file tree
Hide file tree
Showing 10 changed files with 306 additions and 10 deletions.
1 change: 1 addition & 0 deletions changelog/new_add_negated_if_else_condition_cop.md
@@ -0,0 +1 @@
* [#8967](https://github.com/rubocop-hq/rubocop/pull/8967): Add new `Style/NegatedIfElseCondition` cop. ([@fatkodima][])
7 changes: 7 additions & 0 deletions config/default.yml
Expand Up @@ -3611,6 +3611,13 @@ Style/NegatedIf:
- prefix
- postfix

Style/NegatedIfElseCondition:
Description: >-
This cop checks for uses of `if-else` and ternary operators with a negated condition
which can be simplified by inverting condition and swapping branches.
Enabled: pending
VersionAdded: '1.2'

Style/NegatedUnless:
Description: 'Favor if over unless for negative conditions.'
StyleGuide: '#if-for-negatives'
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -442,6 +442,7 @@ In the following section you find all available cops:
* xref:cops_style.adoc#stylemultiplecomparison[Style/MultipleComparison]
* xref:cops_style.adoc#stylemutableconstant[Style/MutableConstant]
* xref:cops_style.adoc#stylenegatedif[Style/NegatedIf]
* xref:cops_style.adoc#stylenegatedifelsecondition[Style/NegatedIfElseCondition]
* xref:cops_style.adoc#stylenegatedunless[Style/NegatedUnless]
* xref:cops_style.adoc#stylenegatedwhile[Style/NegatedWhile]
* xref:cops_style.adoc#stylenestedmodifier[Style/NestedModifier]
Expand Down
40 changes: 40 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -6308,6 +6308,46 @@ end

* https://rubystyle.guide#unless-for-negatives

== Style/NegatedIfElseCondition

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| Yes
| 1.2
| -
|===

This cop checks for uses of `if-else` and ternary operators with a negated condition
which can be simplified by inverting condition and swapping branches.

=== Examples

[source,ruby]
----
# bad
if !x
do_something
else
do_something_else
end
# good
if x
do_something_else
else
do_something
end
# bad
!x ? do_something : do_something_else
# good
x ? do_something_else : do_something
----

== Style/NegatedUnless

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -496,6 +496,7 @@
require_relative 'rubocop/cop/style/multiple_comparison'
require_relative 'rubocop/cop/style/mutable_constant'
require_relative 'rubocop/cop/style/negated_if'
require_relative 'rubocop/cop/style/negated_if_else_condition'
require_relative 'rubocop/cop/style/negated_unless'
require_relative 'rubocop/cop/style/negated_while'
require_relative 'rubocop/cop/style/nested_modifier'
Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/cop/layout/end_alignment.rb
Expand Up @@ -137,10 +137,10 @@ def check_asgn_alignment(outer_node, inner_node)
def asgn_variable_align_with(outer_node, inner_node)
expr = outer_node.source_range

if !line_break_before_keyword?(expr, inner_node)
range_between(expr.begin_pos, inner_node.loc.keyword.end_pos)
else
if line_break_before_keyword?(expr, inner_node)
inner_node.loc.keyword
else
range_between(expr.begin_pos, inner_node.loc.keyword.end_pos)
end
end

Expand Down
8 changes: 4 additions & 4 deletions lib/rubocop/cop/layout/hash_alignment.rb
Expand Up @@ -296,13 +296,13 @@ def correct_node(corrector, node, delta)
# just give each lambda the same reference and they would all get the
# last value of each. A local variable fixes the problem.

if !node.value
delta_value = delta[:key] || 0
correct_no_value(corrector, delta_value, node.source_range)
else
if node.value
correct_key_value(corrector, delta, node.key.source_range,
node.value.source_range,
node.loc.operator)
else
delta_value = delta[:key] || 0
correct_no_value(corrector, delta_value, node.source_range)
end
end

Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/cop/style/hash_syntax.rb
Expand Up @@ -97,10 +97,10 @@ def ruby19_no_mixed_keys_check(pairs)
end

def no_mixed_keys_check(pairs)
if !sym_indices?(pairs)
check(pairs, ':', MSG_NO_MIXED_KEYS)
else
if sym_indices?(pairs)
check(pairs, pairs.first.inverse_delimiter, MSG_NO_MIXED_KEYS)
else
check(pairs, ':', MSG_NO_MIXED_KEYS)
end
end

Expand Down
99 changes: 99 additions & 0 deletions lib/rubocop/cop/style/negated_if_else_condition.rb
@@ -0,0 +1,99 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# This cop checks for uses of `if-else` and ternary operators with a negated condition
# which can be simplified by inverting condition and swapping branches.
#
# @example
# # bad
# if !x
# do_something
# else
# do_something_else
# end
#
# # good
# if x
# do_something_else
# else
# do_something
# end
#
# # bad
# !x ? do_something : do_something_else
#
# # good
# x ? do_something_else : do_something
#
class NegatedIfElseCondition < Base
extend AutoCorrector

MSG = 'Invert the negated condition and swap the %<type>s branches.'

NEGATED_EQUALITY_METHODS = %i[!= !~].freeze

def self.autocorrect_incompatible_with
[Style::InverseMethods, Style::Not]
end

def on_new_investigation
@corrected_nodes = nil
end

def on_if(node)
return unless if_else?(node)

condition = node.condition
return unless negated_condition?(condition)

type = node.ternary? ? 'ternary' : 'if-else'
add_offense(node, message: format(MSG, type: type)) do |corrector|
unless corrected_ancestor?(node)
correct_negated_condition(corrector, condition)
swap_branches(corrector, node)

@corrected_nodes ||= Set.new.compare_by_identity
@corrected_nodes.add(node)
end
end
end

private

def if_else?(node)
else_branch = node.else_branch
!node.elsif? && else_branch && (!else_branch.if_type? || !else_branch.elsif?)
end

def negated_condition?(node)
node.send_type? &&
(node.negation_method? || NEGATED_EQUALITY_METHODS.include?(node.method_name))
end

def corrected_ancestor?(node)
node.each_ancestor(:if).any? { |ancestor| @corrected_nodes&.include?(ancestor) }
end

def correct_negated_condition(corrector, node)
receiver, method_name, rhs = *node
replacement =
if node.negation_method?
receiver.source
else
inverted_method = method_name.to_s.sub('!', '=')
"#{receiver.source} #{inverted_method} #{rhs.source}"
end

corrector.replace(node, replacement)
end

def swap_branches(corrector, node)
corrector.replace(node.if_branch, node.else_branch.source)
corrector.replace(node.else_branch, node.if_branch.source)
end
end
end
end
end
147 changes: 147 additions & 0 deletions spec/rubocop/cop/style/negated_if_else_condition_spec.rb
@@ -0,0 +1,147 @@
# frozen_string_literal: true

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

it 'registers an offense and corrects when negating condition with `!` for `if-else`' do
expect_offense(<<~RUBY)
if !x
^^^^^ Invert the negated condition and swap the if-else branches.
do_something
else
do_something_else
end
RUBY

expect_correction(<<~RUBY)
if x
do_something_else
else
do_something
end
RUBY
end

it 'registers an offense and corrects when negating condition with `not` for `if-else`' do
expect_offense(<<~RUBY)
if not x
^^^^^^^^ Invert the negated condition and swap the if-else branches.
do_something
else
do_something_else
end
RUBY

expect_correction(<<~RUBY)
if x
do_something_else
else
do_something
end
RUBY
end

it 'registers an offense and corrects when negating condition with `not` for ternary' do
expect_offense(<<~RUBY)
!x ? do_something : do_something_else
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invert the negated condition and swap the ternary branches.
RUBY

expect_correction(<<~RUBY)
x ? do_something_else : do_something
RUBY
end

shared_examples 'negation method' do |method, inverted_method|
it "registers an offense and corrects when negating condition with `#{method}` for `if-else`" do
expect_offense(<<~RUBY, method: method)
if x %{method} y
^^^^^^{method}^^ Invert the negated condition and swap the if-else branches.
do_something
else
do_something_else
end
RUBY

expect_correction(<<~RUBY)
if x #{inverted_method} y
do_something_else
else
do_something
end
RUBY
end

it "registers an offense and corrects when negating condition with `#{method}` for ternary" do
expect_offense(<<~RUBY, method: method)
x %{method} y ? do_something : do_something_else
^^^{method}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invert the negated condition and swap the ternary branches.
RUBY

expect_correction(<<~RUBY)
x #{inverted_method} y ? do_something_else : do_something
RUBY
end
end

it_behaves_like('negation method', '!=', '==')
it_behaves_like('negation method', '!~', '=~')

it 'registers an offense and corrects nested `if-else` with negated condition' do
expect_offense(<<~RUBY)
if !x
^^^^^ Invert the negated condition and swap the if-else branches.
do_something
else
if !y
^^^^^ Invert the negated condition and swap the if-else branches.
do_something_else_1
else
do_something_else_2
end
end
RUBY

expect_correction(<<~RUBY)
if x
if y
do_something_else_2
else
do_something_else_1
end
else
do_something
end
RUBY
end

it 'does not register an offense when negating condition for `if-elsif`' do
expect_no_offenses(<<~RUBY)
if !x
do_something
elsif !y
do_something_else
else
do_another_thing
end
RUBY
end

it 'does not register an offense when not whe whole condition is negated' do
expect_no_offenses(<<~RUBY)
if !x && y
do_something
else
do_another_thing
end
RUBY
end

it 'does not register an offense when `if` with negated condition has no `else` branch' do
expect_no_offenses(<<~RUBY)
if !x
do_something
end
RUBY
end
end

0 comments on commit d07ed60

Please sign in to comment.