Skip to content

Commit

Permalink
[Fix rubocop#10607] Fix autocorrect for Style/RedundantCondition wh…
Browse files Browse the repository at this point in the history
…en there are parenthesized method calls in each branch
  • Loading branch information
nobuyo committed May 7, 2022
1 parent 3531610 commit ea7f73f
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 5 deletions.
@@ -0,0 +1 @@
* [#10607](https://github.com/rubocop/rubocop/issues/10607): Fix autocorrect for `Style/RedundantCondition` when there are parenthesized method calls in each branch. ([@nobuyo][])
29 changes: 24 additions & 5 deletions lib/rubocop/cop/style/redundant_condition.rb
Expand Up @@ -44,9 +44,9 @@ def on_if(node)
message = message(node)

add_offense(range_of_offense(node), message: message) do |corrector|
if node.ternary?
if node.ternary? && !branches_have_method?(node)
correct_ternary(corrector, node)
elsif node.modifier_form? || !node.else_branch
elsif redudant_condition?(node)
corrector.replace(node, node.if_branch.source)
else
corrected = make_ternary_form(node)
Expand All @@ -59,7 +59,7 @@ def on_if(node)
private

def message(node)
if node.modifier_form? || !node.else_branch
if redudant_condition?(node)
REDUNDANT_CONDITION
else
MSG
Expand All @@ -68,6 +68,7 @@ def message(node)

def range_of_offense(node)
return node.loc.expression unless node.ternary?
return node.loc.expression if node.ternary? && branches_have_method?(node)

range_between(node.loc.question.begin_pos, node.loc.colon.end_pos)
end
Expand All @@ -81,6 +82,10 @@ def offense?(node)
(node.ternary? || !else_branch.instance_of?(AST::Node) || else_branch.single_line?)
end

def redudant_condition?(node)
node.modifier_form? || !node.else_branch
end

def use_if_branch?(else_branch)
else_branch&.if_type?
end
Expand All @@ -89,6 +94,10 @@ def use_hash_key_assignment?(else_branch)
else_branch&.send_type? && else_branch&.method?(:[]=)
end

def use_hash_key_access?(node)
node.send_type? && node.method?(:[])
end

def synonymous_condition_and_branch?(node)
condition, if_branch, _else_branch = *node
# e.g.
Expand All @@ -113,7 +122,8 @@ def synonymous_condition_and_branch?(node)
# else
# test.value = another_value?
# end
branches_have_method?(node) && condition == if_branch.first_argument
branches_have_method?(node) && condition == if_branch.first_argument &&
!use_hash_key_access?(if_branch)
end

def branches_have_assignment?(node)
Expand All @@ -140,6 +150,14 @@ def branches_have_method?(node)
if_branch.method?(else_branch.method_name)
end

def if_source(if_branch)
if branches_have_method?(if_branch.parent) && if_branch.parenthesized?
if_branch.source.delete_suffix(')')
else
if_branch.source
end
end

def else_source(else_branch)
if branches_have_method?(else_branch.parent)
else_source_if_has_method(else_branch)
Expand Down Expand Up @@ -176,7 +194,8 @@ def else_source_if_has_assignment(else_branch)

def make_ternary_form(node)
_condition, if_branch, else_branch = *node
ternary_form = [if_branch.source, else_source(else_branch)].join(' || ')
ternary_form = [if_source(if_branch), else_source(else_branch)].join(' || ')
ternary_form += ')' if branches_have_method?(node) && if_branch.parenthesized?

if node.parent&.send_type?
"(#{ternary_form})"
Expand Down
47 changes: 47 additions & 0 deletions spec/rubocop/cop/style/redundant_condition_spec.rb
Expand Up @@ -322,6 +322,21 @@ def test
RUBY
end

it 'registers an offense and corrects when the branches contains parenthesized method call' do
expect_offense(<<~RUBY)
if foo
^^^^^^ Use double pipes `||` instead.
bar(foo)
else
bar(1..2)
end
RUBY

expect_correction(<<~RUBY)
bar(foo || (1..2))
RUBY
end

it 'does not register offenses when using `nil?` and the branches contains assignment' do
expect_no_offenses(<<~RUBY)
if foo.nil?
Expand Down Expand Up @@ -351,6 +366,16 @@ def test
end
RUBY
end

it 'does not register offenses when the branches contains hash key access' do
expect_no_offenses(<<~RUBY)
if foo
bar[foo]
else
bar[1]
end
RUBY
end
end
end

Expand Down Expand Up @@ -426,6 +451,28 @@ def test
RUBY
end

it 'registers an offense and corrects with ternary expression and the branches contains parenthesized method call' do
expect_offense(<<~RUBY)
foo ? bar(foo) : bar(quux)
^^^^^^^^^^^^^^^^^^^^^^^^^^ Use double pipes `||` instead.
RUBY

expect_correction(<<~RUBY)
bar(foo || quux)
RUBY
end

it 'registers an offense and corrects with ternary expression and the branches contains chained parenthesized method call' do
expect_offense(<<~RUBY)
foo ? foo(foo).bar(foo) : foo(foo).bar(quux)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use double pipes `||` instead.
RUBY

expect_correction(<<~RUBY)
foo(foo).bar(foo || quux)
RUBY
end

it 'registers an offense and corrects when the else branch contains `rescue`' do
expect_offense(<<~RUBY)
if a
Expand Down

0 comments on commit ea7f73f

Please sign in to comment.