From 0e1a312ff00bf91e99e501f5646288763a0f90dd Mon Sep 17 00:00:00 2001
From: Koichi ITO <koic.ito@gmail.com>
Date: Thu, 29 Apr 2021 09:48:58 +0900
Subject: [PATCH] [Fix #9746] Fix a false positive for `Lint/UnreachableLoop`

Fixes #9746.

This PR fixes a false positive for `Lint/UnreachableLoop`
when using conditional `next` in a loop.
---
 ...alse_positive_for_lint_unreachable_loop.md |  1 +
 lib/rubocop/cop/lint/unreachable_loop.rb      | 14 +++++++++--
 .../rubocop/cop/lint/unreachable_loop_spec.rb | 25 +++++++++++++++++++
 3 files changed, 38 insertions(+), 2 deletions(-)
 create mode 100644 changelog/fix_false_positive_for_lint_unreachable_loop.md

diff --git a/changelog/fix_false_positive_for_lint_unreachable_loop.md b/changelog/fix_false_positive_for_lint_unreachable_loop.md
new file mode 100644
index 00000000000..c0be208d4f6
--- /dev/null
+++ b/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][])
diff --git a/lib/rubocop/cop/lint/unreachable_loop.rb b/lib/rubocop/cop/lint/unreachable_loop.rb
index 1c5dd6c59fd..18ca07e0bc6 100644
--- a/lib/rubocop/cop/lint/unreachable_loop.rb
+++ b/lib/rubocop/cop/lint/unreachable_loop.rb
@@ -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)
@@ -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)
@@ -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
diff --git a/spec/rubocop/cop/lint/unreachable_loop_spec.rb b/spec/rubocop/cop/lint/unreachable_loop_spec.rb
index 9e97281e2d7..753c15177f1 100644
--- a/spec/rubocop/cop/lint/unreachable_loop_spec.rb
+++ b/spec/rubocop/cop/lint/unreachable_loop_spec.rb
@@ -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