deprecate, join, preload, eager load of instance dependent associations.

Closes #15024.

These operations happen before instances are created.
The current behavior is misleading and can result in broken behavior.

6 files changed, 56 insertions, 4 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 6b3bfc18e7..f40fe33bcd 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,12 @@
+*   Deprecate joining, eager loading and preloading of instance dependent
+    associations without replacement. These operations happen before instances
+    are created. The current behavior is unexpected and can result in broken
+    behavior.
+
+    Fixes #15024.
+
+    *Yves Senn*
+
 *   Fixed HABTM's CollectionAssociation size calculation.
 
     HABTM should fall back to using the normal CollectionAssociation's size
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index d66a79a137..af6dabb793 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -419,6 +419,10 @@ module ActiveRecord
     #     has_many :birthday_events, ->(user) { where starts_on: user.birthday }, class_name: 'Event'
     #   end
     #
+    # Note: Joining, eager loading and preloading of these associations is not fully possibly.
+    # These operations happen before instance creation. The scope will be called with a +nil+ argument.
+    # This can lead to unexpected behavior and is deprecated.
+    #
     # == Association callbacks
     #
     # Similar to the normal callbacks that hook into the life cycle of an Active Record object,
diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb
index b7dc037a65..5842be3a7b 100644
--- a/activerecord/lib/active_record/associations/join_dependency.rb
+++ b/activerecord/lib/active_record/associations/join_dependency.rb
@@ -215,6 +215,7 @@ module ActiveRecord
         associations.map do |name, right|
           reflection = find_reflection base_klass, name
           reflection.check_validity!
+          reflection.check_eager_loadable!
 
           if reflection.options[:polymorphic]
             raise EagerLoadPolymorphicError.new(reflection)
diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb
index 311684d886..20bd4947dc 100644
--- a/activerecord/lib/active_record/associations/preloader.rb
+++ b/activerecord/lib/active_record/associations/preloader.rb
@@ -175,6 +175,7 @@ module ActiveRecord
         if owners.first.association(reflection.name).loaded?
           return AlreadyLoaded
         end
+        reflection.check_preloadable!
 
         case reflection.macro
         when :has_many
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 7f4d77849a..0eec6774a0 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -278,6 +278,20 @@ module ActiveRecord
         end
       end
 
+      def check_preloadable!
+        return unless scope
+
+        if scope.arity > 0
+          ActiveSupport::Deprecation.warn <<-WARNING
+The association scope '#{name}' is instance dependent (the scope block takes an argument).
+Preloading happens before the individual instances are created. This means that there is no instance
+being passed to the association scope. This will most likely result in broken or incorrect behavior.
+Joining, Preloading and eager loading of these associations is deprecated and will be removed in the future.
+          WARNING
+        end
+      end
+      alias :check_eager_loadable! :check_preloadable!
+
       def join_id_for(owner) #:nodoc:
         key = (source_macro == :belongs_to) ? foreign_key : active_record_primary_key
         owner[key]
diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
index 7eaa5adc86..07903a3441 100644
--- a/activerecord/test/cases/associations/eager_test.rb
+++ b/activerecord/test/cases/associations/eager_test.rb
@@ -826,11 +826,15 @@ class EagerAssociationTest < ActiveRecord::TestCase
   end
 
   def test_preload_with_interpolation
-    post = Post.includes(:comments_with_interpolated_conditions).find(posts(:welcome).id)
-    assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions
+    assert_deprecated do
+      post = Post.includes(:comments_with_interpolated_conditions).find(posts(:welcome).id)
+      assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions
+    end
 
-    post = Post.joins(:comments_with_interpolated_conditions).find(posts(:welcome).id)
-    assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions
+    assert_deprecated do
+      post = Post.joins(:comments_with_interpolated_conditions).find(posts(:welcome).id)
+      assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions
+    end
   end
 
   def test_polymorphic_type_condition
@@ -1232,4 +1236,23 @@ class EagerAssociationTest < ActiveRecord::TestCase
       assert_equal 2, author.posts.size
     }
   end
+
+  test "include instance dependent associations is deprecated" do
+    message = "association scope 'posts_with_signature' is"
+    assert_deprecated message do
+      begin
+        Author.includes(:posts_with_signature).to_a
+      rescue NoMethodError
+        # it's expected that preloading of this association fails
+      end
+    end
+
+    assert_deprecated message do
+      Author.preload(:posts_with_signature).to_a rescue NoMethodError
+    end
+
+    assert_deprecated message do
+      Author.eager_load(:posts_with_signature).to_a
+    end
+  end
 end