Skip to content

Commit

Permalink
[Fix rubocop#2138] Change Style/Next to highlight condition instead o…
Browse files Browse the repository at this point in the history
…f iterator
  • Loading branch information
rrosenblum committed Aug 26, 2015
1 parent 4789ab8 commit c4e7527
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* [#2146](https://github.com/bbatsov/rubocop/pull/2146): Add STDIN support. ([@caseywebdev][])
* [#2175](https://github.com/bbatsov/rubocop/pull/2175): Files that are excluded from a cop (e.g. using the `Exclude:` config option) are no longer being processed by that cop. ([@bquorning][])
* `Rails/ActionFilter` now handles complete list of methods found in the Rails 4.2 [release notes](https://github.com/rails/rails/blob/4115a12da1409c753c747fd4bab6e612c0c6e51a/guides/source/4_2_release_notes.md#notable-changes-1). ([@MGerrior][])
* [*2138](https://github.com/bbatsov/rubocop/issues/2138): Change the offense in `Style/Next` to highlight the condition instead of the iteration. ([@rrosenblum][])

### Bug Fixes

Expand Down
12 changes: 9 additions & 3 deletions lib/rubocop/cop/style/next.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,29 @@ def on_block(node)
_, method_name = *block_owner
return unless enumerator?(method_name)
return unless ends_with_condition?(body)
*_, condition = *body

add_offense(block_owner, :selector, MSG)
offense_node = (condition && condition.if_type?) ? condition : body
add_offense(offense_node, :keyword, MSG)
end

def on_while(node)
_, body = *node
return unless body && ends_with_condition?(body)
*_, condition = *body

add_offense(node, :keyword, MSG)
offense_node = (condition && condition.if_type?) ? condition : body
add_offense(offense_node, :keyword, MSG)
end
alias_method :on_until, :on_while

def on_for(node)
_, _, body = *node
return unless body && ends_with_condition?(body)
*_, condition = *body

add_offense(node, :keyword, MSG)
offense_node = (condition && condition.if_type?) ? condition : body
add_offense(offense_node, :keyword, MSG)
end

private
Expand Down
40 changes: 29 additions & 11 deletions spec/rubocop/cop/style/next_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,30 @@
' end',
'end'])
expect(cop.offenses.size).to eq(9)
expect(cop.offenses.map(&:line).sort).to eq([1, 7, 13, 19, 25, 31, 37, 43,
49])
expect(cop.offenses.map(&:line).sort).to eq([2, 8, 14, 20, 26, 32, 38, 44,
50])
expect(cop.messages) .to eq(['Use `next` to skip iteration.'] * 9)
expect(cop.highlights).to eq(%w(downto each each_with_object for loop map
times until while))
expect(cop.highlights).to eq(%w(if if if if if if if if if))
end

it 'registers an offense for unless inside until' do
inspect_source(cop, ['until false',
' unless o == 1',
' puts o',
' end',
'end'])

expect(cop.highlights).to eq(['unless'])
end

it 'registers an offense for unless inside for' do
inspect_source(cop, ['for o in 1..3 do',
' unless o == 1',
' puts o',
' end',
'end'])

expect(cop.highlights).to eq(['unless'])
end

it 'finds loop with condition at the end in different styles' do
Expand All @@ -92,10 +111,10 @@
'end'])

expect(cop.offenses.size).to eq(3)
expect(cop.offenses.map(&:line).sort).to eq([1, 7, 14])
expect(cop.offenses.map(&:line).sort).to eq([2, 9, 15])
expect(cop.messages)
.to eq(['Use `next` to skip iteration.'] * 3)
expect(cop.highlights).to eq(['each'] * 3)
expect(cop.highlights).to eq(%w(if if unless))
end

it 'ignores empty blocks' do
Expand Down Expand Up @@ -184,10 +203,9 @@
'end'])

expect(cop.offenses.size).to eq(2)
expect(cop.offenses.map(&:line).sort).to eq([1, 5])
expect(cop.messages)
.to eq(['Use `next` to skip iteration.'] * 2)
expect(cop.highlights).to eq(['each'] * 2)
expect(cop.offenses.map(&:line).sort).to eq([2, 6])
expect(cop.messages).to eq(['Use `next` to skip iteration.'] * 2)
expect(cop.highlights).to eq(%w(if unless))
end
end

Expand Down Expand Up @@ -289,7 +307,7 @@
' end',
'end'])
expect(cop.offenses.size).to eq(1)
expect(cop.highlights).to eq(['each'])
expect(cop.highlights).to eq(['if'])
end
end

Expand Down

0 comments on commit c4e7527

Please sign in to comment.