Skip to content

Commit

Permalink
[Fix #10833] Fix an incorrect autocorrect for Style/RedundantCondition
Browse files Browse the repository at this point in the history
Fixes #10833.

This PR fixes an incorrect autocorrect for `Style/RedundantCondition`
when branches contains arithmetic operation.
  • Loading branch information
koic authored and bbatsov committed Jul 27, 2022
1 parent ac73f3b commit 6462d67
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 4 deletions.
@@ -0,0 +1 @@
* [#10833](https://github.com/rubocop/rubocop/issues/10833): Fix an incorrect autocorrect for `Style/RedundantCondition` when branches contains arithmetic operation. ([@koic][])
23 changes: 19 additions & 4 deletions lib/rubocop/cop/style/redundant_condition.rb
Expand Up @@ -154,16 +154,22 @@ def same_method?(if_branch, else_branch)
if_branch.method?(else_branch.method_name) && if_branch.receiver == else_branch.receiver
end

def if_source(if_branch)
def if_source(if_branch, arithmetic_operation)
if branches_have_method?(if_branch.parent) && if_branch.parenthesized?
if_branch.source.delete_suffix(')')
elsif arithmetic_operation
argument_source = if_branch.first_argument.source

"#{if_branch.receiver.source} #{if_branch.method_name} (#{argument_source}"
else
if_branch.source
end
end

def else_source(else_branch)
if branches_have_method?(else_branch.parent)
def else_source(else_branch, arithmetic_operation) # rubocop:disable Metrics/AbcSize
if arithmetic_operation
"#{else_branch.first_argument.source})"
elsif branches_have_method?(else_branch.parent)
else_source_if_has_method(else_branch)
elsif require_parentheses?(else_branch)
"(#{else_branch.source})"
Expand Down Expand Up @@ -198,7 +204,12 @@ def else_source_if_has_assignment(else_branch)

def make_ternary_form(node)
_condition, if_branch, else_branch = *node
ternary_form = [if_source(if_branch), else_source(else_branch)].join(' || ')
arithmetic_operation = use_arithmetic_operation?(if_branch)

ternary_form = [
if_source(if_branch, arithmetic_operation),
else_source(else_branch, arithmetic_operation)
].join(' || ')
ternary_form += ')' if branches_have_method?(node) && if_branch.parenthesized?

if node.parent&.send_type?
Expand Down Expand Up @@ -227,6 +238,10 @@ def require_braces?(node)
node.hash_type? && !node.braces?
end

def use_arithmetic_operation?(node)
node.respond_to?(:arithmetic_operation?) && node.arithmetic_operation?
end

def without_argument_parentheses_method?(node)
node.send_type? && !node.arguments.empty? &&
!node.parenthesized? && !node.operator_method? && !node.assignment_method?
Expand Down
15 changes: 15 additions & 0 deletions spec/rubocop/cop/style/redundant_condition_spec.rb
Expand Up @@ -277,6 +277,21 @@ def test
RUBY
end

it 'registers an offense and corrects when the branches contains arithmetic operation' do
expect_offense(<<~RUBY)
if foo
^^^^^^ Use double pipes `||` instead.
@value - foo
else
@value - 'bar'
end
RUBY

expect_correction(<<~RUBY)
@value - (foo || 'bar')
RUBY
end

it 'registers an offense and corrects when the branches contains method call' do
expect_offense(<<~RUBY)
if foo
Expand Down

0 comments on commit 6462d67

Please sign in to comment.