Raise deprecation for calling [:f] = 'b' or [:f] << 'b'

Revert some tests to ensure back compatibility

actionview/test/template/active_model_helper_test.rb

@@ -20,11 +20,11 @@ class ActiveModelHelperTest < ActionView::TestCase
     super
 
     @post = Post.new
-    @post.errors[:author_name] << "can't be empty"
-    @post.errors[:body] << "foo"
-    @post.errors[:category] << "must exist"
-    @post.errors[:published] << "must be accepted"
-    @post.errors[:updated_at] << "bar"
+    assert_deprecated { @post.errors[:author_name] << "can't be empty" }
+    assert_deprecated { @post.errors[:body] << "foo" }
+    assert_deprecated { @post.errors[:category] << "must exist" }
+    assert_deprecated { @post.errors[:published] << "must be accepted" }
+    assert_deprecated { @post.errors[:updated_at] << "bar" }
 
     @post.author_name = ""
     @post.body        = "Back to the hill and over it again!"

actionview/test/template/form_options_helper_test.rb

@@ -29,10 +29,20 @@ class FormOptionsHelperTest < ActionView::TestCase
                   end
     Continent   = Struct.new("Continent", :continent_name, :countries)
     Country     = Struct.new("Country", :country_id, :country_name)
-    Firm        = Struct.new("Firm", :time_zone)
     Album       = Struct.new("Album", :id, :title, :genre)
   end
 
+  class Firm
+    include ActiveModel::Validations
+    extend ActiveModel::Naming
+
+    attr_accessor :time_zone
+
+    def initialize(time_zone = nil)
+      @time_zone = time_zone
+    end
+  end
+
   module FakeZones
     FakeZone = Struct.new(:name) do
       def to_s; name; end
@@ -1294,7 +1304,7 @@ class FormOptionsHelperTest < ActionView::TestCase
   def test_time_zone_select_with_priority_zones_and_errors
     @firm = Firm.new("D")
     @firm.extend ActiveModel::Validations
-    @firm.errors[:time_zone] << "invalid"
+    assert_deprecated { @firm.errors[:time_zone] << "invalid" }
     zones = [ ActiveSupport::TimeZone.new("A"), ActiveSupport::TimeZone.new("D") ]
     html = time_zone_select("firm", "time_zone", zones)
     assert_dom_equal "<div class=\"field_with_errors\">" \

activemodel/lib/active_model/errors.rb

@@ -302,11 +302,11 @@ module ActiveModel
     #   person.errors.to_hash(true) # => {:name=>["name cannot be nil"]}
     def to_hash(full_messages = false)
       hash = {}
-      message_method = full_messages ? :full_message : :message
+      message_method = full_messages ? :full_messages_for : :messages_for
       group_by_attribute.each do |attribute, errors|
-        hash[attribute] = errors.map(&message_method)
+        hash[attribute] = public_send(message_method, attribute)
       end
-      hash
+      DeprecationHandlingMessageHash.new(hash, self)
     end
     alias :messages :to_hash
 
@@ -458,7 +458,7 @@ module ActiveModel
     end
 
     def messages_for(attribute)
-      where(attribute).map(&:message).freeze
+      DeprecationHandlingMessageArray.new(where(attribute).map(&:message).freeze, self, attribute)
     end
 
     # Returns a full message for a given attribute.
@@ -619,6 +619,48 @@ module ActiveModel
       end
   end
 
+  class DeprecationHandlingMessageHash < SimpleDelegator
+    def initialize(content, errors)
+      @errors = errors
+      super(content)
+    end
+
+    def []=(attribute, value)
+      ActiveSupport::Deprecation.warn("Calling `[]=` to an ActiveModel::Errors is deprecated. Please call `ActiveModel::Errors#add` instead.")
+
+      Array(value).each do |message|
+        @errors.add(attribute, message)
+      end
+      __setobj__ @errors.messages
+      value
+    end
+
+    def [](attribute)
+      messages = super(attribute)
+
+      return messages if messages
+
+      __getobj__[attribute]= DeprecationHandlingMessageArray.new([], @errors, attribute)
+    end
+  end
+
+  class DeprecationHandlingMessageArray < SimpleDelegator
+    def initialize(content, errors, attribute)
+      @errors = errors
+      @attribute = attribute
+      super(content)
+    end
+
+    def <<(message)
+      ActiveSupport::Deprecation.warn("Calling `<<` to an ActiveModel::Errors message array in order to add an error is deprecated. Please call `ActiveModel::Errors#add` instead.")
+
+      @errors.add(@attribute, message)
+      __setobj__ @errors[@attribute]
+      self
+    end
+  end
+
+
   # Raised when a validation cannot be corrected by end users and are considered
   # exceptional.
   #

