Add new Style/RedundantEach cop

changelog/new_add_new_style_redundant_each_cop.md

@@ -0,0 +1 @@
+* [#11110](https://github.com/rubocop/rubocop/pull/11110): Add new `Style/RedundantEach` cop. ([@koic][])

config/default.yml

@@ -4696,6 +4696,12 @@ Style/RedundantConditional:
   Enabled: true
   VersionAdded: '0.50'
 
+Style/RedundantEach:
+  Description: 'Checks for redundant `each`.'
+  Enabled: pending
+  Safe: false
+  VersionAdded: '<<next>>'
+
 Style/RedundantException:
   Description: "Checks for an obsolete RuntimeException argument in raise/fail."
   StyleGuide: '#no-explicit-runtimeerror'

lib/rubocop.rb

@@ -540,6 +540,7 @@
 require_relative 'rubocop/cop/style/open_struct_use'
 require_relative 'rubocop/cop/style/operator_method_call'
 require_relative 'rubocop/cop/style/redundant_assignment'
+require_relative 'rubocop/cop/style/redundant_each'
 require_relative 'rubocop/cop/style/redundant_fetch_block'
 require_relative 'rubocop/cop/style/redundant_file_extension_in_require'
 require_relative 'rubocop/cop/style/redundant_initialize'

lib/rubocop/cop/layout/indentation_style.rb

@@ -90,7 +90,7 @@ def string_literal_ranges(ast)
           # which lines start inside a string literal?
           return [] if ast.nil?
 
-          ast.each_node(:str, :dstr).each_with_object(Set.new) do |str, ranges|
+          ast.each_node(:str, :dstr).with_object(Set.new) do |str, ranges|
             loc = str.location
 
             if str.heredoc?

lib/rubocop/cop/layout/line_continuation_spacing.rb

@@ -92,7 +92,7 @@ def string_literal_ranges(ast)
           # which lines start inside a string literal?
           return [] if ast.nil?
 
-          ast.each_node(:str, :dstr).each_with_object(Set.new) do |str, ranges|
+          ast.each_node(:str, :dstr).with_object(Set.new) do |str, ranges|
             loc = str.location
 
             if str.heredoc?

lib/rubocop/cop/style/redundant_each.rb

@@ -0,0 +1,111 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Style
+      # Checks for redundant `each`.
+      #
+      # @safety
+      #   This cop is unsafe, as it can produce false positives if the receiver
+      #   is not an `Enumerable`.
+      #
+      # @example
+      #
+      #   # bad
+      #   array.each.each { |v| do_something(v) }
+      #
+      #   # good
+      #   array.each { |v| do_something(v) }
+      #
+      #   # bad
+      #   array.each.each_with_index { |v, i| do_something(v, i) }
+      #
+      #   # good
+      #   array.each.with_index { |v, i| do_something(v, i) }
+      #   array.each_with_index { |v, i| do_something(v, i) }
+      #
+      #   # bad
+      #   array.each.each_with_object { |v, o| do_something(v, o) }
+      #
+      #   # good
+      #   array.each.with_object { |v, o| do_something(v, o) }
+      #   array.each_with_object { |v, o| do_something(v, o) }
+      #
+      class RedundantEach < Base
+        extend AutoCorrector
+
+        MSG = 'Remove redundant `each`.'
+        MSG_WITH_INDEX = 'Use `with_index` to remove redundant `each`.'
+        MSG_WITH_OBJECT = 'Use `with_object` to remove redundant `each`.'
+
+        RESTRICT_ON_SEND = %i[each each_with_index each_with_object].freeze
+
+        def on_send(node)
+          return unless (redundant_node = redundant_each_method(node))
+
+          range = range(node)
+
+          add_offense(range, message: message(node)) do |corrector|
+            case node.method_name
+            when :each
+              remove_redundant_each(corrector, range, redundant_node)
+            when :each_with_index
+              corrector.replace(node.loc.selector, 'with_index')
+            when :each_with_object
+              corrector.replace(node.loc.selector, 'with_object')
+            end
+          end
+        end
+
+        private
+
+        # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+        def redundant_each_method(node)
+          if node.method?(:each) && !node.parent.block_type?
+            ancestor_node = node.each_ancestor(:send).detect do |ancestor|
+              RESTRICT_ON_SEND.include?(ancestor.method_name) || ancestor.method?(:reverse_each)
+            end
+          end
+
+          ancestor_node || node.each_descendant(:send).detect do |descendant|
+            next if descendant.parent.block_type?
+
+            detected = descendant.method_name.to_s.start_with?('each_') unless node.method?(:each)
+
+            detected || descendant.method?(:reverse_each)
+          end
+        end
+        # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+
+        def range(node)
+          if node.method?(:each)
+            node.loc.dot.join(node.loc.selector)
+          else
+            node.loc.selector
+          end
+        end
+
+        def message(node)
+          case node.method_name
+          when :each
+            MSG
+          when :each_with_index
+            MSG_WITH_INDEX
+          when :each_with_object
+            MSG_WITH_OBJECT
+          end
+        end
+
+        def remove_redundant_each(corrector, range, redundant_node)
+          corrector.remove(range)
+
+          if redundant_node.method?(:each_with_index)
+            corrector.replace(redundant_node.loc.selector, 'each.with_index')
+          elsif redundant_node.method?(:each_with_object)
+            corrector.replace(redundant_node.loc.selector, 'each.with_object')
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/variable_force/variable_table.rb

@@ -109,7 +109,7 @@ def variable_exist?(name)
         end
 
         def accessible_variables
-          scope_stack.reverse_each.each_with_object([]) do |scope, variables|
+          scope_stack.reverse_each.with_object([]) do |scope, variables|
             variables.concat(scope.variables.values)
             break variables unless scope.node.block_type?
           end

spec/rubocop/cop/style/redundant_each_spec.rb

@@ -0,0 +1,184 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Style::RedundantEach, :config do
+  it 'registers an offense when using `each.each`' do
+    expect_offense(<<~RUBY)
+      array.each.each { |v| do_something(v) }
+           ^^^^^ Remove redundant `each`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      array.each { |v| do_something(v) }
+    RUBY
+  end
+
+  it 'registers an offense when using `each.each_with_index`' do
+    expect_offense(<<~RUBY)
+      array.each.each_with_index { |v| do_something(v) }
+           ^^^^^ Remove redundant `each`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      array.each.with_index { |v| do_something(v) }
+    RUBY
+  end
+
+  it 'registers an offense when using `each.each_with_object`' do
+    expect_offense(<<~RUBY)
+      array.each.each_with_object([]) { |v, o| do_something(v, o) }
+           ^^^^^ Remove redundant `each`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      array.each.with_object([]) { |v, o| do_something(v, o) }
+    RUBY
+  end
+
+  it 'registers an offense when using a method starting with `each_` with `each_with_index`' do
+    expect_offense(<<~RUBY)
+      context.each_child_node.each_with_index.any? do |node, index|
+                              ^^^^^^^^^^^^^^^ Use `with_index` to remove redundant `each`.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      context.each_child_node.with_index.any? do |node, index|
+      end
+    RUBY
+  end
+
+  it 'registers an offense when using a method starting with `each_` with `each_with_object`' do
+    expect_offense(<<~RUBY)
+      context.each_child_node.each_with_object([]) do |node, object|
+                              ^^^^^^^^^^^^^^^^ Use `with_object` to remove redundant `each`.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      context.each_child_node.with_object([]) do |node, object|
+      end
+    RUBY
+  end
+
+  it 'registers an offense when using `reverse_each.each`' do
+    expect_offense(<<~RUBY)
+      context.reverse_each.each { |i| do_something(i) }
+                          ^^^^^ Remove redundant `each`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      context.reverse_each { |i| do_something(i) }
+    RUBY
+  end
+
+  it 'registers an offense when using `each.reverse_each`' do
+    expect_offense(<<~RUBY)
+      context.each.reverse_each { |i| do_something(i) }
+             ^^^^^ Remove redundant `each`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      context.reverse_each { |i| do_something(i) }
+    RUBY
+  end
+
+  it 'registers an offense when using `reverse_each.each_with_index`' do
+    expect_offense(<<~RUBY)
+      context.reverse_each.each_with_index.any? do |node, index|
+                           ^^^^^^^^^^^^^^^ Use `with_index` to remove redundant `each`.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      context.reverse_each.with_index.any? do |node, index|
+      end
+    RUBY
+  end
+
+  it 'registers an offense when using `reverse_each.each_with_object`' do
+    expect_offense(<<~RUBY)
+      scope_stack.reverse_each.each_with_object([]) do |scope, variables|
+                               ^^^^^^^^^^^^^^^^ Use `with_object` to remove redundant `each`.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      scope_stack.reverse_each.with_object([]) do |scope, variables|
+      end
+    RUBY
+  end
+
+  it 'does not register an offense when using only single `each`' do
+    expect_no_offenses(<<~RUBY)
+      array.each { |v| do_something(v) }
+    RUBY
+  end
+
+  it 'does not register an offense when using `each.with_index`' do
+    expect_no_offenses(<<~RUBY)
+      array.each.with_index { |v, i| do_something(v, i) }
+    RUBY
+  end
+
+  it 'does not register an offense when using `each_with_index`' do
+    expect_no_offenses(<<~RUBY)
+      array.each_with_index { |v, i| do_something(v, i) }
+    RUBY
+  end
+
+  it 'does not register an offense when using `each.with_object`' do
+    expect_no_offenses(<<~RUBY)
+      array.each.with_object { |v, o| do_something(v, o) }
+    RUBY
+  end
+
+  it 'does not register an offense when using `each_with_object`' do
+    expect_no_offenses(<<~RUBY)
+      array.each_with_object { |v, o| do_something(v, o) }
+    RUBY
+  end
+
+  it 'does not register an offense when using `each_with_index.reverse_each`' do
+    expect_no_offenses(<<~RUBY)
+      array.each_with_index.reverse_each do |value, index|
+      end
+    RUBY
+  end
+
+  it 'does not register an offense when using `each_ancestor.each`' do
+    expect_no_offenses(<<~RUBY)
+      node.each_ancestor(:def, :defs, :block).each do |ancestor|
+      end
+    RUBY
+  end
+
+  it 'does not register an offense when using `reverse_each {}.each {}`' do
+    expect_no_offenses(<<~RUBY)
+      array.reverse_each { |i| foo(i) }.each { |i| bar(i) }
+    RUBY
+  end
+
+  it 'does not register an offense when using `each {}.reverse_each {}`' do
+    expect_no_offenses(<<~RUBY)
+      array.each { |i| foo(i) }.reverse_each { |i| bar(i) }
+    RUBY
+  end
+
+  it 'does not register an offense when using `each {}.each_with_index {}`' do
+    expect_no_offenses(<<~RUBY)
+      array.each { |i| foo(i) }.each_with_index { |i| bar(i) }
+    RUBY
+  end
+
+  it 'does not register an offense when using `each {}.each_with_object([]) {}`' do
+    expect_no_offenses(<<~RUBY)
+      array.each { |i| foo(i) }.each_with_object([]) { |i| bar(i) }
+    RUBY
+  end
+
+  it 'does not register an offense when using `each_foo {}.each_with_object([]) {}`' do
+    expect_no_offenses(<<~RUBY)
+      array.each_foo { |i| foo(i) }.each_with_object([]) { |i| bar(i) }
+    RUBY
+  end
+end