Deprecate Error#on(attribute) in favour of Errors#[attribute]

8 files changed, 94 insertions, 70 deletions

diff --git a/activemodel/lib/active_model/deprecated_error_methods.rb b/activemodel/lib/active_model/deprecated_error_methods.rb
index c3cc1d53ae..39c79bb0c6 100644
--- a/activemodel/lib/active_model/deprecated_error_methods.rb
+++ b/activemodel/lib/active_model/deprecated_error_methods.rb
@@ -1,10 +1,10 @@
 module ActiveModel
   module DeprecatedErrorMethods
     def on(attribute)
-      # message = "Errors#on have been deprecated, use Errors#[] instead.\n"
-      # message << "Also note that the behaviour of Errors#[] has changed. Errors#[] now always returns an Array and an empty Array when "
-      # message << "there are not errors on the specified attribute."
-      # ActiveSupport::Deprecation.warn(message)
+      message = "Errors#on have been deprecated, use Errors#[] instead.\n"
+      message << "Also note that the behaviour of Errors#[] has changed. Errors#[] now always returns an Array and an empty Array when "
+      message << "there are not errors on the specified attribute."
+      ActiveSupport::Deprecation.warn(message)
 
       errors = self[attribute]
       errors.size < 2 ? errors.first : errors
diff --git a/activemodel/test/cases/validations/exclusion_validation_test.rb b/activemodel/test/cases/validations/exclusion_validation_test.rb
index b9b53ffb23..ad3123c1e4 100644
--- a/activemodel/test/cases/validations/exclusion_validation_test.rb
+++ b/activemodel/test/cases/validations/exclusion_validation_test.rb
@@ -24,7 +24,7 @@ class ExclusionValidationTest < ActiveModel::TestCase
 
     t = Topic.create("title" => "monkey")
     assert !t.valid?
-    assert t.errors.on(:title)
-    assert_equal "option monkey is restricted", t.errors.on(:title)
+    assert t.errors[:title].any?
+    assert_equal ["option monkey is restricted"], t.errors[:title]
   end
 end
diff --git a/activemodel/test/cases/validations/format_validation_test.rb b/activemodel/test/cases/validations/format_validation_test.rb
index adf90adf6f..301cb36e61 100644
--- a/activemodel/test/cases/validations/format_validation_test.rb
+++ b/activemodel/test/cases/validations/format_validation_test.rb
@@ -66,7 +66,7 @@ class PresenceValidationTest < ActiveModel::TestCase
   def test_validate_format_with_formatted_message
     Topic.validates_format_of(:title, :with => /^Valid Title$/, :message => "can't be {{value}}")
     t = Topic.create(:title => 'Invalid title')
-    assert_equal "can't be Invalid title", t.errors.on(:title)
+    assert_equal ["can't be Invalid title"], t.errors[:title]
   end
 
   def test_validates_format_of_with_custom_error_using_quotes
diff --git a/activemodel/test/cases/validations/i18n_validation_test.rb b/activemodel/test/cases/validations/i18n_validation_test.rb
index 52a6c87668..8eedf76d41 100644
--- a/activemodel/test/cases/validations/i18n_validation_test.rb
+++ b/activemodel/test/cases/validations/i18n_validation_test.rb
@@ -389,7 +389,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_confirmation_of :title
     @topic.title_confirmation = 'foo'
     @topic.valid?
-    assert_equal 'custom message', @topic.errors.on(:title)
+    assert_equal ['custom message'], @topic.errors[:title]
   end
 
   def test_validates_confirmation_of_finds_global_default_translation
@@ -398,7 +398,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_confirmation_of :title
     @topic.title_confirmation = 'foo'
     @topic.valid?
-    assert_equal 'global message', @topic.errors.on(:title)
+    assert_equal ['global message'], @topic.errors[:title]
   end
 
   # validates_acceptance_of w/o mocha
@@ -409,7 +409,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_acceptance_of :title, :allow_nil => false
     @topic.valid?
-    assert_equal 'custom message', @topic.errors.on(:title)
+    assert_equal ['custom message'], @topic.errors[:title]
   end
 
   def test_validates_acceptance_of_finds_global_default_translation
@@ -417,7 +417,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_acceptance_of :title, :allow_nil => false
     @topic.valid?
-    assert_equal 'global message', @topic.errors.on(:title)
+    assert_equal ['global message'], @topic.errors[:title]
   end
 
   # validates_presence_of w/o mocha
@@ -428,7 +428,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_presence_of :title
     @topic.valid?
