Skip to content

Commit

Permalink
[Fix rubocop#3788] Prevent bad auto-correct by Style/Next for neste…
Browse files Browse the repository at this point in the history
…d conditionals

When inspecting and auto-correcting code with nested conditionals where
the inner conditional has an `else` statement, but the outer conditional
does not, the auto-correct messes up:

```
foos.each do |foo|
  unless foo[:bar]
    if baz
      do_something
    else
      do_something_else
    end
  end
end
```

was corrected to:

```
foos.each do |foo|
  unless foo[bar]
    next unless baz
      do_something
    else
      do_something_else
    end
end
```

This change fixes that.
  • Loading branch information
Drenmi committed Dec 16, 2016
1 parent 6004788 commit be603cb
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
* [#3783](https://github.com/bbatsov/rubocop/pull/3783): Maintain parentheses in `Rails/HttpPositionalArguments` when methods are defined with them. ([@kevindew][])
* [#3786](https://github.com/bbatsov/rubocop/pull/3786): Avoid crash `Style/ConditionalAssignment` cop with mass assign method. ([@pocke][])
* [#3749](https://github.com/bbatsov/rubocop/pull/3749): Detect corner case of `Style/NumericLitterals`. ([@kamaradclimber][])
* [#3788](https://github.com/bbatsov/rubocop/pull/3788): Prevent bad auto-correct in `Style/Next` when block has nested conditionals. ([@drenmi][])

## 0.46.0 (2016-11-30)

Expand Down
15 changes: 13 additions & 2 deletions lib/rubocop/cop/style/next.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,23 @@ def ends_with_condition?(body)
def simple_if_without_break?(node)
return false unless node
return false unless if_without_else?(node)
return false if style == :skip_modifier_ifs && modifier_if?(node)
return false if !modifier_if?(node) && !min_body_length?(node)
return false if if_else_children?(node)
return false if allowed_modifier_if?(node)

!exit_body_type?(node)
end

def allowed_modifier_if?(node)
style == :skip_modifier_ifs && modifier_if?(node) ||
!modifier_if?(node) && !min_body_length?(node)
end

def if_else_children?(node)
node.each_child_node(:if).any? do |child|
if_else?(child)
end
end

def if_without_else?(node)
node.if_type? && !ternary?(node) && !if_else?(node)
end
Expand Down
15 changes: 15 additions & 0 deletions spec/rubocop/cop/style/next_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,21 @@
expect(cop.offenses).to be_empty
end

it "allows loops with #{condition} with else, nested in another " \
'condition' do
inspect_source(cop, ['[].each do |o|',
' if foo',
" #{condition} o == 1",
' puts o',
' else',
" puts 'no'",
' end',
' end',
'end'])

expect(cop.offenses).to be_empty
end

it "allows loops with #{condition} with else at the end" do
inspect_source(cop, ['[].each do |o|',
' puts o',
Expand Down

0 comments on commit be603cb

Please sign in to comment.