Skip to content

Commit

Permalink
[Fix rubocop#3411] Avoid crash in Performance::CaseWhenSplat#autocorrect
Browse files Browse the repository at this point in the history
A special case is when there is only one `when` branch, and it has
multiple conditions, with a splat followed by non-splat. This change
only fixes the crash. I think it's possible that reordering the
conditions on the line leads to faster code, but that should be checked.
It's also a more complicated change that could lead to new bugs.
  • Loading branch information
jonas054 committed Sep 30, 2016
1 parent eb54e93 commit a210a4d
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* [#3311](https://github.com/bbatsov/rubocop/issues/3311): Detect incompatibilities with the external encoding to prevent bad autocorrections in `Style/StringLiterals`. ([@deivid-rodriguez][])
* [#3499](https://github.com/bbatsov/rubocop/issues/3499): Ensure `Lint/UnusedBlockArgument` doesn't make recommendations that would change arity for methods defined using `#define_method`. ([@drenmi][])
* [#3430](https://github.com/bbatsov/rubocop/issues/3430): Fix exception in `Performance/RedundantMerge` when inspecting a `#merge!` with implicit receiver. ([@drenmi][])
* [#3411](https://github.com/bbatsov/rubocop/issues/3411): Avoid auto-correction crash for single `when` in `Performance/CaseWhenSplat`. ([@jonas054][])

### Changes

Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/performance/case_when_splat.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ def inline_fix_branch(corrector, _node, conditions, new_condition)
def reorder_condition(corrector, node, new_condition)
*_conditions, body = *node
_case_branch, *when_branches, _else_branch = *node.parent
return if when_branches.size == 1 # Can't reorder one branch

corrector.remove(when_branch_range(node, when_branches))

correction = if same_line?(node, body)
Expand Down
24 changes: 24 additions & 0 deletions spec/rubocop/cop/performance/case_when_splat_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,18 @@
expect(cop.highlights).to eq(['when *cond'])
end

# TODO: Examine if changing `when *Foo, Bar` to `when Bar, *Foo` gives faster
# code.
it 'registers an offense for a single when with splat expansion followed ' \
'by another value' do
inspect_source(cop, ['case foo',
'when *Foo, Bar',
' nil',
'end'])
expect(cop.messages).to eq([described_class::MSG])
expect(cop.highlights).to eq(['when *Foo'])
end

it 'registers an offense for multiple splat conditions at the beginning' do
inspect_source(cop, ['case foo',
'when *cond1',
Expand Down Expand Up @@ -182,6 +194,18 @@
end

context 'autocorrect' do
# TODO: Either report and auto-correct, or don't do anything. Currently we
# report but don't auto-correct.
it 'does not correct a single when with splat expansion followed by ' \
'another value' do
old_source = ['case foo',
'when *Foo, Bar',
' nil',
'end']
new_source = autocorrect_source(cop, old_source)
expect(new_source).to eq(old_source.join("\n"))
end

it 'moves a single splat condition to the end of the when conditions' do
new_source = autocorrect_source(cop, ['case foo',
'when *cond',
Expand Down

0 comments on commit a210a4d

Please sign in to comment.