-    assert_equal 'custom message', @topic.errors.on(:title)
+    assert_equal ['custom message'], @topic.errors[:title]
   end
 
   def test_validates_presence_of_finds_global_default_translation
@@ -436,7 +436,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_presence_of :title
     @topic.valid?
-    assert_equal 'global message', @topic.errors.on(:title)
+    assert_equal ['global message'], @topic.errors[:title]
   end
 
   # validates_length_of :within w/o mocha
@@ -447,7 +447,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_length_of :title, :within => 3..5
     @topic.valid?
-    assert_equal 'custom message', @topic.errors.on(:title)
+    assert_equal ['custom message'], @topic.errors[:title]
   end
 
   def test_validates_length_of_within_finds_global_default_translation
@@ -455,7 +455,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_length_of :title, :within => 3..5
     @topic.valid?
-    assert_equal 'global message', @topic.errors.on(:title)
+    assert_equal ['global message'], @topic.errors[:title]
   end
 
   # validates_length_of :is w/o mocha
@@ -466,7 +466,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_length_of :title, :is => 5
     @topic.valid?
-    assert_equal 'custom message', @topic.errors.on(:title)
+    assert_equal ['custom message'], @topic.errors[:title]
   end
 
   def test_validates_length_of_is_finds_global_default_translation
@@ -474,7 +474,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_length_of :title, :is => 5
     @topic.valid?
-    assert_equal 'global message', @topic.errors.on(:title)
+    assert_equal ['global message'], @topic.errors[:title]
   end
 
   def test_validates_length_of_is_finds_custom_model_key_translation
@@ -483,7 +483,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_length_of :title, :is => 5
     @topic.valid?
-    assert_equal 'custom message', @topic.errors.on(:title)
+    assert_equal ['custom message'], @topic.errors[:title]
   end
 
   def test_validates_length_of_is_finds_global_default_translation
@@ -491,7 +491,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_length_of :title, :is => 5
     @topic.valid?
-    assert_equal 'global message', @topic.errors.on(:title)
+    assert_equal ['global message'], @topic.errors[:title]
   end
 
 
@@ -503,7 +503,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_format_of :title, :with => /^[1-9][0-9]*$/
     @topic.valid?
-    assert_equal 'custom message', @topic.errors.on(:title)
+    assert_equal ['custom message'], @topic.errors[:title]
   end
 
   def test_validates_format_of_finds_global_default_translation
@@ -511,7 +511,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_format_of :title, :with => /^[1-9][0-9]*$/
     @topic.valid?
-    assert_equal 'global message', @topic.errors.on(:title)
+    assert_equal ['global message'], @topic.errors[:title]
   end
 
   # validates_inclusion_of w/o mocha
@@ -522,7 +522,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_inclusion_of :title, :in => %w(a b c)
     @topic.valid?
-    assert_equal 'custom message', @topic.errors.on(:title)
+    assert_equal ['custom message'], @topic.errors[:title]
   end
 
   def test_validates_inclusion_of_finds_global_default_translation
@@ -530,7 +530,7 @@ class I18nValidationTest < ActiveModel::TestCase
 
     Topic.validates_inclusion_of :title, :in => %w(a b c)
     @topic.valid?
-    assert_equal 'global message', @topic.errors.on(:title)
+    assert_equal ['global message'], @topic.errors[:title]
   end
 
   # validates_exclusion_of w/o mocha
@@ -542,7 +542,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_exclusion_of :title, :in => %w(a b c)
     @topic.title = 'a'
     @topic.valid?
-    assert_equal 'custom message', @topic.errors.on(:title)
+    assert_equal ['custom message'], @topic.errors[:title]
   end
 
   def test_validates_exclusion_of_finds_global_default_translation
@@ -551,7 +551,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_exclusion_of :title, :in => %w(a b c)
     @topic.title = 'a'
     @topic.valid?
-    assert_equal 'global message', @topic.errors.on(:title)
+    assert_equal ['global message'], @topic.errors[:title]
   end
 
   # validates_numericality_of without :only_integer w/o mocha
@@ -563,7 +563,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_numericality_of :title
     @topic.title = 'a'
     @topic.valid?
-    assert_equal 'custom message', @topic.errors.on(:title)
+    assert_equal ['custom message'], @topic.errors[:title]
   end
 
   def test_validates_numericality_of_finds_global_default_translation
@@ -572,7 +572,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_numericality_of :title, :only_integer => true
     @topic.title = 'a'
     @topic.valid?
-    assert_equal 'global message', @topic.errors.on(:title)
+    assert_equal ['global message'], @topic.errors[:title]
   end
 
   # validates_numericality_of with :only_integer w/o mocha
