From 0950d409b041415f13d037aa4293ac31f97ed236 Mon Sep 17 00:00:00 2001 From: sonnym Date: Thu, 17 Jul 2014 17:02:54 -0400 Subject: [PATCH] check for valid options in validate method This change prevents a certain class of user error which results when mistakenly using the `validate` class method instead of the `validates` class method. Only apply when all arguments are symbols, because some validations use the `validate` method and pass in additional options, namely the `LenghValidator` via the `ActiveMode::Validations::validates_with` method. --- activemodel/CHANGELOG.md | 6 ++++++ activemodel/lib/active_model/validations.rb | 6 ++++++ activemodel/test/cases/validations_test.rb | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 555cd259d2..c8ee8ad7de 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,9 @@ +* Validate options passed to `ActiveModel::Validations::validate` + + Preventing, in many cases, the simple mistake of using `validate` instead of `validate`. + + *Sonny Michaud* + * Deprecate `reset_#{attribute}` in favor of `restore_#{attribute}`. These methods may cause confusion with the `reset_changes` that behaves differently diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index cf97f45dba..b3d345c8ca 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -141,6 +141,11 @@ def validates_each(*attr_names, &block) # value. def validate(*args, &block) options = args.extract_options! + + if args.all? { |arg| arg.is_a?(Symbol) } + options.assert_valid_keys(%i(on if unless)) + end + if options.key?(:on) options = options.dup options[:if] = Array(options[:if]) @@ -148,6 +153,7 @@ def validate(*args, &block) Array(options[:on]).include?(o.validation_context) } end + args << options set_callback(:validate, *args, &block) end diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb index 4fee704ef5..948efd0f0a 100644 --- a/activemodel/test/cases/validations_test.rb +++ b/activemodel/test/cases/validations_test.rb @@ -167,6 +167,12 @@ def test_invalid_validator end end + def test_invalid_options_to_validate + assert_raises(ArgumentError) do + Topic.validate :title, presence: true + end + end + def test_errors_conversions Topic.validates_presence_of %w(title content) t = Topic.new