Deprecated define_attr_method in ActiveModel::AttributeMethods

This only existed to support methods like `set_table_name` in Active
Record, which are themselves being deprecated.

activemodel/CHANGELOG.md

@@ -1,5 +1,10 @@
 ## Rails 3.2.0 (unreleased) ##
 
+*   Deprecated `define_attr_method` in `ActiveModel::AttributeMethods`, because this only existed to
+    support methods like `set_table_name` in Active Record, which are themselves being deprecated.
+
+    *Jon Leighton*
+
 *   Renamed (with a deprecation the following constants):
 
         ActiveModel::Serialization     => ActiveModel::Serializable

activemodel/lib/active_model/attribute_methods.rb

@@ -66,46 +66,29 @@ module AttributeMethods
     end
 
     module ClassMethods
-      # Defines an "attribute" method (like +inheritance_column+ or +table_name+).
-      # A new (class) method will be created with the given name. If a value is
-      # specified, the new method will return that value (as a string).
-      # Otherwise, the given block will be used to compute the value of the
-      # method.
-      #
-      # The original method will be aliased, with the new name being prefixed
-      # with "original_". This allows the new method to access the original
-      # value.
-      #
-      # Example:
-      #
-      #   class Person
-      #
-      #     include ActiveModel::AttributeMethods
-      #
-      #     cattr_accessor :primary_key
-      #     cattr_accessor :inheritance_column
-      #
-      #     define_attr_method :primary_key, "sysid"
-      #     define_attr_method( :inheritance_column ) do
-      #       original_inheritance_column + "_id"
-      #     end
-      #
-      #   end
-      #
-      # Provides you with:
-      #
-      #   Person.primary_key
-      #   # => "sysid"
-      #   Person.inheritance_column = 'address'
-      #   Person.inheritance_column
-      #   # => 'address_id'
-      def define_attr_method(name, value=nil, &block)
+      def define_attr_method(name, value=nil, deprecation_warning = true, &block) #:nodoc:
+        # This deprecation_warning param is for internal use so that we can silence
+        # the warning from Active Record, because we are implementing more specific
+        # messages there instead.
+        #
+        # It doesn't apply to the original_#{name} method as we want to warn if
+        # people are calling that regardless.
+        if deprecation_warning
+          ActiveSupport::Deprecation.warn("define_attr_method is deprecated and will be removed without replacement.")
+        end
+
         sing = singleton_class
         sing.class_eval <<-eorb, __FILE__, __LINE__ + 1
-          if method_defined?('original_#{name}')
-            undef :'original_#{name}'
+          remove_possible_method :'original_#{name}'
+          remove_possible_method :'_original_#{name}'
+          alias_method :'_original_#{name}', :'#{name}'
+          define_method :'original_#{name}' do
+            ActiveSupport::Deprecation.warn(
+              "This method is generated by ActiveModel::AttributeMethods::ClassMethods#define_attr_method, " \
+              "which is deprecated and will be removed."
+            )
+            send(:'_original_#{name}')
           end
-          alias_method :'original_#{name}', :'#{name}'
         eorb
         if block_given?
           sing.send :define_method, name, &block

activemodel/test/cases/attribute_methods_test.rb

@@ -134,20 +134,33 @@ def foo
   end
 
   test '#define_attr_method generates attribute method' do
-    ModelWithAttributes.define_attr_method(:bar, 'bar')
+    assert_deprecated do
+      ModelWithAttributes.define_attr_method(:bar, 'bar')
+    end
 
     assert_respond_to ModelWithAttributes, :bar
-    assert_equal "original bar", ModelWithAttributes.original_bar
+
+    assert_deprecated do
+      assert_equal "original bar", ModelWithAttributes.original_bar
+    end
+
     assert_equal "bar", ModelWithAttributes.bar
-    ModelWithAttributes.define_attr_method(:bar)
+    ActiveSupport::Deprecation.silence do
+      ModelWithAttributes.define_attr_method(:bar)
+    end
     assert !ModelWithAttributes.bar
   end
 
   test '#define_attr_method generates attribute method with invalid identifier characters' do
