Make ActiveModel::Errors#add_on_blank and #add_on_empty accept an options hash and make various Validators pass their (filtered) options.

This makes it possible to pass additional options through Validators to message generation. E.g. plugin authors want to add validates_presence_of :foo, :format => "some format".

Also, cleanup the :default vs :message options confusion in ActiveModel validation message generation.

Also, deprecate ActiveModel::Errors#add_on_blank(attributes, custom_message) in favor of ActiveModel::Errors#add_on_blank(attributes, options).

Also, refactoring of ActiveModel and ActiveRecord Validation tests. Test are a lot more DRY now. Better test coverage as well now.

The first four points were reapplied from an older patch of Sven Fuchs which didn't apply cleanly anymore and was not complete yet.

Signed-off-by: José Valim <jose.valim@gmail.com>

9 files changed, 69 insertions, 32 deletions

diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb
index 9efb683547..b4660f3587 100644
--- a/activemodel/lib/active_model/errors.rb
+++ b/activemodel/lib/active_model/errors.rb
@@ -182,25 +182,43 @@ module ActiveModel
     # If +message+ is a proc, it will be called, allowing for things like <tt>Time.now</tt> to be used within an error.
     def add(attribute, message = nil, options = {})
       message ||= :invalid
-      message = generate_message(attribute, message, options) if message.is_a?(Symbol)
+
+      validation_conditionals = [:if, :unless, :on]
+
+      message = generate_message(attribute, message, options.except(*validation_conditionals)) if message.is_a?(Symbol)
+
       message = message.call if message.is_a?(Proc)
       self[attribute] << message
     end
 
     # Will add an error message to each of the attributes in +attributes+ that is empty.
-    def add_on_empty(attributes, custom_message = nil)
+    def add_on_empty(attributes, options = {})
+      if options && !options.is_a?(Hash)
+        options = { :message => options }
+        ActiveSupport::Deprecation.warn \
+          "ActiveModel::Errors#add_on_empty(attributes, custom_message) has been deprecated.\n" +
+          "Instead of passing a custom_message pass an options Hash { :message => custom_message }."
+      end
+
       [attributes].flatten.each do |attribute|
         value = @base.send(:read_attribute_for_validation, attribute)
         is_empty = value.respond_to?(:empty?) ? value.empty? : false
-        add(attribute, :empty, :default => custom_message) unless !value.nil? && !is_empty
+        add(attribute, :empty, options) if value.nil? || is_empty
       end
     end
 
     # Will add an error message to each of the attributes in +attributes+ that is blank (using Object#blank?).
-    def add_on_blank(attributes, custom_message = nil)
+    def add_on_blank(attributes, options = {})
+      if options && !options.is_a?(Hash)
+        options = { :message => options }
+        ActiveSupport::Deprecation.warn \
+          "ActiveModel::Errors#add_on_blank(attributes, custom_message) has been deprecated.\n" +
+          "Instead of passing a custom_message pass an options Hash { :message => custom_message }."
+      end
+
       [attributes].flatten.each do |attribute|
         value = @base.send(:read_attribute_for_validation, attribute)
-        add(attribute, :blank, :default => custom_message) if value.blank?
+        add(attribute, :blank, options) if value.blank?
       end
     end
 
@@ -262,18 +280,26 @@ module ActiveModel
     # <li><tt>errors.attributes.title.blank</tt></li>
     # <li><tt>errors.messages.blank</tt></li>
     # </ol>
-    def generate_message(attribute, message = :invalid, options = {})
-      message, options[:default] = options[:default], message if options[:default].is_a?(Symbol)
+
+    def generate_message(attribute, type = :invalid, options = {})
+      type = options.delete(:message) if options[:message].is_a?(Symbol)
+
+      if options[:default]
+        ActiveSupport::Deprecation.warn \
+          "ActiveModel::Errors#generate_message(attributes, custom_message) has been deprecated.\n" +
+          "Use ActiveModel::Errors#generate_message(attributes, :message => 'your message') instead."
+        options[:message] = options.delete(:default)
+      end
 
       defaults = @base.class.lookup_ancestors.map do |klass|
