Skip to content

Commit

Permalink
Merge pull request #11336 from fatkodima/identical_conditional_branch…
Browse files Browse the repository at this point in the history
…es-last-child

Fix `Style/IdenticalConditionalBranches` to ignore identical leading lines when branch has single child and is used in return context
  • Loading branch information
koic committed Dec 26, 2022
2 parents 1519828 + 23b3935 commit 75c1963
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#10858](https://github.com/rubocop/rubocop/issues/10858): Fix `Style/IdenticalConditionalBranches` to ignore identical leading lines when branch has single child and is used in return context. ([@fatkodima][])
15 changes: 15 additions & 0 deletions lib/rubocop/cop/style/identical_conditional_branches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ def on_case_match(node)

private

# rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
def check_branches(node, branches)
# return if any branch is empty. An empty branch can be an `if`
# without an `else` or a branch that contains only comments.
Expand All @@ -144,9 +145,13 @@ def check_branches(node, branches)
tails = branches.map { |branch| tail(branch) }
check_expressions(node, tails, :after_condition) if duplicated_expressions?(node, tails)

return if last_child_of_parent?(node) &&
branches.any? { |branch| single_child_branch?(branch) }

heads = branches.map { |branch| head(branch) }
check_expressions(node, heads, :before_condition) if duplicated_expressions?(node, heads)
end
# rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity

def duplicated_expressions?(node, expressions)
unique_expressions = expressions.uniq
Expand Down Expand Up @@ -180,6 +185,16 @@ def check_expressions(node, expressions, insert_position) # rubocop:disable Metr
end
end

def last_child_of_parent?(node)
return true unless (parent = node.parent)

parent.child_nodes.last == node
end

def single_child_branch?(branch_node)
!branch_node.begin_type? || branch_node.children.size == 1
end

def message(node)
format(MSG, source: node.source)
end
Expand Down
91 changes: 91 additions & 0 deletions spec/rubocop/cop/style/identical_conditional_branches_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,31 @@
end
end

context 'on if..else with identical leading lines, single child branch and last node of the parent' do
it "doesn't register an offense" do
expect_no_offenses(<<~RUBY)
def foo
if something
do_x
else
do_x
1 + 2 + 3
end
end
def bar
y = if something
do_x
else
do_x
1 + 2 + 3
end
do_something_else
end
RUBY
end
end

context 'on if..elsif with no else' do
it "doesn't register an offense" do
expect_no_offenses(<<~RUBY)
Expand Down Expand Up @@ -219,6 +244,39 @@
end
end

context 'on case with identical leading lines, single child branch and last node of the parent' do
it "doesn't register an offense" do
expect_no_offenses(<<~RUBY)
def foo
case something
when :a
do_x
when :b
do_x
x2
else
do_x
x3
end
end
def bar
x = case something
when :a
do_x
when :b
do_x
x2
else
do_x
x3
end
do_something
end
RUBY
end
end

context 'on case without else' do
it "doesn't register an offense" do
expect_no_offenses(<<~RUBY)
Expand Down Expand Up @@ -388,6 +446,39 @@
end
end

context 'on case-match with identical leading lines, single child branch and last node of the parent' do
it "doesn't register an offense" do
expect_no_offenses(<<~RUBY)
def foo
case something
in :a
do_x
in :b
do_x
x2
else
do_x
x3
end
end
def bar
y = case something
in :a
do_x
in :b
do_x
x2
else
do_x
x3
end
do_something
end
RUBY
end
end

context 'on case-match without else' do
it "doesn't register an offense" do
expect_no_offenses(<<~RUBY)
Expand Down

0 comments on commit 75c1963

Please sign in to comment.