activemodel/test/cases/errors_test.rb

@@ -39,7 +39,7 @@ class ErrorsTest < ActiveModel::TestCase
 
   def test_include?
     errors = ActiveModel::Errors.new(Person.new)
-    errors[:foo] << "omg"
+    assert_deprecated { errors[:foo] << "omg" }
     assert_includes errors, :foo, "errors should include :foo"
     assert_includes errors, "foo", "errors should include 'foo' as :foo"
   end
@@ -93,8 +93,8 @@ class ErrorsTest < ActiveModel::TestCase
 
   test "values returns an array of messages" do
     errors = ActiveModel::Errors.new(Person.new)
-    errors.messages[:foo] = "omg"
-    errors.messages[:baz] = "zomg"
+    assert_deprecated { errors.messages[:foo] = "omg" }
+    assert_deprecated { errors.messages[:baz] = "zomg" }
 
     assert_deprecated do
       assert_equal ["omg", "zomg"], errors.values
@@ -113,11 +113,11 @@ class ErrorsTest < ActiveModel::TestCase
 
   test "keys returns the error keys" do
     errors = ActiveModel::Errors.new(Person.new)
-    errors.add(:name)
-    errors.add(:age)
+    assert_deprecated { errors.messages[:foo] << "omg" }
+    assert_deprecated { errors.messages[:baz] << "zomg" }
 
     assert_deprecated do
-      assert_equal [:name, :age], errors.keys
+      assert_equal [:foo, :baz], errors.keys
     end
   end
 
@@ -153,6 +153,32 @@ class ErrorsTest < ActiveModel::TestCase
     assert_equal ["cannot be nil"], person.errors[:name]
   end
 
+  test "add an error message on a specific attribute (deprecated)" do
+    person = Person.new
+    person.errors.add(:name, "cannot be blank")
+    assert_equal ["cannot be blank"], person.errors[:name]
+  end
+
+  test "add an error message on a specific attribute with a defined type (deprecated)" do
+    person = Person.new
+    person.errors.add(:name, :blank, message: "cannot be blank")
+    assert_equal ["cannot be blank"], person.errors[:name]
+  end
+
+  test "add an error with a symbol (deprecated)" do
+    person = Person.new
+    person.errors.add(:name, :blank)
+    message = person.errors.generate_message(:name, :blank)
+    assert_equal [message], person.errors[:name]
+  end
+
+  test "add an error with a proc (deprecated)" do
+    person = Person.new
+    message = Proc.new { "cannot be blank" }
+    person.errors.add(:name, message)
+    assert_equal ["cannot be blank"], person.errors[:name]
+  end
+
   test "add creates an error object and returns it" do
     person = Person.new
     error = person.errors.add(:name, :blank)
@@ -532,6 +558,16 @@ class ErrorsTest < ActiveModel::TestCase
     assert_empty person.errors.details
   end
 
+  test "copy errors (deprecated)" do
+    errors = ActiveModel::Errors.new(Person.new)
+    errors.add(:name, :invalid)
+    person = Person.new
+    person.errors.copy!(errors)
+
+    assert_equal [:name], person.errors.messages.keys
+    assert_equal [:name], person.errors.details.keys
+  end
+
   test "copy errors" do
     errors = ActiveModel::Errors.new(Person.new)
     errors.add(:name, :invalid)
@@ -544,6 +580,18 @@ class ErrorsTest < ActiveModel::TestCase
     end
   end
 
+  test "merge errors (deprecated)" do
+    errors = ActiveModel::Errors.new(Person.new)
+    errors.add(:name, :invalid)
+
+    person = Person.new
+    person.errors.add(:name, :blank)
+    person.errors.merge!(errors)
+
+    assert_equal({ name: ["can't be blank", "is invalid"] }, person.errors.messages)
+    assert_equal({ name: [{ error: :blank }, { error: :invalid }] }, person.errors.details)
+  end
+
   test "merge errors" do
     errors = ActiveModel::Errors.new(Person.new)
     errors.add(:name, :invalid)

activemodel/test/cases/validations/validations_context_test.rb

@@ -14,13 +14,13 @@ class ValidationsContextTest < ActiveModel::TestCase
 
   class ValidatorThatAddsErrors < ActiveModel::Validator
     def validate(record)
-      record.errors[:base] << ERROR_MESSAGE
+      record.errors.add(:base, ERROR_MESSAGE)
     end
   end
 
   class AnotherValidatorThatAddsErrors < ActiveModel::Validator
     def validate(record)
