Merge pull request rails#44471 from postmodern/better_activerecord_enum_assertions

Better ActiveRecord enum assertions (issue rails#44457)

Author: byroot

activerecord/lib/active_record/enum.rb

@@ -267,15 +267,29 @@ def _enum_methods_module
       end
 
       def assert_valid_enum_definition_values(values)
-        unless values.is_a?(Hash) || values.all?(Symbol) || values.all?(String)
-          error_message = <<~MSG
-            Enum values #{values} must be either a hash, an array of symbols, or an array of strings.
-          MSG
-          raise ArgumentError, error_message
-        end
+        case values
+        when Hash
+          if values.empty?
+            raise ArgumentError, "Enum values #{values} must not be empty."
+          end
+
+          if values.keys.any?(&:blank?)
+            raise ArgumentError, "Enum values #{values} must not contain a blank name."
+          end
+        when Array
+          if values.empty?
+            raise ArgumentError, "Enum values #{values} must not be empty."
+          end
 
-        if values.is_a?(Hash) && values.keys.any?(&:blank?) || values.is_a?(Array) && values.any?(&:blank?)
-          raise ArgumentError, "Enum label name must not be blank."
+          unless values.all?(Symbol) || values.all?(String)
+            raise ArgumentError, "Enum values #{values} must only contain symbols or strings."
+          end
+
+          if values.any?(&:blank?)
+            raise ArgumentError, "Enum values #{values} must not contain a blank name."
+          end
+        else
+          raise ArgumentError, "Enum values #{values} must be either a non-empty hash or an array."
         end
       end
 

activerecord/test/cases/enum_test.rb

@@ -401,14 +401,41 @@ class EnumTest < ActiveRecord::TestCase
   end
 
   test "invalid definition values raise an ArgumentError" do
+    e = assert_raises(ArgumentError) do
+      Class.new(ActiveRecord::Base) do
+        self.table_name = "books"
+        enum :status
+      end
+    end
+
+    assert_match(/must not be empty\.$/, e.message)
+
+    e = assert_raises(ArgumentError) do
+      Class.new(ActiveRecord::Base) do
+        self.table_name = "books"
+        enum :status, {}
+      end
+    end
+
+    assert_match(/must not be empty\.$/, e.message)
+
+    e = assert_raises(ArgumentError) do
+      Class.new(ActiveRecord::Base) do
+        self.table_name = "books"
+        enum :status, []
+      end
+    end
+
+    assert_match(/must not be empty\.$/, e.message)
+
     e = assert_raises(ArgumentError) do
       Class.new(ActiveRecord::Base) do
         self.table_name = "books"
         enum status: [proposed: 1, written: 2, published: 3]
       end
     end
 
-    assert_match(/must be either a hash, an array of symbols, or an array of strings./, e.message)
+    assert_match(/must only contain symbols or strings\.$/, e.message)
 
     e = assert_raises(ArgumentError) do
       Class.new(ActiveRecord::Base) do
@@ -417,7 +444,7 @@ class EnumTest < ActiveRecord::TestCase
       end
     end
 
-    assert_match(/Enum label name must not be blank/, e.message)
+    assert_match(/must not contain a blank name\.$/, e.message)
 
     e = assert_raises(ArgumentError) do
       Class.new(ActiveRecord::Base) do
@@ -426,7 +453,16 @@ class EnumTest < ActiveRecord::TestCase
       end
     end
 
-    assert_match(/Enum label name must not be blank/, e.message)
+    assert_match(/must not contain a blank name\.$/, e.message)
+
+    e = assert_raises(ArgumentError) do
+      Class.new(ActiveRecord::Base) do
+        self.table_name = "books"
+        enum status: Object.new
+      end
+    end
+
+    assert_match(/must be either a non-empty hash or an array\.$/, e.message)
   end
 
   test "reserved enum names" do