Add new Style/CombinableLoops cop

CHANGELOG.md

@@ -7,6 +7,7 @@
 * [#8451](https://github.com/rubocop-hq/rubocop/issues/8451): Add new `Style/RedundantSelfAssignment` cop. ([@fatkodima][])
 * [#8390](https://github.com/rubocop-hq/rubocop/pull/8390): Add new `Style/SoleNestedConditional` cop. ([@fatkodima][])
 * [#8562](https://github.com/rubocop-hq/rubocop/pull/8562): Add new `Style/KeywordParametersOrder` cop. ([@fatkodima][])
+* [#8486](https://github.com/rubocop-hq/rubocop/pull/8486): Add new `Style/CombinableLoops` cop. ([@fatkodima][])
 * [#8381](https://github.com/rubocop-hq/rubocop/pull/8381): Add new `Style/ClassMethodsDefinitions` cop. ([@fatkodima][])
 * [#8474](https://github.com/rubocop-hq/rubocop/pull/8474): Add new `Lint/DuplicateRequire` cop. ([@fatkodima][])
 * [#8472](https://github.com/rubocop-hq/rubocop/issues/8472): Add new `Lint/UselessMethodDefinition` cop. ([@fatkodima][])

config/default.yml

@@ -2721,6 +2721,14 @@ Style/ColonMethodDefinition:
   Enabled: true
   VersionAdded: '0.52'
 
+Style/CombinableLoops:
+  Description: >-
+                  Checks for places where multiple consecutive loops over the same data
+                  can be combined into a single loop.
+  Enabled: pending
+  Safe: false
+  VersionAdded: '0.90'
+
 Style/CommandLiteral:
   Description: 'Use `` or %x around command literals.'
   StyleGuide: '#percent-x'

docs/modules/ROOT/pages/cops.adoc

@@ -350,6 +350,7 @@ In the following section you find all available cops:
 * xref:cops_style.adoc#stylecollectionmethods[Style/CollectionMethods]
 * xref:cops_style.adoc#stylecolonmethodcall[Style/ColonMethodCall]
 * xref:cops_style.adoc#stylecolonmethoddefinition[Style/ColonMethodDefinition]
+* xref:cops_style.adoc#stylecombinableloops[Style/CombinableLoops]
 * xref:cops_style.adoc#stylecommandliteral[Style/CommandLiteral]
 * xref:cops_style.adoc#stylecommentannotation[Style/CommentAnnotation]
 * xref:cops_style.adoc#stylecommentedkeyword[Style/CommentedKeyword]

docs/modules/ROOT/pages/cops_style.adoc

@@ -1374,6 +1374,68 @@ end
 
 * https://rubystyle.guide#colon-method-definition
 
+== Style/CombinableLoops
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Pending
+| No
+| No
+| 0.90
+| -
+|===
+
+This cop checks for places where multiple consecutive loops over the same data
+can be combined into a single loop. It is very likely that combining them
+will make the code more efficient and more concise.
+
+It is marked as unsafe, because the first loop might modify
+a state that the second loop depends on; these two aren't combinable.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+def method
+  items.each do |item|
+    do_something(item)
+  end
+
+  items.each do |item|
+    do_something_else(item)
+  end
+end
+
+# good
+def method
+  items.each do |item|
+    do_something(item)
+    do_something_else(item)
+  end
+end
+
+# bad
+def method
+  for item in items do
+    do_something(item)
+  end
+
+  for item in items do
+    do_something_else(item)
+  end
+end
+
+# good
+def method
+  for item in items do
+    do_something(item)
+    do_something_else(item)
+  end
+end
+----
+
 == Style/CommandLiteral
 
 |===

lib/rubocop.rb

@@ -395,6 +395,7 @@
 require_relative 'rubocop/cop/style/collection_methods'
 require_relative 'rubocop/cop/style/colon_method_call'
 require_relative 'rubocop/cop/style/colon_method_definition'
+require_relative 'rubocop/cop/style/combinable_loops'
 require_relative 'rubocop/cop/style/command_literal'
 require_relative 'rubocop/cop/style/comment_annotation'
 require_relative 'rubocop/cop/style/commented_keyword'

lib/rubocop/cop/style/combinable_loops.rb

@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Style
+      # This cop checks for places where multiple consecutive loops over the same data
+      # can be combined into a single loop. It is very likely that combining them
+      # will make the code more efficient and more concise.
+      #
+      # It is marked as unsafe, because the first loop might modify
+      # a state that the second loop depends on; these two aren't combinable.
+      #
+      # @example
+      #   # bad
+      #   def method
+      #     items.each do |item|
+      #       do_something(item)
+      #     end
+      #
+      #     items.each do |item|
+      #       do_something_else(item)
+      #     end
+      #   end
+      #
+      #   # good
+      #   def method
+      #     items.each do |item|
+      #       do_something(item)
+      #       do_something_else(item)
+      #     end
+      #   end
+      #
+      #   # bad
+      #   def method
+      #     for item in items do
+      #       do_something(item)
+      #     end
+      #
+      #     for item in items do
+      #       do_something_else(item)
+      #     end
+      #   end
+      #
+      #   # good
+      #   def method
+      #     for item in items do
+      #       do_something(item)
+      #       do_something_else(item)
+      #     end
+      #   end
+      #
+      class CombinableLoops < Base
+        MSG = 'Combine this loop with the previous loop.'
+
+        def on_block(node)
+          return unless collection_looping_method?(node)
+
+          sibling = left_sibling_of(node)
+          add_offense(node) if same_collection_looping?(node, sibling)
+        end
+
+        def on_for(node)
+          sibling = left_sibling_of(node)
+          add_offense(node) if sibling&.for_type? && node.collection == sibling.collection
+        end
+
+        private
+
+        def collection_looping_method?(node)
+          method_name = node.send_node.method_name
+          method_name.match?(/^each/) || method_name.match?(/_each$/)
+        end
+
+        def left_sibling_of(node)
+          return unless node.parent&.begin_type?
+
+          index = node.sibling_index - 1
+          node.parent.children[index] if index >= 0
+        end
+
+        def same_collection_looping?(node, sibling)
+          sibling&.block_type? &&
+            sibling.send_node.method?(node.method_name) &&
+            sibling.send_node.receiver == node.send_node.receiver
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/lint/void_spec.rb

@@ -15,18 +15,14 @@
         a %{op} b
       RUBY
     end
-  end
 
-  described_class::BINARY_OPERATORS.each do |op|
     it "accepts void op #{op} if on last line" do
       expect_no_offenses(<<~RUBY)
         something
         a #{op} b
       RUBY
     end
-  end
 
-  described_class::BINARY_OPERATORS.each do |op|
     it "accepts void op #{op} by itself without a begin block" do
       expect_no_offenses("a #{op} b")
     end
@@ -67,9 +63,7 @@
         #{op}b
       RUBY
     end
-  end
 
-  unary_operators.each do |op|
     it "accepts void unary op #{op} by itself without a begin block" do
       expect_no_offenses("#{op}b")
     end

spec/rubocop/cop/style/combinable_loops_spec.rb

@@ -0,0 +1,94 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Style::CombinableLoops do
+  subject(:cop) { described_class.new }
+
+  context 'when looping method' do
+    it 'registers an offense when looping over the same data as previous loop' do
+      expect_offense(<<~RUBY)
+        items.each { |item| do_something(item) }
+        items.each { |item| do_something_else(item, arg) }
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Combine this loop with the previous loop.
+
+        items.each_with_index { |item| do_something(item) }
+        items.each_with_index { |item| do_something_else(item, arg) }
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Combine this loop with the previous loop.
+
+        items.reverse_each { |item| do_something(item) }
+        items.reverse_each { |item| do_something_else(item, arg) }
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Combine this loop with the previous loop.
+      RUBY
+    end
+
+    it 'does not register an offense when the same loops are interleaved with some code' do
+      expect_no_offenses(<<~RUBY)
+        items.each { |item| do_something(item) }
+
+        some_code
+
+        items.each { |item| do_something_else(item, arg) }
+      RUBY
+    end
+
+    it 'does not register an offense when the same loop method is used over different collections' do
+      expect_no_offenses(<<~RUBY)
+        items.each { |item| do_something(item) }
+        bars.each { |bar| do_something(bar) }
+      RUBY
+    end
+
+    it 'does not register an offense when different loop methods are used over the same collection' do
+      expect_no_offenses(<<~RUBY)
+        items.reverse_each { |item| do_something(item) }
+        items.each { |item| do_something(item) }
+      RUBY
+    end
+
+    it 'does not register an offense when each branch contains the same single loop over the same collection' do
+      expect_no_offenses(<<~RUBY)
+        if condition
+          items.each { |item| do_something(item) }
+        else
+          items.each { |item| do_something_else(item, arg) }
+        end
+      RUBY
+    end
+  end
+
+  context 'when for loop' do
+    it 'registers an offense when looping over the same data as previous loop' do
+      expect_offense(<<~RUBY)
+        for item in items do do_something(item) end
+        for item in items do do_something_else(item, arg) end
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Combine this loop with the previous loop.
+      RUBY
+    end
+
+    it 'does not register an offense when the same loops are interleaved with some code' do
+      expect_no_offenses(<<~RUBY)
+        for item in items do do_something(item) end
+
+        some_code
+
+        for item in items do do_something_else(item, arg) end
+      RUBY
+    end
+
+    it 'does not register an offense when the same loop method is used over different collections' do
+      expect_no_offenses(<<~RUBY)
+        for item in items do do_something(item) end
+        for foo in foos do do_something(foo) end
+      RUBY
+    end
+
+    it 'does not register an offense when each branch contains the same single loop over the same collection' do
+      expect_no_offenses(<<~RUBY)
+        if condition
+          for item in items do do_something(item) end
+        else
+          for item in items do do_something_else(item, arg) end
+        end
+      RUBY
+    end
+  end
+end