Merge pull request #27485 from cih/hmt-warning

Warn when has_many through is defined before through association

5 files changed, 38 insertions, 0 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 05e498575c..97af75546d 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Raise error when has_many through is defined before through association
+
+    Fixes #26834
+
+    *Chris Holmes*
+
 *   Deprecate passing `name` to `indexes`.
 
     *Ryuta Kamizono*
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 1db5fc0dd1..c05a6c87df 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -97,6 +97,16 @@ module ActiveRecord
     end
   end
 
+  class HasManyThroughOrderError < ActiveRecordError #:nodoc:
+    def initialize(owner_class_name = nil, reflection = nil, through_reflection = nil)
+      if owner_class_name && reflection && through_reflection
+        super("Cannot have a has_many :through association '#{owner_class_name}##{reflection.name}' which goes through '#{owner_class_name}##{through_reflection.name}' before the through association is defined.")
+      else
+        super("Cannot have a has_many :through association before the through association is defined.")
+      end
+    end
+  end
+
   class ThroughCantAssociateThroughHasOneOrManyReflection < ActiveRecordError #:nodoc:
     def initialize(owner = nil, reflection = nil)
       if owner && reflection
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index f3e81ee1e2..a0016f0735 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -931,6 +931,14 @@ module ActiveRecord
           raise HasOneThroughCantAssociateThroughCollection.new(active_record.name, self, through_reflection)
         end
 
+        if parent_reflection.nil?
+          reflections = active_record.reflections.keys.map(&:to_sym)
+
+          if reflections.index(through_reflection.name) > reflections.index(name)
+            raise HasManyThroughOrderError.new(active_record.name, self, through_reflection)
+          end
+        end
+
         check_validity_of_inverse!
       end
 
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 47c6480a8e..ac005b7010 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -1231,4 +1231,12 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   ensure
     TenantMembership.current_member = nil
   end
+
+  def test_incorrectly_ordered_through_associations
+    assert_raises(ActiveRecord::HasManyThroughOrderError) do
+      DeveloperWithIncorrectlyOrderedHasManyThrough.create(
+        companies: [Company.create]
+      )
+    end
+  end
 end
diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb
index ea4f719517..654830ba11 100644
--- a/activerecord/test/models/developer.rb
+++ b/activerecord/test/models/developer.rb
@@ -260,3 +260,9 @@ class CachedDeveloper < ActiveRecord::Base
   self.table_name = "developers"
   self.cache_timestamp_format = :number
 end
+
+class DeveloperWithIncorrectlyOrderedHasManyThrough < ActiveRecord::Base
+  self.table_name = "developers"
+  has_many :companies, through: :contracts
+  has_many :contracts, foreign_key: :developer_id
+end