Fix some issues in Lint/UnModifiedReduceAccumulator (#8949)

- Moved file to correct filename
- Made examples more descriptive
- Fixed not apply when reduce called with no argument or no block arguments
- Fixed false positives when the element is modified by another value

docs/modules/ROOT/pages/cops_lint.adoc

@@ -4324,6 +4324,12 @@ end
   acc + el * 2
 end
 
+# good, element is returned but modified using the accumulator
+values.reduce do |acc, el|
+  el << acc
+  el
+end
+
 # good, returns the accumulator instead of the index
 %w(a b c).reduce({}) do |acc, letter|
   acc[letter] = true
@@ -4338,7 +4344,8 @@ end
 
 # ignored as the return value cannot be determined
 enum.reduce do |acc, el|
-  x + y
+  x = foo(acc, el)
+  bar(x)
 end
 ----
 

lib/rubocop.rb

@@ -308,7 +308,6 @@
 require_relative 'rubocop/cop/lint/percent_symbol_array'
 require_relative 'rubocop/cop/lint/raise_exception'
 require_relative 'rubocop/cop/lint/rand_one'
-require_relative 'rubocop/cop/lint/reduce_accumulator'
 require_relative 'rubocop/cop/lint/redundant_cop_disable_directive'
 require_relative 'rubocop/cop/lint/redundant_cop_enable_directive'
 require_relative 'rubocop/cop/lint/redundant_require_statement'
@@ -340,6 +339,7 @@
 require_relative 'rubocop/cop/lint/trailing_comma_in_attribute_declaration'
 require_relative 'rubocop/cop/lint/underscore_prefixed_variable_name'
 require_relative 'rubocop/cop/lint/unified_integer'
+require_relative 'rubocop/cop/lint/unmodified_reduce_accumulator'
 require_relative 'rubocop/cop/lint/unreachable_code'
 require_relative 'rubocop/cop/lint/unreachable_loop'
 require_relative 'rubocop/cop/lint/unused_block_argument'

lib/rubocop/cop/lint/reduce_accumulator.rb → lib/rubocop/cop/lint/unmodified_reduce_accumulator.rb

@@ -36,6 +36,12 @@ module Lint
       #     acc + el * 2
       #   end
       #
+      #   # good, element is returned but modified using the accumulator
+      #   values.reduce do |acc, el|
+      #     el << acc
+      #     el
+      #   end
+      #
       #   # good, returns the accumulator instead of the index
       #   %w(a b c).reduce({}) do |acc, letter|
       #     acc[letter] = true
@@ -50,20 +56,30 @@ module Lint
       #
       #   # ignored as the return value cannot be determined
       #   enum.reduce do |acc, el|
-      #     x + y
+      #     x = foo(acc, el)
+      #     bar(x)
       #   end
       class UnmodifiedReduceAccumulator < Base
         MSG = 'Ensure the accumulator `%<accum>s` will be modified by `%<method>s`.'
         MSG_INDEX = 'Do not return an element of the accumulator in `%<method>s`.'
 
         def_node_matcher :reduce_with_block?, <<~PATTERN
-          (block (send _recv {:reduce :inject} !sym) ...)
+          (block (send _recv {:reduce :inject} ...) (args arg+) ...)
         PATTERN
 
         def_node_matcher :accumulator_index?, <<~PATTERN
           (send (lvar %1) {:[] :[]=} ...)
         PATTERN
 
+        def_node_search :element_modified?, <<~PATTERN
+          {
+            (send _receiver !{:[] :[]=} <`(lvar %1) `_ ...>)               # method(el, ...)
+            (send (lvar %1) _message <{ivar gvar cvar lvar send} ...>)     # el.method(...)
+            (lvasgn %1 _)                                                  # el = ...
+            (%RuboCop::AST::Node::SHORTHAND_ASSIGNMENTS (lvasgn %1) ... _) # el += ...
+          }
+        PATTERN
+
         def_node_matcher :lvar_used?, <<~PATTERN
           {
             (lvar %1)
@@ -75,12 +91,12 @@ class UnmodifiedReduceAccumulator < Base
         PATTERN
 
         def_node_search :expression_values, <<~PATTERN
-          `{
+          {
             (%RuboCop::AST::Node::VARIABLES $_)
             (%RuboCop::AST::Node::EQUALS_ASSIGNMENTS $_ ...)
             (send (%RuboCop::AST::Node::VARIABLES $_) :<< ...)
-            (send nil? $_)
-            (dstr (begin {(send nil? $_) (%RuboCop::AST::Node::VARIABLES $_)}))
+            $(send _ _)
+            (dstr (begin {(%RuboCop::AST::Node::VARIABLES $_)}))
             (%RuboCop::AST::Node::SHORTHAND_ASSIGNMENTS (%RuboCop::AST::Node::EQUALS_ASSIGNMENTS $_) ...)
           }
         PATTERN
@@ -117,7 +133,7 @@ def check_return_values(block_node)
 
           if (node = returned_accumulator_index(return_values, accumulator_name))
             add_offense(node, message: format(MSG_INDEX, message_opts))
-          elsif !returns_accumulator_anywhere?(return_values, accumulator_name)
+          elsif potential_offense?(return_values, block_node.body, element_name, accumulator_name)
             return_values.each do |return_val|
               unless acceptable_return?(return_val, element_name)
                 add_offense(return_val, message: format(MSG, message_opts))
@@ -137,6 +153,11 @@ def returned_accumulator_index(return_values, accumulator_name)
           return_values.detect { |val| accumulator_index?(val, accumulator_name) }
         end
 
+        def potential_offense?(return_values, block_body, element_name, accumulator_name)
+          !(element_modified?(block_body, element_name) ||
+            returns_accumulator_anywhere?(return_values, accumulator_name))
+        end
+
         # If the accumulator is used in any return value, the node is acceptable since
         # the accumulator has a chance to change each iteration
         def returns_accumulator_anywhere?(return_values, accumulator_name)