Merge pull request #28830 from kamipo/dont_regard_extension_block_as_scope

Fix `automatic_inverse_of` not to be disabled if extension block is given

6 files changed, 33 insertions, 40 deletions

diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb
index 7c69cd65ee..3b4b243148 100644
--- a/activerecord/lib/active_record/associations/builder/association.rb
+++ b/activerecord/lib/active_record/associations/builder/association.rb
@@ -27,40 +27,32 @@ module ActiveRecord::Associations::Builder # :nodoc:
                              "Please choose a different association name."
       end
 
-      extension = define_extensions model, name, &block
-      reflection = create_reflection model, name, scope, options, extension
+      reflection = create_reflection(model, name, scope, options, &block)
       define_accessors model, reflection
       define_callbacks model, reflection
       define_validations model, reflection
       reflection
     end
 
-    def self.create_reflection(model, name, scope, options, extension = nil)
+    def self.create_reflection(model, name, scope, options, &block)
       raise ArgumentError, "association names must be a Symbol" unless name.kind_of?(Symbol)
 
       validate_options(options)
 
-      scope = build_scope(scope, extension)
+      extension = define_extensions(model, name, &block)
+      options[:extend] = [*options[:extend], extension] if extension
+
+      scope = build_scope(scope)
 
       ActiveRecord::Reflection.create(macro, name, scope, options, model)
     end
 
-    def self.build_scope(scope, extension)
-      new_scope = scope
-
+    def self.build_scope(scope)
       if scope && scope.arity == 0
-        new_scope = proc { instance_exec(&scope) }
-      end
-
-      if extension
-        new_scope = wrap_scope new_scope, extension
+        proc { instance_exec(&scope) }
+      else
+        scope
       end
-
-      new_scope
-    end
-
-    def self.wrap_scope(scope, extension)
-      scope
     end
 
     def self.macro
diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb
index 5848cd9112..9fccfcce0c 100644
--- a/activerecord/lib/active_record/associations/builder/collection_association.rb
+++ b/activerecord/lib/active_record/associations/builder/collection_association.rb
@@ -66,17 +66,5 @@ module ActiveRecord::Associations::Builder # :nodoc:
         end
       CODE
     end
-
-    def self.wrap_scope(scope, mod)
-      if scope
-        if scope.arity > 0
-          proc { |owner| instance_exec(owner, &scope).extending(mod) }
-        else
-          proc { instance_exec(&scope).extending(mod) }
-        end
-      else
-        proc { extending(mod) }
-      end
-    end
   end
 end
diff --git a/activerecord/test/cases/associations/eager_load_nested_include_test.rb b/activerecord/test/cases/associations/eager_load_nested_include_test.rb
index 525ad3197a..849939de75 100644
--- a/activerecord/test/cases/associations/eager_load_nested_include_test.rb
+++ b/activerecord/test/cases/associations/eager_load_nested_include_test.rb
@@ -110,10 +110,10 @@ class EagerLoadNestedIncludeWithMissingDataTest < ActiveRecord::TestCase
   end
 
   teardown do
-    @davey_mcdave.destroy
-    @first_post.destroy
     @first_comment.destroy
     @first_categorization.destroy
+    @davey_mcdave.destroy
+    @first_post.destroy
   end
 
   def test_missing_data_in_a_nested_include_should_not_cause_errors_when_constructing_objects
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 6a7efe2121..32285f269a 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -2578,22 +2578,22 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   test "association with extend option" do
     post = posts(:welcome)
-    assert_equal "lifo",  post.comments_with_extend.author
-    assert_equal "hello", post.comments_with_extend.greeting
+    assert_equal "lifo", post.comments_with_extend.author
+    assert_equal "hello :)", post.comments_with_extend.greeting
   end
 
   test "association with extend option with multiple extensions" do
     post = posts(:welcome)
-    assert_equal "lifo",  post.comments_with_extend_2.author
-    assert_equal "hullo", post.comments_with_extend_2.greeting
+    assert_equal "lifo", post.comments_with_extend_2.author
+    assert_equal "hullo :)", post.comments_with_extend_2.greeting
   end
 
   test "extend option affects per association" do
     post = posts(:welcome)
-    assert_equal "lifo",  post.comments_with_extend.author
-    assert_equal "lifo",  post.comments_with_extend_2.author
-    assert_equal "hello", post.comments_with_extend.greeting
-    assert_equal "hullo", post.comments_with_extend_2.greeting
+    assert_equal "lifo", post.comments_with_extend.author
+    assert_equal "lifo", post.comments_with_extend_2.author
+    assert_equal "hello :)", post.comments_with_extend.greeting
+    assert_equal "hullo :)", post.comments_with_extend_2.greeting
   end
 
   test "delete record with complex joins" do
diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb
index da3a42e2b5..669e176dcb 100644
--- a/activerecord/test/cases/associations/inverse_associations_test.rb
+++ b/activerecord/test/cases/associations/inverse_associations_test.rb
@@ -8,6 +8,7 @@ require "models/zine"
 require "models/club"
 require "models/sponsor"
 require "models/rating"
+require "models/post"
 require "models/comment"
 require "models/car"
 require "models/bulb"
@@ -62,6 +63,14 @@ class AutomaticInverseFindingTests < ActiveRecord::TestCase
     assert_equal rating_reflection, comment_reflection.inverse_of, "The Comment reflection's inverse should be the Rating reflection"
   end
 
+  def test_has_many_and_belongs_to_should_find_inverse_automatically_for_extension_block
+    comment_reflection = Comment.reflect_on_association(:post)
+    post_reflection = Post.reflect_on_association(:comments)
+
+    assert_predicate post_reflection, :has_inverse?
+    assert_equal comment_reflection, post_reflection.inverse_of
+  end
+
   def test_has_many_and_belongs_to_should_find_inverse_automatically_for_sti
     author_reflection = Author.reflect_on_association(:posts)
     author_child_reflection = Author.reflect_on_association(:special_posts)
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index 395b534c63..c34968590f 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -11,6 +11,10 @@ class Post < ActiveRecord::Base
     def author
       "lifo"
     end
+
+    def greeting
+      super + " :)"
+    end
   end
 
   module NamedExtension2