-      record.errors[:base] << ANOTHER_ERROR_MESSAGE
+      record.errors.add(:base, ANOTHER_ERROR_MESSAGE)
     end
   end
 

activemodel/test/cases/validations/with_validation_test.rb

@@ -14,13 +14,13 @@ class ValidatesWithTest < ActiveModel::TestCase
 
   class ValidatorThatAddsErrors < ActiveModel::Validator
     def validate(record)
-      record.errors[:base] << ERROR_MESSAGE
+      record.errors.add(:base, message: ERROR_MESSAGE)
     end
   end
 
   class OtherValidatorThatAddsErrors < ActiveModel::Validator
     def validate(record)
-      record.errors[:base] << OTHER_ERROR_MESSAGE
+      record.errors.add(:base, message: OTHER_ERROR_MESSAGE)
     end
   end
 
@@ -32,14 +32,14 @@ class ValidatesWithTest < ActiveModel::TestCase
   class ValidatorThatValidatesOptions < ActiveModel::Validator
     def validate(record)
       if options[:field] == :first_name
-        record.errors[:base] << ERROR_MESSAGE
+        record.errors.add(:base, message: ERROR_MESSAGE)
       end
     end
   end
 
   class ValidatorPerEachAttribute < ActiveModel::EachValidator
     def validate_each(record, attribute, value)
-      record.errors[attribute] << "Value is #{value}"
+      record.errors.add(attribute, message: "Value is #{value}")
     end
   end
 

activemodel/test/cases/validations_test.rb

@@ -74,7 +74,7 @@ class ValidationsTest < ActiveModel::TestCase
 
   def test_errors_on_nested_attributes_expands_name
     t = Topic.new
-    t.errors["replies.name"] << "can't be blank"
+    assert_deprecated { t.errors["replies.name"] << "can't be blank" }
     assert_equal ["Replies name can't be blank"], t.errors.full_messages
   end
 

activemodel/test/models/person_with_validator.rb

@@ -5,7 +5,7 @@ class PersonWithValidator
 
   class PresenceValidator < ActiveModel::EachValidator
     def validate_each(record, attribute, value)
-      record.errors[attribute] << "Local validator#{options[:custom]}" if value.blank?
+      record.errors.add(attribute, message: "Local validator#{options[:custom]}") if value.blank?
     end
   end
 

activemodel/test/models/reply.rb

@@ -11,24 +11,24 @@ class Reply < Topic
   validate :check_wrong_update,     on: :update
 
   def check_empty_title
-    errors[:title] << "is Empty" unless title && title.size > 0
+    errors.add(:title, "is Empty") unless title && title.size > 0
   end
 
   def errors_on_empty_content
-    errors[:content] << "is Empty" unless content && content.size > 0
+    errors.add(:content, "is Empty") unless content && content.size > 0
   end
 
   def check_content_mismatch
     if title && content && content == "Mismatch"
-      errors[:title] << "is Content Mismatch"
+      errors.add(:title, "is Content Mismatch")
     end
   end
 
   def title_is_wrong_create
-    errors[:title] << "is Wrong Create" if title && title == "Wrong Create"
+    errors.add(:title, "is Wrong Create") if title && title == "Wrong Create"
   end
 
   def check_wrong_update
-    errors[:title] << "is Wrong Update" if title && title == "Wrong Update"
+    errors.add(:title, "is Wrong Update") if title && title == "Wrong Update"
   end
 end

activemodel/test/validators/email_validator.rb

@@ -2,7 +2,8 @@
 
 class EmailValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
-    record.errors[attribute] << (options[:message] || "is not an email") unless
-      /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i.match?(value)
+    unless /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i.match?(value)
+      record.errors.add(attribute, message: options[:message] || "is not an email")
+    end
   end
 end

activerecord/test/cases/associations/has_many_through_associations_test.rb

@@ -746,10 +746,14 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
 
       firm = companies(:first_firm)
       lifo = Developer.new(name: "lifo")
-      assert_raises(ActiveRecord::RecordInvalid) { firm.developers << lifo }
+      assert_raises(ActiveRecord::RecordInvalid) do
+        assert_deprecated { firm.developers << lifo }
+      end
 
       lifo = Developer.create!(name: "lifo")
-      assert_raises(ActiveRecord::RecordInvalid) { firm.developers << lifo }
+      assert_raises(ActiveRecord::RecordInvalid) do
+        assert_deprecated { firm.developers << lifo }
+      end
     end
   end
 
@@ -1160,7 +1164,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   def test_create_should_not_raise_exception_when_join_record_has_errors
     repair_validations(Categorization) do
       Categorization.validate { |r| r.errors[:base] << "Invalid Categorization" }