@@ -584,7 +584,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_numericality_of :title, :only_integer => true
     @topic.title = 'a'
     @topic.valid?
-    assert_equal 'custom message', @topic.errors.on(:title)
+    assert_equal ['custom message'], @topic.errors[:title]
   end
 
   def test_validates_numericality_of_only_integer_finds_global_default_translation
@@ -593,7 +593,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_numericality_of :title, :only_integer => true
     @topic.title = 'a'
     @topic.valid?
-    assert_equal 'global message', @topic.errors.on(:title)
+    assert_equal ['global message'], @topic.errors[:title]
   end
 
   # validates_numericality_of :odd w/o mocha
@@ -605,7 +605,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_numericality_of :title, :only_integer => true, :odd => true
     @topic.title = 0
     @topic.valid?
-    assert_equal 'custom message', @topic.errors.on(:title)
+    assert_equal ['custom message'], @topic.errors[:title]
   end
 
   def test_validates_numericality_of_odd_finds_global_default_translation
@@ -614,7 +614,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_numericality_of :title, :only_integer => true, :odd => true
     @topic.title = 0
     @topic.valid?
-    assert_equal 'global message', @topic.errors.on(:title)
+    assert_equal ['global message'], @topic.errors[:title]
   end
 
   # validates_numericality_of :less_than w/o mocha
@@ -626,7 +626,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_numericality_of :title, :only_integer => true, :less_than => 0
     @topic.title = 1
     @topic.valid?
-    assert_equal 'custom message', @topic.errors.on(:title)
+    assert_equal ['custom message'], @topic.errors[:title]
   end
 
   def test_validates_numericality_of_less_than_finds_global_default_translation
@@ -635,7 +635,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_numericality_of :title, :only_integer => true, :less_than => 0
     @topic.title = 1
     @topic.valid?
-    assert_equal 'global message', @topic.errors.on(:title)
+    assert_equal ['global message'], @topic.errors[:title]
   end
 
   def test_validations_with_message_symbol_must_translate
@@ -643,7 +643,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_presence_of :title, :message => :custom_error
     @topic.title = nil
     @topic.valid?
-    assert_equal "I am a custom error", @topic.errors.on(:title)
+    assert_equal ["I am a custom error"], @topic.errors[:title]
   end
 
   def test_validates_with_message_symbol_must_translate_per_attribute
@@ -651,7 +651,7 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_presence_of :title, :message => :custom_error
     @topic.title = nil
     @topic.valid?
-    assert_equal "I am a custom error", @topic.errors.on(:title)
+    assert_equal ["I am a custom error"], @topic.errors[:title]
   end
 
   def test_validates_with_message_symbol_must_translate_per_model
@@ -659,14 +659,14 @@ class I18nValidationTest < ActiveModel::TestCase
     Topic.validates_presence_of :title, :message => :custom_error
     @topic.title = nil
     @topic.valid?
-    assert_equal "I am a custom error", @topic.errors.on(:title)
+    assert_equal ["I am a custom error"], @topic.errors[:title]
   end
 
   def test_validates_with_message_string
     Topic.validates_presence_of :title, :message => "I am a custom error"
     @topic.title = nil
     @topic.valid?
-    assert_equal "I am a custom error", @topic.errors.on(:title)
+    assert_equal ["I am a custom error"], @topic.errors[:title]
   end
 
 end
\ No newline at end of file
diff --git a/activemodel/test/cases/validations/inclusion_validation_test.rb b/activemodel/test/cases/validations/inclusion_validation_test.rb
index 8d81cca96b..678461a037 100644
--- a/activemodel/test/cases/validations/inclusion_validation_test.rb
+++ b/activemodel/test/cases/validations/inclusion_validation_test.rb
@@ -21,8 +21,8 @@ class InclusionValidationTest < ActiveModel::TestCase
     assert t.valid?
     t.title = "uhoh"
     assert !t.valid?
-    assert t.errors.on(:title)
-    assert_equal "is not included in the list", t.errors.on(:title)
+    assert t.errors[:title].any?
+    assert_equal ["is not included in the list"], t.errors[:title]
 
     assert_raise(ArgumentError) { Topic.validates_inclusion_of( :title, :in => nil ) }
     assert_raise(ArgumentError) { Topic.validates_inclusion_of( :title, :in => 0) }
@@ -47,8 +47,8 @@ class InclusionValidationTest < ActiveModel::TestCase
 
     t = Topic.create("title" => "uhoh", "content" => "abc")
     assert !t.valid?
