Skip to content

Commit

Permalink
[Fix rubocop#3382] Fix EmptyElse with multiple elsifs
Browse files Browse the repository at this point in the history
An `if-elsif-else` expression containing multiple elsifs does not crash
when auto-correcting.
  • Loading branch information
lumeet committed Oct 4, 2016
1 parent d78e1c2 commit 19193f3
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 7 additions & 5 deletions lib/rubocop/cop/style/empty_else.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 28 additions & 8 deletions spec/rubocop/cop/style/empty_else_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 19193f3

Please sign in to comment.