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

Revert some tests to ensure back compatibility

8 files changed, 116 insertions, 25 deletions

diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb
index 7c6346f577..5440988d27 100644
--- a/activemodel/lib/active_model/errors.rb
+++ b/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.
   #
diff --git a/activemodel/test/cases/errors_test.rb b/activemodel/test/cases/errors_test.rb
index 5dbbf910de..513bd163fc 100644
--- a/activemodel/test/cases/errors_test.rb
+++ b/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)
diff --git a/activemodel/test/cases/validations/validations_context_test.rb b/activemodel/test/cases/validations/validations_context_test.rb
index 024eb1882f..3d2dea9828 100644
--- a/activemodel/test/cases/validations/validations_context_test.rb
+++ b/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
 
diff --git a/activemodel/test/cases/validations/with_validation_test.rb b/activemodel/test/cases/validations/with_validation_test.rb
index 8239792c79..e6ae6603f2 100644
--- a/activemodel/test/cases/validations/with_validation_test.rb
+++ b/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
 
diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb
index 7776233db5..82bb66b550 100644
--- a/activemodel/test/cases/validations_test.rb
+++ b/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
 
diff --git a/activemodel/test/models/person_with_validator.rb b/activemodel/test/models/person_with_validator.rb
index 44e78cbc29..fbb28d2a0f 100644
--- a/activemodel/test/models/person_with_validator.rb
+++ b/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
 
diff --git a/activemodel/test/models/reply.rb b/activemodel/test/models/reply.rb
index 6bb18f95fe..f340b6fb14 100644
--- a/activemodel/test/models/reply.rb
+++ b/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
diff --git a/activemodel/test/validators/email_validator.rb b/activemodel/test/validators/email_validator.rb
index 0c634d8659..774a10b2ba 100644
--- a/activemodel/test/validators/email_validator.rb
+++ b/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