diff --git a/changelog/fix_error_for_rails_redundant_active_record_all_method_when_all_is_an_argument_for_ar_methods.md b/changelog/fix_error_for_rails_redundant_active_record_all_method_when_all_is_an_argument_for_ar_methods.md new file mode 100644 index 0000000000..876455528c --- /dev/null +++ b/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][]) diff --git a/lib/rubocop/cop/rails/redundant_active_record_all_method.rb b/lib/rubocop/cop/rails/redundant_active_record_all_method.rb index 8dfdf7cb14..3d7e171f1e 100644 --- a/lib/rubocop/cop/rails/redundant_active_record_all_method.rb +++ b/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 diff --git a/spec/rubocop/cop/rails/redundant_active_record_all_method_spec.rb b/spec/rubocop/cop/rails/redundant_active_record_all_method_spec.rb index 6484dcf3fc..c009fbcd3f 100644 --- a/spec/rubocop/cop/rails/redundant_active_record_all_method_spec.rb +++ b/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