Use a whitelist to delegate methods to array

2 files changed, 38 insertions, 75 deletions

diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb
index 1e15bddcdf..af6be24351 100644
--- a/activerecord/lib/active_record/relation/delegation.rb
+++ b/activerecord/lib/active_record/relation/delegation.rb
@@ -36,7 +36,11 @@ module ActiveRecord
     # may vary depending on the klass of a relation, so we create a subclass of Relation
     # for each different klass, and the delegations are compiled into that subclass only.
 
-    delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, :to => :to_a
+    delegate :&, :+, :[], :all?, :collect, :detect, :each, :each_cons,
+             :each_with_index, :flat_map, :group_by, :include?, :length,
+             :map, :none?, :one?, :reverse, :sample, :second, :sort, :sort_by,
+             :to_ary, :to_set, :to_xml, :to_yaml, :to => :to_a
+
     delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key,
              :connection, :columns_hash, :to => :klass
 
@@ -84,9 +88,6 @@ module ActiveRecord
         if @klass.respond_to?(method)
           self.class.delegate_to_scoped_klass(method)
           scoping { @klass.send(method, *args, &block) }
-        elsif array_delegable?(method)
-          self.class.delegate method, :to => :to_a
-          to_a.send(method, *args, &block)
         elsif arel.respond_to?(method)
           self.class.delegate method, :to => :arel
           arel.send(method, *args, &block)
@@ -109,28 +110,15 @@ module ActiveRecord
     end
 
     def respond_to?(method, include_private = false)
-      super || array_delegable?(method) ||
-        @klass.respond_to?(method, include_private) ||
+      super || @klass.respond_to?(method, include_private) ||
         arel.respond_to?(method, include_private)
     end
 
     protected
 
-    def array_delegable?(method)
-      defined = Array.method_defined?(method)
-      if defined && method.to_s.ends_with?('!')
-        ActiveSupport::Deprecation.warn(
-          "Association will no longer delegate #{method} to #to_a as of Rails 4.2. You instead must first call #to_a on the association to expose the array to be acted on."
-        )
-      end
-      defined
-    end
-
     def method_missing(method, *args, &block)
       if @klass.respond_to?(method)
         scoping { @klass.send(method, *args, &block) }
-      elsif array_delegable?(method)
-        to_a.send(method, *args, &block)
       elsif arel.respond_to?(method)
         arel.send(method, *args, &block)
       else
diff --git a/activerecord/test/cases/relation/delegation_test.rb b/activerecord/test/cases/relation/delegation_test.rb
index 13677797cf..c5ad1e8976 100644
--- a/activerecord/test/cases/relation/delegation_test.rb
+++ b/activerecord/test/cases/relation/delegation_test.rb
@@ -6,22 +6,21 @@ module ActiveRecord
   class DelegationTest < ActiveRecord::TestCase
     fixtures :posts
 
-    def assert_responds(target, method)
-      assert target.respond_to?(method)
-      assert_nothing_raised do
-        method_arity = target.to_a.method(method).arity
+    def call_method(target, method)
+      method_arity = target.to_a.method(method).arity
 
-        if method_arity.zero?
+      if method_arity.zero?
+        target.send(method)
+      elsif method_arity < 0
+        if method == :shuffle!
           target.send(method)
-        elsif method_arity < 0
-          if method == :shuffle!
-            target.send(method)
-          else
-            target.send(method, 1)
-          end
         else
-          raise NotImplementedError
+          target.send(method, 1)
         end
+       elsif method_arity == 1
+        target.send(method, 1)
+      else
+        raise NotImplementedError
       end
     end
   end
@@ -31,31 +30,20 @@ module ActiveRecord
       Post.first.comments
     end
 
-    [:map, :collect].each do |method|
-      test "##{method} is delegated" do
-        assert_responds(target, method)
-        assert_equal(target.pluck(:body), target.send(method) {|post| post.body })
-      end
-
-      test "##{method}! is not delegated" do
-        assert_deprecated do
-          assert_responds(target, "#{method}!")
-        end
+    [:&, :+, :[], :all?, :collect, :detect, :each, :each_cons,
+     :each_with_index, :flat_map, :group_by, :include?, :length,
+     :map, :none?, :one?, :reverse, :sample, :second, :sort, :sort_by,
+     :to_ary, :to_set, :to_xml, :to_yaml].each do |method|
+      test "association delegates #{method} to Array" do
+        assert_respond_to target, method
       end
     end
 
     [:compact!, :flatten!, :reject!, :reverse!, :rotate!,
-      :shuffle!, :slice!, :sort!, :sort_by!].each do |method|
-      test "##{method} delegation is deprecated" do
-        assert_deprecated do
-          assert_responds(target, method)
-        end
-      end
-    end
-
-    [:select!, :uniq!].each do |method|
-      test "##{method} is implemented" do
-        assert_responds(target, method)
+     :shuffle!, :slice!, :sort!, :sort_by!, :delete_if,
+     :keep_if, :pop, :shift, :delete_at, :compact].each do |method|
+      test "#{method} is not delegated to Array" do
+        assert_raises(NoMethodError) { call_method(target, method) }
       end
     end
   end
@@ -64,36 +52,23 @@ module ActiveRecord
     fixtures :comments
 
     def target
-      Comment.where(body: 'Normal type')
+      Comment.all
     end
 
-    [:map, :collect].each do |method|
-      test "##{method} is delegated" do
-        assert_responds(target, method)
-        assert_equal(target.pluck(:body), target.send(method) {|post| post.body })
-      end
-
-      test "##{method}! is not delegated" do
-        assert_deprecated do
-          assert_responds(target, "#{method}!")
-        end
+    [:&, :+, :[], :all?, :collect, :detect, :each, :each_cons,
+     :each_with_index, :flat_map, :group_by, :include?, :length,
+     :map, :none?, :one?, :reverse, :sample, :second, :sort, :sort_by,
+     :to_ary, :to_set, :to_xml, :to_yaml].each do |method|
+      test "relation delegates #{method} to Array" do
+        assert_respond_to target, method
       end
     end
 
     [:compact!, :flatten!, :reject!, :reverse!, :rotate!,
-      :shuffle!, :slice!, :sort!, :sort_by!].each do |method|
-      test "##{method} delegation is deprecated" do
-        assert_deprecated do
-          assert_responds(target, method)
-        end
-      end
-    end
-
-    [:select!, :uniq!].each do |method|
-      test "##{method} triggers an immutable error" do
-        assert_raises ActiveRecord::ImmutableRelation do
-          assert_responds(target, method)
-        end
+     :shuffle!, :slice!, :sort!, :sort_by!, :delete_if,
+     :keep_if, :pop, :shift, :delete_at, :compact].each do |method|
+      test "#{method} is not delegated to Array" do
+        assert_raises(NoMethodError) { call_method(target, method) }
       end
     end
   end