-    ModelWithWeirdNamesAttributes.define_attr_method(:'c?d', 'c?d')
+    ActiveSupport::Deprecation.silence do
+      ModelWithWeirdNamesAttributes.define_attr_method(:'c?d', 'c?d')
+    end
 
     assert_respond_to ModelWithWeirdNamesAttributes, :'c?d'
-    assert_equal "original c?d", ModelWithWeirdNamesAttributes.send('original_c?d')
+
+    ActiveSupport::Deprecation.silence do
+      assert_equal "original c?d", ModelWithWeirdNamesAttributes.send('original_c?d')
+    end
     assert_equal "c?d", ModelWithWeirdNamesAttributes.send('c?d')
   end
 

activerecord/lib/active_record/base.rb

@@ -594,13 +594,13 @@ def deprecated_property_setter(property, value, block) #:nodoc:
             "to get the default #{property} where you would have called `original_#{property}`."
           )
 
-          define_attr_method property, value, &block
+          define_attr_method property, value, false, &block
         else
           ActiveSupport::Deprecation.warn(
             "Calling set_#{property} is deprecated. Please use `self.#{property} = 'the_name'` instead."
           )
 
-          define_attr_method property, value
+          define_attr_method property, value, false
         end
       end
 

activerecord/test/cases/base_test.rb

@@ -1410,23 +1410,6 @@ def test_reload_with_exclusive_scope
     assert_equal dev, dev.reload
   end
 
-  def test_define_attr_method_with_value
-    k = Class.new( ActiveRecord::Base )
-    k.send(:define_attr_method, :table_name, "foo")
-    assert_equal "foo", k.table_name
-  end
-
-  def test_define_attr_method_with_block
-    k = Class.new( ActiveRecord::Base ) do
-      class << self
-        attr_accessor :foo_key
-      end
-    end
-    k.foo_key = "id"
-    k.send(:define_attr_method, :foo_key) { "sys_" + original_foo_key }
-    assert_equal "sys_id", k.foo_key
-  end
-
   def test_set_table_name_with_value
     k = Class.new( ActiveRecord::Base )
     k.table_name = "foo"
@@ -1464,7 +1447,9 @@ def test_set_table_name_with_block
     k = Class.new( ActiveRecord::Base )
     assert_deprecated do
       k.set_table_name "foo"
-      k.set_table_name { original_table_name + "ks" }
+      k.set_table_name do
+        ActiveSupport::Deprecation.silence { original_table_name } + "ks"
+      end
     end
     assert_equal "fooks", k.table_name
   end
@@ -1510,7 +1495,9 @@ def test_set_primary_key_with_block
     k.primary_key = 'id'
 
     assert_deprecated do
-      k.set_primary_key { "sys_" + original_primary_key }
+      k.set_primary_key do
+        "sys_" + ActiveSupport::Deprecation.silence { original_primary_key }
+      end
     end
     assert_equal "sys_id", k.primary_key
   end
@@ -1547,7 +1534,9 @@ def test_set_inheritance_column_with_value
   def test_set_inheritance_column_with_block
     k = Class.new( ActiveRecord::Base )
     assert_deprecated do
-      k.set_inheritance_column { original_inheritance_column + "_id" }
+      k.set_inheritance_column do
+        ActiveSupport::Deprecation.silence { original_inheritance_column } + "_id"
+      end
     end
     assert_equal "type_id", k.inheritance_column
   end
@@ -1580,7 +1569,9 @@ def test_set_sequence_name_with_block
     return skip "sequences not supported by db" unless orig_name
 
     assert_deprecated do
-      k.set_sequence_name { original_sequence_name + "_lol" }
+      k.set_sequence_name do
+        ActiveSupport::Deprecation.silence { original_sequence_name } + "_lol"
+      end
     end
     assert_equal orig_name + "_lol", k.sequence_name
   end

activerecord/test/cases/locking_test.rb

@@ -243,7 +243,9 @@ def test_set_locking_column_with_block
     k.locking_column = 'foo'
 
     assert_deprecated do
-      k.set_locking_column { "lock_" + original_locking_column }
+      k.set_locking_column do
+        "lock_" + ActiveSupport::Deprecation.silence { original_locking_column }
+      end
     end
     assert_equal "lock_foo", k.locking_column
   end