Merge pull request rails#46633 from jonathanhefner/follow-up-46605

Fix association model class error message and error type

Author: jonathanhefner

activerecord/lib/active_record/reflection.rb

@@ -425,15 +425,17 @@ def compute_class(name)
 
         begin
           klass = active_record.send(:compute_type, name)
-
-          unless klass < ActiveRecord::Base
-            raise ArgumentError, compute_class_error_message(klass)
-          end
-
-          klass
         rescue NameError
-          raise ArgumentError, compute_class_error_message
+          message = "Missing model class #{name} for the #{active_record}##{self.name} association."
+          message += " You can specify a different model class with the :class_name option." unless options[:class_name]
+          raise NameError, message
+        end
+
+        unless klass < ActiveRecord::Base
+          raise ArgumentError, "The #{name} model class for the #{active_record}##{self.name} association is not an ActiveRecord::Base subclass."
         end
+
+        klass
       end
 
       attr_reader :type, :foreign_type
@@ -599,16 +601,6 @@ def extensions
       end
 
       private
-        def compute_class_error_message(klass = nil)
-          msg = +"Rails couldn't find a valid model for the #{name} association. "
-          if !options[:class_name]
-            msg << "Use the :class_name option on the association declaration to tell Rails which model to use."
-          else
-            msg << "Ensure the class provided to :class_name exists and is an ActiveRecord::Base subclass."
-          end
-          msg
-        end
-
         # Attempts to find the inverse association name automatically.
         # If it cannot find a suitable inverse association name, it returns
         # +nil+.

activerecord/test/cases/associations_test.rb

@@ -421,15 +421,15 @@ class ModelAssociatedToClassesThatDoNotExist < ActiveRecord::Base
   end
 
   def test_associations_raise_with_name_error_if_associated_to_classes_that_do_not_exist
-    assert_raises ArgumentError do
+    assert_raises NameError do
       ModelAssociatedToClassesThatDoNotExist.new.non_existent_has_one_class
     end
 
-    assert_raises ArgumentError do
+    assert_raises NameError do
       ModelAssociatedToClassesThatDoNotExist.new.non_existent_belongs_to_class
     end
 
-    assert_raises ArgumentError do
+    assert_raises NameError do
       ModelAssociatedToClassesThatDoNotExist.new.non_existent_has_many_classes
     end
   end

activerecord/test/cases/finder_test.rb

@@ -18,7 +18,6 @@
 require "models/customer"
 require "models/toy"
 require "models/matey"
-require "models/dog_lover"
 require "models/dog"
 require "models/car"
 require "models/tyre"

activerecord/test/cases/fixtures_test.rb

@@ -17,7 +17,6 @@
 require "models/computer"
 require "models/course"
 require "models/developer"
-require "models/dog_lover"
 require "models/dog"
 require "models/doubloon"
 require "models/essay"

activerecord/test/cases/nested_attributes_test.rb

@@ -2,7 +2,6 @@
 
 require "cases/helper"
 require "models/pirate"
-require "models/developer"
 require "models/ship"
 require "models/ship_part"
 require "models/bird"

activerecord/test/cases/reflection_test.rb

@@ -139,50 +139,40 @@ def test_irregular_reflection_class_name
   end
 
   def test_reflection_klass_not_found_with_no_class_name_option
-    [:account_invalid_no_class_name_option,
-     :info_invalids,
-    ].each do |rel|
-      expected = <<-MSG.squish
-        Rails couldn't find a valid model for the #{rel} association.
-        Use the :class_name option on the association declaration to tell Rails which model to use.
-      MSG
-
-      err = assert_raise(ArgumentError) do
-        UserWithInvalidRelation.reflect_on_association(rel).klass
-      end
-
-      assert_includes err.message, expected
+    error = assert_raise(NameError) do
+      UserWithInvalidRelation.reflect_on_association(:not_a_class).klass
     end
+
+    assert_match %r/missing/i, error.message
+    assert_match "NotAClass", error.message
+    assert_match "UserWithInvalidRelation#not_a_class", error.message
+    assert_match ":class_name", error.message
   end
 
   def test_reflection_klass_not_found_with_pointer_to_non_existent_class_name
-    expected = <<-MSG.squish
-      Rails couldn't find a valid model for the class_name_provided_not_a_class association.
-      Ensure the class provided to :class_name exists and is an ActiveRecord::Base subclass.
-    MSG
-
-    err = assert_raise(ArgumentError) do
+    error = assert_raise(NameError) do
       UserWithInvalidRelation.reflect_on_association(:class_name_provided_not_a_class).klass
     end
 
-    assert_includes err.message, expected
+    assert_match %r/missing/i, error.message
+    assert_match %r/\bNotAClass\b/, error.message
+    assert_match "UserWithInvalidRelation#class_name_provided_not_a_class", error.message
+    assert_no_match ":class_name", error.message
   end
 
   def test_reflection_klass_requires_ar_subclass
-    [:account_class_name,
-     :infos_class_name,
-     :infos_through_class_name,
+    [ :account_invalid,          # has_one, without :class_name
+      :account_class_name,       # has_one, with :class_name
+      :info_invalids,            # has_many through, without :class_name
+      :infos_class_name,         # has_many through, with :class_name
+      :infos_through_class_name, # has_many through other :class_name, with :class_name
     ].each do |rel|
-      expected = <<-MSG.squish
-        Rails couldn't find a valid model for the #{rel} association.
-        Ensure the class provided to :class_name exists and is an ActiveRecord::Base subclass.
-      MSG
-
-      err = assert_raise(ArgumentError) do
+      error = assert_raise(ArgumentError) do
         UserWithInvalidRelation.reflect_on_association(rel).klass
       end
 
-      assert_includes err.message, expected
+      assert_match "not an ActiveRecord::Base subclass", error.message
+      assert_match "UserWithInvalidRelation##{rel}", error.message
     end
   end
 
@@ -570,6 +560,11 @@ def test_reflect_on_association_accepts_strings
     end
   end
 
+  def test_automatic_inverse_suppresses_name_error_for_association
+    reflection = UserWithInvalidRelation.reflect_on_association(:not_a_class)
+    assert_not reflection.dup.has_inverse? # dup to prevent global memoization
+  end
+
   private
     def assert_reflection(klass, association, options)
       assert reflection = klass.reflect_on_association(association)

activerecord/test/models/user_with_invalid_relation.rb

@@ -1,12 +1,14 @@
 # frozen_string_literal: true
 
 class UserWithInvalidRelation < ActiveRecord::Base
-  has_one :account_invalid_no_class_name_option
-
-  has_one :account_class_name, class_name: "AccountInvalid"
+  has_one :not_a_class
 
   has_one :class_name_provided_not_a_class, class_name: "NotAClass"
 
+  has_one :account_invalid
+
+  has_one :account_class_name, class_name: "AccountInvalid"
+
   has_many :user_info_invalid
   has_many :info_invalids, through: :user_info_invalid