Skip to content

Commit

Permalink
[Fix rubocop#9746] Fix a false positive for Lint/UnreachableLoop
Browse files Browse the repository at this point in the history
Fixes rubocop#9746.

This PR fixes a false positive for `Lint/UnreachableLoop`
when using conditional `next` in a loop.
  • Loading branch information
koic committed Apr 29, 2021
1 parent 74377b0 commit 0e1a312
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 2 deletions.
1 change: 1 addition & 0 deletions changelog/fix_false_positive_for_lint_unreachable_loop.md
@@ -0,0 +1 @@
* [#9746](https://github.com/rubocop/rubocop/pull/9746): Fix a false positive for `Lint/UnreachableLoop` when using conditional `next` in a loop. ([@koic][])
14 changes: 12 additions & 2 deletions lib/rubocop/cop/lint/unreachable_loop.rb
Expand Up @@ -87,6 +87,7 @@ class UnreachableLoop < Base
include IgnoredPattern

MSG = 'This loop will have at most one iteration.'
CONTINUE_KEYWORDS = %i[next redo].freeze

def on_while(node)
check(node)
Expand Down Expand Up @@ -116,7 +117,10 @@ def check(node)
break_statement = statements.find { |statement| break_statement?(statement) }
return unless break_statement

add_offense(node) unless preceded_by_continue_statement?(break_statement)
unless preceded_by_continue_statement?(break_statement) ||
conditional_continue_keyword?(break_statement)
add_offense(node)
end
end

def statements(node)
Expand Down Expand Up @@ -177,9 +181,15 @@ def preceded_by_continue_statement?(break_statement)
break_statement.left_siblings.any? do |sibling|
next if sibling.loop_keyword? || loop_method?(sibling)

sibling.each_descendant(:next, :redo).any?
sibling.each_descendant(*CONTINUE_KEYWORDS).any?
end
end

def conditional_continue_keyword?(break_statement)
or_node = break_statement.each_descendant(:or).to_a.last

or_node && CONTINUE_KEYWORDS.include?(or_node.rhs.type)
end
end
end
end
Expand Down
25 changes: 25 additions & 0 deletions spec/rubocop/cop/lint/unreachable_loop_spec.rb
Expand Up @@ -225,4 +225,29 @@
end
RUBY
end

it 'does not register an offense when using `return do_something(value) || next` in a loop' do
expect_no_offenses(<<~RUBY)
[nil, nil, 42].each do |value|
return do_something(value) || next
end
RUBY
end

it 'does not register an offense when using `return do_something(value) || redo` in a loop' do
expect_no_offenses(<<~RUBY)
[nil, nil, 42].each do |value|
return do_something(value) || redo
end
RUBY
end

it 'registers an offense when using `return do_something(value) || break` in a loop' do
expect_offense(<<~RUBY)
[nil, nil, 42].each do |value|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This loop will have at most one iteration.
return do_something(value) || break
end
RUBY
end
end

0 comments on commit 0e1a312

Please sign in to comment.