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

[Fix #10607] Fix autocorrect for Style/RedundantCondition with parenthesized method call #10609

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
@@ -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