Enhance ActiveModel length validation checks to catch unexpected options

RobertChang0722 · Jul 22, 2024 · daa5e56 · daa5e56
1 parent b291408
commit daa5e56
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 6 deletions.
diff --git a/activemodel/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb
@@ -10,7 +10,7 @@ class LengthValidator < EachValidator # :nodoc:
       MESSAGES  = { is: :wrong_length, minimum: :too_short, maximum: :too_long }.freeze
       CHECKS    = { is: :==, minimum: :>=, maximum: :<= }.freeze
 
-      RESERVED_OPTIONS = [:minimum, :maximum, :within, :is, :too_short, :too_long]
+      RESERVED_OPTIONS = [:minimum, :maximum, :within, :is, :too_short, :too_long, :wrong_length]
 
       def initialize(options)
         if range = (options.delete(:in) || options.delete(:within))
@@ -27,15 +27,25 @@ def initialize(options)
       end
 
       def check_validity!
-        keys = CHECKS.keys & options.keys
+        valid_options = CHECKS.keys +
+                        RESERVED_OPTIONS +
+                        ActiveModel::Error::CALLBACKS_OPTIONS +
+                        ActiveModel::Error::MESSAGE_OPTIONS
 
-        if keys.empty?
-          raise ArgumentError, "Range unspecified. Specify the :in, :within, :maximum, :minimum, or :is option."
+        keys = options.keys
+        unexpected_keys = keys - valid_options
+
+        unless unexpected_keys.empty?
+          raise ArgumentError, "Unexpected option(s) provided: #{unexpected_keys.join(', ')}. Valid options are: #{valid_options.join(', ')}"
         end
 
-        keys.each do |key|
-          value = options[key]
+        length_keys = CHECKS.keys & keys
+        if length_keys.empty?
+          raise ArgumentError, "Length specification missing. Specify at least one of: :in, :within, :minimum, :maximum, :is"
+        end
 
+        length_keys.each do |key|
+          value = options[key]
           unless (value.is_a?(Integer) && value >= 0) ||
                   value == Float::INFINITY || value == -Float::INFINITY ||
                   value.is_a?(Symbol) || value.is_a?(Proc)

diff --git a/activemodel/test/cases/validations/length_validation_test.rb b/activemodel/test/cases/validations/length_validation_test.rb
@@ -496,4 +496,10 @@ def test_validates_length_of_using_lambda_as_maximum
     t.title = ""
     assert_predicate t, :valid?
   end
+
+  def test_typo_in_length_options_raises_error
+    assert_raise(ArgumentError) { Topic.validates_length_of :title, minium: 5, maximum: 10 }
+    assert_raise(ArgumentError) { Topic.validates_length_of :title, mimum: 5, maximum: 10 }
+    assert_raise(ArgumentError) { Topic.validates_length_of :title, mimum: 5, maxium: 10 }
+  end
 end