-    assert t.errors.on(:title)
-    assert_equal "option uhoh is not in the list", t.errors.on(:title)
+    assert t.errors[:title].any?
+    assert_equal ["option uhoh is not in the list"], t.errors[:title]
   end
 
   def test_validates_inclusion_of_with_custom_error_using_quotes
diff --git a/activemodel/test/cases/validations/length_validation_test.rb b/activemodel/test/cases/validations/length_validation_test.rb
index 9c4639a141..e84e2815fa 100644
--- a/activemodel/test/cases/validations/length_validation_test.rb
+++ b/activemodel/test/cases/validations/length_validation_test.rb
@@ -36,17 +36,17 @@ class LengthValidationTest < ActiveModel::TestCase
 
     t.title = "not"
     assert !t.valid?
-    assert t.errors.on(:title)
-    assert_equal "is too short (minimum is 5 characters)", t.errors.on(:title)
+    assert t.errors[:title].any?
+    assert_equal ["is too short (minimum is 5 characters)"], t.errors[:title]
 
     t.title = ""
     assert !t.valid?
-    assert t.errors.on(:title)
-    assert_equal "is too short (minimum is 5 characters)", t.errors.on(:title)
+    assert t.errors[:title].any?
+    assert_equal ["is too short (minimum is 5 characters)"], t.errors[:title]
 
     t.title = nil
     assert !t.valid?
-    assert t.errors.on(:title)
+    assert t.errors[:title].any?
     assert_equal ["is too short (minimum is 5 characters)"], t.errors["title"]
   end
 
@@ -68,8 +68,8 @@ class LengthValidationTest < ActiveModel::TestCase
 
     t.title = "notvalid"
     assert !t.valid?
-    assert t.errors.on(:title)
-    assert_equal "is too long (maximum is 5 characters)", t.errors.on(:title)
+    assert t.errors[:title].any?
+    assert_equal ["is too long (maximum is 5 characters)"], t.errors[:title]
 
     t.title = ""
     assert t.valid?
@@ -93,14 +93,14 @@ class LengthValidationTest < ActiveModel::TestCase
 
     t = Topic.new("title" => "a!", "content" => "I'm ooooooooh so very long")
     assert !t.valid?
-    assert_equal "is too short (minimum is 3 characters)", t.errors.on(:title)
-    assert_equal "is too long (maximum is 5 characters)", t.errors.on(:content)
+    assert_equal ["is too short (minimum is 3 characters)"], t.errors[:title]
+    assert_equal ["is too long (maximum is 5 characters)"], t.errors[:content]
 
     t.title = nil
     t.content = nil
     assert !t.valid?
-    assert_equal "is too short (minimum is 3 characters)", t.errors.on(:title)
-    assert_equal "is too short (minimum is 3 characters)", t.errors.on(:content)
+    assert_equal ["is too short (minimum is 3 characters)"], t.errors[:title]
+    assert_equal ["is too short (minimum is 3 characters)"], t.errors[:content]
 
     t.title = "abe"
     t.content  = "mad"
@@ -122,8 +122,8 @@ class LengthValidationTest < ActiveModel::TestCase
 
     t = Topic.create("title" => "thisisnotvalid", "content" => "whatever")
     assert !t.save
-    assert t.errors.on(:title)
-    assert_equal "my string is too long: 10", t.errors.on(:title)
+    assert t.errors[:title].any?
+    assert_equal ["my string is too long: 10"], t.errors[:title]
 
     t.title = "butthisis"
     assert t.save
@@ -143,17 +143,17 @@ class LengthValidationTest < ActiveModel::TestCase
 
     t = Topic.create("title" => "vali", "content" => "whatever")
     assert !t.save
-    assert t.errors.on(:title)
+    assert t.errors[:title].any?
 
     t.title = "not"
     assert !t.save
-    assert t.errors.on(:title)
-    assert_equal "my string is too short: 5", t.errors.on(:title)
+    assert t.errors[:title].any?
+    assert_equal ["my string is too short: 5"], t.errors[:title]
 
     t.title = "valid"
     t.content = "andthisistoolong"
     assert !t.save
-    assert t.errors.on(:content)
+    assert t.errors[:content].any?
 
     t.content = "iamfine"
     assert t.save
@@ -167,8 +167,8 @@ class LengthValidationTest < ActiveModel::TestCase
 
     t.title = "notvalid"
     assert !t.valid?
-    assert t.errors.on(:title)
-    assert_equal "is the wrong length (should be 5 characters)", t.errors.on(:title)
+    assert t.errors[:title].any?
+    assert_equal ["is the wrong length (should be 5 characters)"], t.errors[:title]
 
     t.title = ""
     assert !t.valid?
