Merge pull request #1113 from masato-bkn/fix/rails_redundant_active_r…

…ecord_all_method_when_all_is_argument

[Fix: #1109] Fix error for `Rails/RedundantActiveRecordAllMethod` when `all` is an argument for AR methods

changelog/fix_error_for_rails_redundant_active_record_all_method_when_all_is_an_argument_for_ar_methods.md

@@ -0,0 +1 @@
+* [#1109](https://github.com/rubocop/rubocop-rails/issues/1109): Fix error for `Rails/RedundantActiveRecordAllMethod` when `all` is an argument for AR methods.([@masato-bkn][])

lib/rubocop/cop/rails/redundant_active_record_all_method.rb

@@ -124,19 +124,20 @@ class RedundantActiveRecordAllMethod < Base
           update_all
           where
           without
-        ].freeze
+        ].to_set.freeze
 
-        def on_send(node)
-          query_node = node.parent
+        def_node_matcher :followed_by_query_method?, <<~PATTERN
+          (send (send _ :all ...) QUERYING_METHODS ...)
+        PATTERN
 
-          return unless query_node&.send_type?
-          return unless QUERYING_METHODS.include?(query_node.method_name)
+        def on_send(node)
+          return unless followed_by_query_method?(node.parent)
           return if node.receiver.nil? && !inherit_active_record_base?(node)
 
           range_of_all_method = node.loc.selector
           add_offense(range_of_all_method) do |collector|
             collector.remove(range_of_all_method)
-            collector.remove(query_node.loc.dot)
+            collector.remove(node.parent.loc.dot)
           end
         end
       end

spec/rubocop/cop/rails/redundant_active_record_all_method_spec.rb

@@ -99,7 +99,7 @@
           update_all
           where
           without
-        ]
+        ].to_set
       )
     end
   end
@@ -235,6 +235,31 @@
         User.all.map(&:do_something)
       RUBY
     end
+
+    context 'when `all` is used as a method parameter' do
+      it 'does not register an offense when no method follows `all`' do
+        expect_no_offenses(<<~RUBY)
+          do_something(User.all)
+        RUBY
+      end
+
+      it 'registers an offense and corrects when `ActiveRecord::Querying::QUERYING_METHODS` follows `all`' do
+        expect_offense(<<~RUBY)
+          do_something(User.all.order(:created_at))
+                            ^^^ Redundant `all` detected.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          do_something(User.order(:created_at))
+        RUBY
+      end
+
+      it 'does not register an offense when method matches `ActiveRecord::Querying::QUERYING_METHODS`' do
+        expect_no_offenses(<<~RUBY)
+          sum(User.all)
+        RUBY
+      end
+    end
   end
 
   context 'with no receiver' do