-        [ :"#{@base.class.i18n_scope}.errors.models.#{klass.model_name.underscore}.attributes.#{attribute}.#{message}",
-          :"#{@base.class.i18n_scope}.errors.models.#{klass.model_name.underscore}.#{message}" ]
+        [ :"#{@base.class.i18n_scope}.errors.models.#{klass.model_name.underscore}.attributes.#{attribute}.#{type}",
+          :"#{@base.class.i18n_scope}.errors.models.#{klass.model_name.underscore}.#{type}" ]
       end
 
-      defaults << options.delete(:default)
-      defaults << :"#{@base.class.i18n_scope}.errors.messages.#{message}"
-      defaults << :"errors.attributes.#{attribute}.#{message}"
-      defaults << :"errors.messages.#{message}"
+      defaults << options.delete(:message)
+      defaults << :"#{@base.class.i18n_scope}.errors.messages.#{type}"
+      defaults << :"errors.attributes.#{attribute}.#{type}"
+      defaults << :"errors.messages.#{type}"
 
       defaults.compact!
       defaults.flatten!
diff --git a/activemodel/lib/active_model/validations/acceptance.rb b/activemodel/lib/active_model/validations/acceptance.rb
index 77c401e0cc..99b8966def 100644
--- a/activemodel/lib/active_model/validations/acceptance.rb
+++ b/activemodel/lib/active_model/validations/acceptance.rb
@@ -9,10 +9,10 @@ module ActiveModel
 
       def validate_each(record, attribute, value)
         unless value == options[:accept]
-          record.errors.add(attribute, :accepted, :default => options[:message])
+          record.errors.add(attribute, :accepted, options.except(:accept, :allow_nil))
         end
       end
-      
+
       def setup(klass)
         # Note: instance_methods.map(&:to_s) is important for 1.9 compatibility
         # as instance_methods returns symbols unlike 1.8 which returns strings.
diff --git a/activemodel/lib/active_model/validations/confirmation.rb b/activemodel/lib/active_model/validations/confirmation.rb
index 5e13db7491..3a80893866 100644
--- a/activemodel/lib/active_model/validations/confirmation.rb
+++ b/activemodel/lib/active_model/validations/confirmation.rb
@@ -6,11 +6,11 @@ module ActiveModel
       def validate_each(record, attribute, value)
         confirmed = record.send(:"#{attribute}_confirmation")
         return if confirmed.nil? || value == confirmed
-        record.errors.add(attribute, :confirmation, :default => options[:message])
+        record.errors.add(attribute, :confirmation, options)
       end
-      
+
       def setup(klass)
-        klass.send(:attr_accessor, *attributes.map { |attribute| :"#{attribute}_confirmation" })        
+        klass.send(:attr_accessor, *attributes.map { |attribute| :"#{attribute}_confirmation" })
       end
     end
 
diff --git a/activemodel/lib/active_model/validations/exclusion.rb b/activemodel/lib/active_model/validations/exclusion.rb
index 599623368f..6fe43c7219 100644
--- a/activemodel/lib/active_model/validations/exclusion.rb
+++ b/activemodel/lib/active_model/validations/exclusion.rb
@@ -9,8 +9,9 @@ module ActiveModel
       end
 
       def validate_each(record, attribute, value)
-        return unless options[:in].include?(value)
-        record.errors.add(attribute, :exclusion, :default => options[:message], :value => value)
+        if options[:in].include?(value)
+          record.errors.add(attribute, :exclusion, options.except(:in).merge(:value => value))
+        end
       end
     end
 
diff --git a/activemodel/lib/active_model/validations/format.rb b/activemodel/lib/active_model/validations/format.rb
index 3b57d4fd77..6f6933205f 100644
--- a/activemodel/lib/active_model/validations/format.rb
+++ b/activemodel/lib/active_model/validations/format.rb
@@ -5,12 +5,12 @@ module ActiveModel
     class FormatValidator < EachValidator
       def validate_each(record, attribute, value)
         if options[:with] && value.to_s !~ options[:with]
-          record.errors.add(attribute, :invalid, :default => options[:message], :value => value)
+          record.errors.add(attribute, :invalid, options.except(:with).merge(:value => value))
         elsif options[:without] && value.to_s =~ options[:without]
-          record.errors.add(attribute, :invalid, :default => options[:message], :value => value)
+          record.errors.add(attribute, :invalid, options.except(:with).merge(:value => value))
         end
       end
-      
+
       def check_validity!
         unless options.include?(:with) ^ options.include?(:without)  # ^ == xor, or "exclusive or"
           raise ArgumentError, "Either :with or :without must be supplied (but not both)"