@@ -208,7 +208,7 @@ class LengthValidationTest < ActiveModel::TestCase
     t = Topic.create(:title => 'too short')
     assert !t.valid?
 
-    assert_equal 'tu est trops petit hombre 10', t.errors.on(:title)
+    assert_equal ['tu est trops petit hombre 10'], t.errors[:title]
   end
 
   def test_validates_length_of_nasty_params
@@ -226,23 +226,23 @@ class LengthValidationTest < ActiveModel::TestCase
     Topic.validates_length_of( :title, :minimum=>5, :message=>"boo {{count}}" )
     t = Topic.create("title" => "uhoh", "content" => "whatever")
     assert !t.valid?
-    assert t.errors.on(:title)
-    assert_equal "boo 5", t.errors.on(:title)
+    assert t.errors[:title].any?
+    assert_equal ["boo 5"], t.errors[:title]
   end
 
   def test_validates_length_of_custom_errors_for_minimum_with_too_short
     Topic.validates_length_of( :title, :minimum=>5, :too_short=>"hoo {{count}}" )
     t = Topic.create("title" => "uhoh", "content" => "whatever")
     assert !t.valid?
-    assert t.errors.on(:title)
-    assert_equal "hoo 5", t.errors.on(:title)
+    assert t.errors[:title].any?
+    assert_equal ["hoo 5"], t.errors[:title]
   end
 
   def test_validates_length_of_custom_errors_for_maximum_with_message
     Topic.validates_length_of( :title, :maximum=>5, :message=>"boo {{count}}" )
     t = Topic.create("title" => "uhohuhoh", "content" => "whatever")
     assert !t.valid?
-    assert t.errors.on(:title)
+    assert t.errors[:title].any?
     assert_equal ["boo 5"], t.errors[:title]
   end
 
@@ -374,7 +374,7 @@ class LengthValidationTest < ActiveModel::TestCase
       t.title = "1二三4"
       assert !t.save
       assert t.errors[:title].any?
-      assert_equal "短すぎます: 5", t.errors.on(:title)
+      assert_equal ["短すぎます: 5"], t.errors[:title]
 
       t.title = "一二三四五六七八九十A"
       assert !t.save
diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb
index 0fd87313e0..53d3621149 100644
--- a/activemodel/test/cases/validations/numericality_validation_test.rb
+++ b/activemodel/test/cases/validations/numericality_validation_test.rb
@@ -110,13 +110,13 @@ class NumericalityValidationTest < ActiveModel::TestCase
     topic = Topic.new("title" => "numeric test", "approved" => 10)
 
     assert !topic.valid?
-    assert_equal "smaller than 4", topic.errors.on(:approved)
+    assert_equal ["smaller than 4"], topic.errors[:approved]
 
     Topic.validates_numericality_of :approved, :greater_than => 4, :message => "greater than {{count}}"
     topic = Topic.new("title" => "numeric test", "approved" => 1)
 
     assert !topic.valid?
-    assert_equal "greater than 4", topic.errors.on(:approved)
+    assert_equal ["greater than 4"], topic.errors[:approved]
   end
 
   private
diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb
index 5b4f955eae..6c1b382cf2 100644
--- a/activemodel/test/cases/validations_test.rb
+++ b/activemodel/test/cases/validations_test.rb
@@ -80,7 +80,7 @@ class ValidationsTest < ActiveModel::TestCase
     errors = []
     r.errors.each_full { |error| errors << error }
 
-    assert_equal "Reply is not dignifying", r.errors.on_base
+    assert_equal ["Reply is not dignifying"], r.errors[:base]
 
     assert errors.include?("Title Empty")
     assert errors.include?("Reply is not dignifying")
@@ -141,4 +141,28 @@ class ValidationsTest < ActiveModel::TestCase
     t.title = 'Things are going to change'
     assert !t.invalid?
   end
+
+  def test_deprecated_error_messages_on
+    Topic.validates_presence_of :title
+
+    t = Topic.new
+    assert t.invalid?
+
+    [:title, "title"].each do |attribute|
+      assert_deprecated { assert_equal "can't be blank", t.errors.on(attribute) }
+    end
+
+    Topic.validates_each(:title) do |record, attribute|
+      record.errors[attribute] << "invalid"
+    end
+
+    assert t.invalid?
+
+    [:title, "title"].each do |attribute|
+      assert_deprecated do
+        assert t.errors.on(attribute).include?("invalid")
+        assert t.errors.on(attribute).include?("can't be blank")
+      end
+    end
+  end
 end
\ No newline at end of file