Skip to content

Commit

Permalink
Support auto-correction for Style/IdenticalConditionalBranches
Browse files Browse the repository at this point in the history
This PR supports auto-correction for `Style/IdenticalConditionalBranches`.
  • Loading branch information
koic authored and bbatsov committed May 27, 2021
1 parent 2b27105 commit 9cb3e05
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 14 deletions.
@@ -0,0 +1 @@
* [#9812](https://github.com/rubocop/rubocop/pull/9812): Support auto-correction for `Style/IdenticalConditionalBranches`. ([@koic][])
1 change: 1 addition & 0 deletions config/default.yml
Expand Up @@ -3561,6 +3561,7 @@ Style/IdenticalConditionalBranches:
out of the conditional.
Enabled: true
VersionAdded: '0.36'
VersionChanged: <<next>>

Style/IfInsideElse:
Description: 'Finds if nodes inside else, which can be converted to elsif.'
Expand Down
37 changes: 29 additions & 8 deletions lib/rubocop/cop/style/identical_conditional_branches.rb
Expand Up @@ -68,39 +68,60 @@ module Style
# do_z
# end
class IdenticalConditionalBranches < Base
include RangeHelp
extend AutoCorrector

MSG = 'Move `%<source>s` out of the conditional.'

def on_if(node)
return if node.elsif?

branches = expand_elses(node.else_branch).unshift(node.if_branch)
check_branches(branches)
check_branches(node, branches)
end

def on_case(node)
return unless node.else? && node.else_branch

branches = node.when_branches.map(&:body).push(node.else_branch)
check_branches(branches)
check_branches(node, branches)
end

private

def check_branches(branches) # rubocop:todo 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.
return if branches.any?(&:nil?)

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

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

def duplicated_expressions?(expressions)
expressions.size > 1 && expressions.uniq.one?
end

def check_expressions(expressions)
return unless expressions.size > 1 && expressions.uniq.one?
def check_expressions(node, expressions, insert_position)
inserted_expression = false

expressions.each do |expression|
add_offense(expression) do |corrector|
range = range_by_whole_lines(expression.source_range, include_final_newline: true)
corrector.remove(range)
next if inserted_expression

expressions.each { |expression| add_offense(expression) }
if insert_position == :after_condition
corrector.insert_after(node, "\n#{expression.source}")
else
corrector.insert_before(node, "#{expression.source}\n")
end
inserted_expression = true
end
end
end

def message(node)
Expand Down
70 changes: 64 additions & 6 deletions spec/rubocop/cop/style/identical_conditional_branches_spec.rb
Expand Up @@ -2,7 +2,7 @@

RSpec.describe RuboCop::Cop::Style::IdenticalConditionalBranches, :config do
context 'on if..else with identical bodies' do
it 'registers an offense' do
it 'registers and corrects an offense' do
expect_offense(<<~RUBY)
if something
do_x
Expand All @@ -12,11 +12,18 @@
^^^^ Move `do_x` out of the conditional.
end
RUBY

expect_correction(<<~RUBY)
if something
else
end
do_x
RUBY
end
end

context 'on if..else with identical trailing lines' do
it 'registers an offense' do
it 'registers and corrects an offense' do
expect_offense(<<~RUBY)
if something
method_call_here(1, 2, 3)
Expand All @@ -28,11 +35,20 @@
^^^^ Move `do_x` out of the conditional.
end
RUBY

expect_correction(<<~RUBY)
if something
method_call_here(1, 2, 3)
else
1 + 2 + 3
end
do_x
RUBY
end
end

context 'on if..else with identical leading lines' do
it 'registers an offense' do
it 'registers and corrects an offense' do
expect_offense(<<~RUBY)
if something
do_x
Expand All @@ -44,6 +60,15 @@
1 + 2 + 3
end
RUBY

expect_correction(<<~RUBY)
do_x
if something
method_call_here(1, 2, 3)
else
1 + 2 + 3
end
RUBY
end
end

Expand Down Expand Up @@ -72,7 +97,7 @@
end

context 'on case with identical bodies' do
it 'registers an offense' do
it 'registers an offense and corrects' do
expect_offense(<<~RUBY)
case something
when :a
Expand All @@ -86,6 +111,15 @@
^^^^ Move `do_x` out of the conditional.
end
RUBY

expect_correction(<<~RUBY)
case something
when :a
when :b
else
end
do_x
RUBY
end
end

Expand All @@ -105,7 +139,7 @@
end

context 'on case with identical trailing lines' do
it 'registers an offense' do
it 'registers and corrects an offense' do
expect_offense(<<~RUBY)
case something
when :a
Expand All @@ -122,11 +156,23 @@
^^^^ Move `do_x` out of the conditional.
end
RUBY

expect_correction(<<~RUBY)
case something
when :a
x1
when :b
x2
else
x3
end
do_x
RUBY
end
end

context 'on case with identical leading lines' do
it 'registers an offense' do
it 'registers and corrects an offense' do
expect_offense(<<~RUBY)
case something
when :a
Expand All @@ -143,6 +189,18 @@
x3
end
RUBY

expect_correction(<<~RUBY)
do_x
case something
when :a
x1
when :b
x2
else
x3
end
RUBY
end
end

Expand Down

0 comments on commit 9cb3e05

Please sign in to comment.