Skip to content

Commit

Permalink
[Fix rubocop#9980] Fix a false positive for `Style/IdenticalCondition…
Browse files Browse the repository at this point in the history
…alBranches`

Fixes rubocop#9980.

This PR fixes a false positive for `Style/IdenticalConditionalBranches`
when assigning to a variable used in a condition.
  • Loading branch information
koic committed Aug 5, 2021
1 parent 4460b6d commit c0390a0
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 4 deletions.
@@ -0,0 +1 @@
* [#9980](https://github.com/rubocop/rubocop/issues/9980): Fix a false positive for `Style/IdenticalConditionalBranches` when assigning to a variable used in a condition. ([@koic][])
15 changes: 11 additions & 4 deletions lib/rubocop/cop/style/identical_conditional_branches.rb
Expand Up @@ -124,14 +124,21 @@ def check_branches(node, branches)
return if branches.any?(&:nil?)

tails = branches.map { |branch| tail(branch) }
check_expressions(node, tails, :after_condition) if duplicated_expressions?(tails)
check_expressions(node, tails, :after_condition) if duplicated_expressions?(node, tails)

heads = branches.map { |branch| head(branch) }
check_expressions(node, heads, :before_condition) if duplicated_expressions?(heads)
check_expressions(node, heads, :before_condition) if duplicated_expressions?(node, heads)
end

def duplicated_expressions?(expressions)
expressions.size > 1 && expressions.uniq.one?
def duplicated_expressions?(node, expressions)
unique_expressions = expressions.uniq
return false unless expressions.size >= 1 && unique_expressions.one?

unique_expression = unique_expressions.first
return true unless unique_expression.assignment?

lhs = unique_expression.child_nodes.first
node.condition.child_nodes.none? { |n| n.source == lhs.source if n.variable? }
end

def check_expressions(node, expressions, insert_position) # rubocop:disable Metrics/MethodLength
Expand Down
47 changes: 47 additions & 0 deletions spec/rubocop/cop/style/identical_conditional_branches_spec.rb
Expand Up @@ -96,6 +96,21 @@
end
end

context 'on if..else with identical bodies and assigning to a variable used in `if` condition' do
it "doesn't register an offense" do
expect_no_offenses(<<~RUBY)
x = 0
if x == 0
x += 1
foo
else
x += 1
bar
end
RUBY
end
end

context 'on case with identical bodies' do
it 'registers an offense and corrects' do
expect_offense(<<~RUBY)
Expand Down Expand Up @@ -233,6 +248,22 @@
end
end

context 'on case..when with identical bodies and assigning to a variable used in `case` condition' do
it "doesn't register an offense" do
expect_no_offenses(<<~RUBY)
x = 0
case x
when 1
x += 1
foo
when 42
x += 1
bar
end
RUBY
end
end

context 'when using pattern matching', :ruby27 do
context 'on case-match with identical bodies' do
it 'registers an offense and corrects' do
Expand All @@ -259,6 +290,22 @@
do_x
RUBY
end

context 'on case..in with identical bodies and assigning to a variable used in `case` condition' do
it "doesn't register an offense" do
expect_no_offenses(<<~RUBY)
x = 0
case x
in 1
x += 1
foo
in 42
x += 1
bar
end
RUBY
end
end
end

context 'when one of the case-match branches is empty' do
Expand Down

0 comments on commit c0390a0

Please sign in to comment.