-      Category.create(name: "Fishing", authors: [Author.first])
+      assert_deprecated { Category.create(name: "Fishing", authors: [Author.first]) }
     end
   end
 
@@ -1173,7 +1177,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     repair_validations(Categorization) do
       Categorization.validate { |r| r.errors[:base] << "Invalid Categorization" }
       assert_raises(ActiveRecord::RecordInvalid) do
-        Category.create!(name: "Fishing", authors: [Author.first])
+        assert_deprecated { Category.create!(name: "Fishing", authors: [Author.first]) }
       end
     end
   end
@@ -1183,7 +1187,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
       Categorization.validate { |r| r.errors[:base] << "Invalid Categorization" }
       c = Category.new(name: "Fishing", authors: [Author.first])
       assert_raises(ActiveRecord::RecordInvalid) do
-        c.save!
+        assert_deprecated { c.save! }
       end
     end
   end
@@ -1192,7 +1196,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     repair_validations(Categorization) do
       Categorization.validate { |r| r.errors[:base] << "Invalid Categorization" }
       c = Category.new(name: "Fishing", authors: [Author.first])
-      assert_not c.save
+      assert_deprecated { assert_not c.save }
     end
   end
 

activerecord/test/models/reply.rb

@@ -34,29 +34,29 @@ class WrongReply < Reply
   validate :check_author_name_is_secret, on: :special_case
 
   def check_empty_title
-    errors[:title] << "Empty" unless attribute_present?("title")
+    errors.add(:title, "Empty") unless attribute_present?("title")
   end
 
   def errors_on_empty_content
-    errors[:content] << "Empty" unless attribute_present?("content")
+    errors.add(:content, "Empty") unless attribute_present?("content")
   end
 
   def check_content_mismatch
     if attribute_present?("title") && attribute_present?("content") && content == "Mismatch"
-      errors[:title] << "is Content Mismatch"
+      errors.add(:title, "is Content Mismatch")
     end
   end
 
   def title_is_wrong_create
-    errors[:title] << "is Wrong Create" if attribute_present?("title") && title == "Wrong Create"
+    errors.add(:title, "is Wrong Create") if attribute_present?("title") && title == "Wrong Create"
   end
 
   def check_wrong_update
-    errors[:title] << "is Wrong Update" if attribute_present?("title") && title == "Wrong Update"
+    errors.add(:title, "is Wrong Update") if attribute_present?("title") && title == "Wrong Update"
   end
 
   def check_author_name_is_secret
-    errors[:author_name] << "Invalid" unless author_name == "secret"
+    errors.add(:author_name, "Invalid") unless author_name == "secret"
   end
 end
 

guides/source/active_record_validations.md

@@ -664,7 +664,7 @@ This helper passes the record to a separate class for validation.
 class GoodnessValidator < ActiveModel::Validator
   def validate(record)
     if record.first_name == "Evil"
-      record.errors[:base] << "This person is evil"
+      record.errors.add :base, "This person is evil"
     end
   end
 end
@@ -692,7 +692,7 @@ validator class as `options`:
 class GoodnessValidator < ActiveModel::Validator
   def validate(record)
     if options[:fields].any?{|field| record.send(field) == "Evil" }
-      record.errors[:base] << "This person is evil"
+      record.errors.add :base, "This person is evil"
     end
   end
 end
@@ -723,7 +723,7 @@ class GoodnessValidator
 
   def validate
     if some_complex_condition_involving_ivars_and_private_methods?
-      @person.errors[:base] << "This person is evil"
+      @person.errors.add :base, "This person is evil"
     end
   end
 
@@ -1004,7 +1004,7 @@ and performs the validation on it. The custom validator is called using the
 class MyValidator < ActiveModel::Validator
   def validate(record)
     unless record.name.starts_with? 'X'
-      record.errors[:name] << 'Need a name starting with X please!'
+      record.errors.add :name, "Need a name starting with X please!"
     end
   end
 end
@@ -1026,7 +1026,7 @@ instance.
 class EmailValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
     unless value =~ /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i
-      record.errors[attribute] << (options[:message] || "is not an email")
+      record.errors.add attribute, (options[:message] || "is not an email")
     end
   end
 end
@@ -1203,7 +1203,7 @@ You can add error messages that are related to the object's state as a whole, in
 ```ruby
 class Person < ApplicationRecord
   def a_method_used_for_validation_purposes
-    errors[:base] << "This person is invalid because ..."
+    errors.add :base, "This person is invalid because ..."
   end
 end
 ```