From 9cb3e053ab2240184340623875f22cff9e095bb2 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 22 May 2021 16:46:57 +0900 Subject: [PATCH] Support auto-correction for `Style/IdenticalConditionalBranches` This PR supports auto-correction for `Style/IdenticalConditionalBranches`. --- ...tion_for_identical_conditional_branches.md | 1 + config/default.yml | 1 + .../style/identical_conditional_branches.rb | 37 +++++++--- .../identical_conditional_branches_spec.rb | 70 +++++++++++++++++-- 4 files changed, 95 insertions(+), 14 deletions(-) create mode 100644 changelog/new_support_autocorrection_for_identical_conditional_branches.md diff --git a/changelog/new_support_autocorrection_for_identical_conditional_branches.md b/changelog/new_support_autocorrection_for_identical_conditional_branches.md new file mode 100644 index 00000000000..aecb151f59b --- /dev/null +++ b/changelog/new_support_autocorrection_for_identical_conditional_branches.md @@ -0,0 +1 @@ +* [#9812](https://github.com/rubocop/rubocop/pull/9812): Support auto-correction for `Style/IdenticalConditionalBranches`. ([@koic][]) diff --git a/config/default.yml b/config/default.yml index 9b995fd4798..26dd618b50a 100644 --- a/config/default.yml +++ b/config/default.yml @@ -3561,6 +3561,7 @@ Style/IdenticalConditionalBranches: out of the conditional. Enabled: true VersionAdded: '0.36' + VersionChanged: <> Style/IfInsideElse: Description: 'Finds if nodes inside else, which can be converted to elsif.' diff --git a/lib/rubocop/cop/style/identical_conditional_branches.rb b/lib/rubocop/cop/style/identical_conditional_branches.rb index 120750560e7..5961764c475 100644 --- a/lib/rubocop/cop/style/identical_conditional_branches.rb +++ b/lib/rubocop/cop/style/identical_conditional_branches.rb @@ -68,39 +68,60 @@ module Style # do_z # end class IdenticalConditionalBranches < Base + include RangeHelp + extend AutoCorrector + MSG = 'Move `%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) diff --git a/spec/rubocop/cop/style/identical_conditional_branches_spec.rb b/spec/rubocop/cop/style/identical_conditional_branches_spec.rb index 45d7150ac57..b5be664900c 100644 --- a/spec/rubocop/cop/style/identical_conditional_branches_spec.rb +++ b/spec/rubocop/cop/style/identical_conditional_branches_spec.rb @@ -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 @@ -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) @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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