From 19193f32268208a8051d0eb744f0b63208ae7415 Mon Sep 17 00:00:00 2001 From: Teemu Date: Tue, 4 Oct 2016 21:51:47 +0300 Subject: [PATCH] [Fix #3382] Fix EmptyElse with multiple elsifs An `if-elsif-else` expression containing multiple elsifs does not crash when auto-correcting. --- CHANGELOG.md | 1 + lib/rubocop/cop/style/empty_else.rb | 12 ++++---- spec/rubocop/cop/style/empty_else_spec.rb | 36 ++++++++++++++++++----- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 215e8df89486..9902b286c88d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ * [#3411](https://github.com/bbatsov/rubocop/issues/3411): Avoid auto-correction crash for single `when` in `Performance/CaseWhenSplat`. ([@jonas054][]) * [#3286](https://github.com/bbatsov/rubocop/issues/3286): Allow `self.a, self.b = b, a` in `Style/ParallelAssignment`. ([@jonas054][]) * [#3419](https://github.com/bbatsov/rubocop/issues/3419): Report offense for `unless x.nil?` in `Style/NonNilCheck` if `IncludeSemanticChanges` is `true`. ([@jonas054][]) +* [#3382](https://github.com/bbatsov/rubocop/issues/3382): Avoid auto-correction crash for multiple elsifs in `Style/EmptyElse`. ([@lumeet][]) ### Changes diff --git a/lib/rubocop/cop/style/empty_else.rb b/lib/rubocop/cop/style/empty_else.rb index cfe82fbd866f..7267df8aed9a 100644 --- a/lib/rubocop/cop/style/empty_else.rb +++ b/lib/rubocop/cop/style/empty_else.rb @@ -110,16 +110,18 @@ def autocorrect(node) return false if autocorrect_forbidden?(node.type.to_s) lambda do |corrector| - end_pos = if node.loc.end - node.loc.end.begin_pos - else - node.parent.loc.end.begin_pos - end + end_pos = base_if_node(node).loc.end.begin_pos corrector.remove(range_between(node.loc.else.begin_pos, end_pos)) end end + def base_if_node(node) + parent_node = node + parent_node = parent_node.parent until parent_node.loc.end + parent_node + end + def autocorrect_forbidden?(type) [type, 'both'].include? missing_else_style end diff --git a/spec/rubocop/cop/style/empty_else_spec.rb b/spec/rubocop/cop/style/empty_else_spec.rb index 394ebd7749a8..2c0db0f7f760 100644 --- a/spec/rubocop/cop/style/empty_else_spec.rb +++ b/spec/rubocop/cop/style/empty_else_spec.rb @@ -235,11 +235,21 @@ context 'with an else-clause containing only the literal nil ' \ 'using semicolons' do - let(:source) { 'if a; foo elsif b; bar else nil end' } - let(:corrected_source) { 'if a; foo elsif b; bar end' } + context 'with one elsif' do + let(:source) { 'if a; foo elsif b; bar else nil end' } + let(:corrected_source) { 'if a; foo elsif b; bar end' } - it_behaves_like 'offense registration' - it_behaves_like 'auto-correct', 'if' + it_behaves_like 'offense registration' + it_behaves_like 'auto-correct', 'if' + end + + context 'with multiple elsifs' do + let(:source) { 'if a; foo elsif b; bar; elsif c; bar else nil end' } + let(:corrected_source) { 'if a; foo elsif b; bar; elsif c; bar end' } + + it_behaves_like 'offense registration' + it_behaves_like 'auto-correct', 'if' + end end context 'with an else-clause with side-effects' do @@ -366,11 +376,21 @@ end context 'with an else-clause containing only the literal nil' do - let(:source) { 'if a; foo elsif b; bar else nil end' } - let(:corrected_source) { 'if a; foo elsif b; bar end' } + context 'with one elsif' do + let(:source) { 'if a; foo elsif b; bar else nil end' } + let(:corrected_source) { 'if a; foo elsif b; bar end' } - it_behaves_like 'offense registration' - it_behaves_like 'auto-correct', 'if' + it_behaves_like 'offense registration' + it_behaves_like 'auto-correct', 'if' + end + + context 'with multiple elsifs' do + let(:source) { 'if a; foo elsif b; bar; elsif c; bar else nil end' } + let(:corrected_source) { 'if a; foo elsif b; bar; elsif c; bar end' } + + it_behaves_like 'offense registration' + it_behaves_like 'auto-correct', 'if' + end end context 'with an else-clause with side-effects' do