Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix false negatives for Lint/UnreachableLoop #12841

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/fix_false_negatives_for_lint_unreachable_loop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12841](https://github.com/rubocop/rubocop/pull/12841): Fix false negatives for `Lint/UnreachableLoop` when using pattern matching. ([@koic][])
10 changes: 8 additions & 2 deletions lib/rubocop/cop/lint/unreachable_loop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def break_statement?(node)
break_statement && !preceded_by_continue_statement?(break_statement)
when :if
check_if(node)
when :case
when :case, :case_match
check_case(node)
else
false
Expand All @@ -178,7 +178,13 @@ def check_case(node)
return false unless else_branch
return false unless break_statement?(else_branch)

node.when_branches.all? { |branch| branch.body && break_statement?(branch.body) }
branches = if node.case_type?
node.when_branches
else
node.in_pattern_branches
end

branches.all? { |branch| branch.body && break_statement?(branch.body) }
end

def preceded_by_continue_statement?(break_statement)
Expand Down
55 changes: 55 additions & 0 deletions spec/rubocop/cop/lint/unreachable_loop_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,46 @@
end
RUBY
end

it 'registers an offense when using `case-in-else` with all break branches' do
expect_offense(<<~RUBY)
while x > 0
^^^^^^^^^^^ This loop will have at most one iteration.
case x
in 1
break
else
raise MyError
end
end
RUBY
end

it 'does not register an offense when using `case` match without `else`' do
expect_no_offenses(<<~RUBY)
while x > 0
case x
in 1
break
end
end
RUBY
end

it 'does not register an offense when using `case-in-else` and not all branches are breaking' do
expect_no_offenses(<<~RUBY)
while x > 0
case x
in 1
break
in 2
do_something
else
raise MyError
end
end
RUBY
end
end

context 'with preceding continue statements' do
Expand Down Expand Up @@ -130,6 +170,21 @@
end
RUBY
end

it 'does not register an offense when using `case-in-else` with all break branches' do
expect_no_offenses(<<~RUBY)
while x > 0
redo if x.odd?

case x
in 1
break
else
raise MyError
end
end
RUBY
end
end

context 'with an enumerator method' do
Expand Down