diff --git a/activemodel/lib/active_model/validations/inclusion.rb b/activemodel/lib/active_model/validations/inclusion.rb
index e9940dbbf0..863f4da0ff 100644
--- a/activemodel/lib/active_model/validations/inclusion.rb
+++ b/activemodel/lib/active_model/validations/inclusion.rb
@@ -9,8 +9,9 @@ module ActiveModel
       end
 
       def validate_each(record, attribute, value)
-        return if options[:in].include?(value)
-        record.errors.add(attribute, :inclusion, :default => options[:message], :value => value)
+        unless options[:in].include?(value)
+          record.errors.add(attribute, :inclusion, options.except(:in).merge(:value => value))
+        end
       end
     end
 
diff --git a/activemodel/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb
index dc191d3150..77db437a33 100644
--- a/activemodel/lib/active_model/validations/length.rb
+++ b/activemodel/lib/active_model/validations/length.rb
@@ -39,7 +39,8 @@ module ActiveModel
 
         CHECKS.each do |key, validity_check|
           next unless check_value = options[key]
-          custom_message = options[:message] || options[MESSAGES[key]]
+          default_message = options[MESSAGES[key]]
+          options[:message] ||= default_message if default_message
 
           valid_value = if key == :maximum
             value.nil? || value.size.send(validity_check, check_value)
@@ -48,7 +49,10 @@ module ActiveModel
           end
 
           next if valid_value
-          record.errors.add(attribute, MESSAGES[key], :default => custom_message, :count => check_value)
+
+          reserved_options = [:minimum, :maximum, :within, :is, :tokenizer, :too_short, :too_long]
+          record.errors.add(attribute, MESSAGES[key],
+                            options.except(*reserved_options).merge(:count => check_value))
         end
       end
     end
diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb
index c2e7223939..dcc3befb08 100644
--- a/activemodel/lib/active_model/validations/numericality.rb
+++ b/activemodel/lib/active_model/validations/numericality.rb
@@ -28,13 +28,13 @@ module ActiveModel
         return if options[:allow_nil] && raw_value.nil?
 
         unless value = parse_raw_value_as_a_number(raw_value)
-          record.errors.add(attr_name, :not_a_number, :value => raw_value, :default => options[:message])
+          record.errors.add(attr_name, :not_a_number, filtered_options(raw_value))
           return
         end
 
         if options[:only_integer]
           unless value = parse_raw_value_as_an_integer(raw_value)
-            record.errors.add(attr_name, :not_an_integer, :value => raw_value, :default => options[:message])
+            record.errors.add(attr_name, :not_an_integer, filtered_options(raw_value))
             return
           end
         end
@@ -43,14 +43,14 @@ module ActiveModel
           case option
           when :odd, :even
             unless value.to_i.send(CHECKS[option])
-              record.errors.add(attr_name, option, :value => value, :default => options[:message])
+              record.errors.add(attr_name, option, filtered_options(value))
             end
           else
             option_value = option_value.call(record) if option_value.is_a?(Proc)
             option_value = record.send(option_value) if option_value.is_a?(Symbol)
 
             unless value.send(CHECKS[option], option_value)
-              record.errors.add(attr_name, option, :default => options[:message], :value => value, :count => option_value)
+              record.errors.add(attr_name, option, filtered_options(value).merge(:count => option_value))
             end
           end
         end
@@ -75,6 +75,11 @@ module ActiveModel
         raw_value.to_i if raw_value.to_s =~ /\A[+-]?\d+\Z/
       end
 
+      def filtered_options(value)
+        reserved_options = [:allow_nil, :odd, :even, :not_an_integer, :only_integer, :allow_nil, :less_than]
+        options.except(*reserved_options).merge(:value => value)
+      end
+
     end
 
     module HelperMethods
diff --git a/activemodel/lib/active_model/validations/presence.rb b/activemodel/lib/active_model/validations/presence.rb
index e36bee8115..28c4640b17 100644
--- a/activemodel/lib/active_model/validations/presence.rb
+++ b/activemodel/lib/active_model/validations/presence.rb
@@ -6,7 +6,7 @@ module ActiveModel
   module Validations
     class PresenceValidator < EachValidator
       def validate(record)
-        record.errors.add_on_blank(attributes, options[:message])
+        record.errors.add_on_blank(attributes, options)
       end
     end