Merge pull request #1171 from masato-bkn/restrict-offense-to-model-re…

…ceiver

Change `Rails/RedundantActiveRecordAllMethod` to ignore `delete_all` and `destroy_all` when receiver is an association

changelog/change_rails_redundant_active_record_all_method.md

@@ -0,0 +1 @@
+* [#1171](https://github.com/rubocop/rubocop-rails/pull/1171): Change `Rails/RedundantActiveRecordAllMethod` to ignore `delete_all` and `destroy_all` when receiver is an association. ([@masato-bkn][])

lib/rubocop/cop/rails/redundant_active_record_all_method.rb

@@ -5,6 +5,13 @@ module Cop
     module Rails
       # Detect redundant `all` used as a receiver for Active Record query methods.
       #
+      # NOTE: For the methods `delete_all` and `destroy_all`,
+      # this cop will only check cases where the receiver is a model.
+      # It will ignore cases where the receiver is an association (e.g., `user.articles.all.delete_all`).
+      # This is because omitting `all` from an association changes the methods
+      # from `ActiveRecord::Relation` to `ActiveRecord::Associations::CollectionProxy`,
+      # which can affect their behavior.
+      #
       # @safety
       #   This cop is unsafe for autocorrection if the receiver for `all` is not an Active Record object.
       #
@@ -144,13 +151,15 @@ class RedundantActiveRecordAllMethod < Base
         ].to_set.freeze
 
         POSSIBLE_ENUMERABLE_BLOCK_METHODS = %i[any? count find none? one? select sum].freeze
+        SENSITIVE_METHODS_ON_ASSOCIATION = %i[delete_all destroy_all].freeze
 
         def_node_matcher :followed_by_query_method?, <<~PATTERN
           (send (send _ :all) QUERYING_METHODS ...)
         PATTERN
 
         def on_send(node)
-          return if !followed_by_query_method?(node.parent) || possible_enumerable_block_method?(node)
+          return unless followed_by_query_method?(node.parent)
+          return if possible_enumerable_block_method?(node) || sensitive_association_method?(node)
           return if node.receiver ? allowed_receiver?(node.receiver) : !inherit_active_record_base?(node)
 
           range_of_all_method = offense_range(node)
@@ -169,6 +178,10 @@ def possible_enumerable_block_method?(node)
           parent.parent&.block_type? || parent.parent&.numblock_type? || parent.first_argument&.block_pass_type?
         end
 
+        def sensitive_association_method?(node)
+          !node.receiver&.const_type? && SENSITIVE_METHODS_ON_ASSOCIATION.include?(node.parent.method_name)
+        end
+
         def offense_range(node)
           range_between(node.loc.selector.begin_pos, node.source_range.end_pos)
         end

spec/rubocop/cop/rails/redundant_active_record_all_method_spec.rb

@@ -421,6 +421,36 @@ class User < ::ActiveRecord::Base
     end
   end
 
+  described_class::SENSITIVE_METHODS_ON_ASSOCIATION.each do |method|
+    context "using `#{method}`" do
+      it "registers an offense when using `#{method}` and the receiver for `all` is a model" do
+        expect_offense(<<~RUBY)
+          User.all.#{method}
+               ^^^ Redundant `all` detected.
+        RUBY
+      end
+
+      it "does not register an offense when using `#{method}` and the receiver for `all` is an association" do
+        expect_no_offenses(<<~RUBY)
+          user.articles.all.#{method}
+        RUBY
+      end
+
+      it "does not register an offense when using `#{method}` and the receiver for `all` is a relation" do
+        expect_no_offenses(<<~RUBY)
+          users = User.all
+          users.all.#{method}
+        RUBY
+      end
+
+      it "does not register an offense when using `#{method}` and `all` has no receiver" do
+        expect_no_offenses(<<~RUBY)
+          all.#{method}
+        RUBY
+      end
+    end
+  end
+
   context 'with `AllowedReceivers` config' do
     let(:cop_config) do
       { 'AllowedReceivers' => %w[ActionMailer::Preview ActiveSupport::TimeZone] }