Unify cattr and mattr accessors declarations

6 files changed, 194 insertions, 269 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 9c7078b9a4..eec290ea79 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   Both `cattr_*` and `mattr_*` method definitions now live in `active_support/core_ext/module/attribute_accessors`.
+
+    Requires to `active_support/core_ext/class/attribute_accessors` are
+    deprecated and will be removed in Ruby on Rails 4.2.
+
+    *Genadi Samokovarov*
+
 *   Deprecated `Numeric#{ago,until,since,from_now}`, the user is expected to explicitly
     convert the value into an AS::Duration, i.e. `5.ago` => `5.seconds.ago`
 
diff --git a/activesupport/lib/active_support/core_ext/class.rb b/activesupport/lib/active_support/core_ext/class.rb
index 86b752c2f3..c750a10bb2 100644
--- a/activesupport/lib/active_support/core_ext/class.rb
+++ b/activesupport/lib/active_support/core_ext/class.rb
@@ -1,4 +1,3 @@
 require 'active_support/core_ext/class/attribute'
-require 'active_support/core_ext/class/attribute_accessors'
 require 'active_support/core_ext/class/delegating_attributes'
 require 'active_support/core_ext/class/subclasses'
diff --git a/activesupport/lib/active_support/core_ext/class/attribute_accessors.rb b/activesupport/lib/active_support/core_ext/class/attribute_accessors.rb
index 0cf955b889..083b165dce 100644
--- a/activesupport/lib/active_support/core_ext/class/attribute_accessors.rb
+++ b/activesupport/lib/active_support/core_ext/class/attribute_accessors.rb
@@ -1,181 +1,6 @@
-require 'active_support/core_ext/array/extract_options'
+require 'active_support/deprecation'
+require 'active_support/core_ext/module/attribute_accessors'
 
-# Extends the class object with class and instance accessors for class attributes,
-# just like the native attr* accessors for instance attributes.
-class Class
-  # Defines a class attribute if it's not defined and creates a reader method that
-  # returns the attribute value.
-  #
-  #   class Person
-  #     cattr_reader :hair_colors
-  #   end
-  #
-  #   Person.class_variable_set("@@hair_colors", [:brown, :black])
-  #   Person.hair_colors     # => [:brown, :black]
-  #   Person.new.hair_colors # => [:brown, :black]
-  #
-  # The attribute name must be a valid method name in Ruby.
-  #
-  #   class Person
-  #     cattr_reader :"1_Badname "
-  #   end
-  #   # => NameError: invalid attribute name
-  #
-  # If you want to opt out the instance reader method, you can pass <tt>instance_reader: false</tt>
-  # or <tt>instance_accessor: false</tt>.
-  #
-  #   class Person
-  #     cattr_reader :hair_colors, instance_reader: false
-  #   end
-  #
-  #   Person.new.hair_colors # => NoMethodError
-  #
-  # Also, you can pass a block to set up the attribute with a default value.
-  #
-  #   class Person
-  #     cattr_reader :hair_colors do
-  #       [:brown, :black, :blonde, :red]
-  #     end
-  #   end
-  #
-  #   Person.hair_colors # => [:brown, :black, :blonde, :red]
-  def cattr_reader(*syms)
-    options = syms.extract_options!
-    syms.each do |sym|
-      raise NameError.new("invalid class attribute name: #{sym}") unless sym =~ /^[_A-Za-z]\w*$/
-      class_eval(<<-EOS, __FILE__, __LINE__ + 1)
-        unless defined? @@#{sym}
-          @@#{sym} = nil
-        end
-
-        def self.#{sym}
-          @@#{sym}
-        end
-      EOS
-
-      unless options[:instance_reader] == false || options[:instance_accessor] == false
-        class_eval(<<-EOS, __FILE__, __LINE__ + 1)
-          def #{sym}
-            @@#{sym}
-          end
-        EOS
-      end
-      class_variable_set("@@#{sym}", yield) if block_given?
-    end
-  end
-
-  # Defines a class attribute if it's not defined and creates a writer method to allow
-  # assignment to the attribute.
-  #
-  #   class Person
-  #     cattr_writer :hair_colors
-  #   end
-  #
-  #   Person.hair_colors = [:brown, :black]
-  #   Person.class_variable_get("@@hair_colors") # => [:brown, :black]
-  #   Person.new.hair_colors = [:blonde, :red]
-  #   Person.class_variable_get("@@hair_colors") # => [:blonde, :red]
-  #
-  # The attribute name must be a valid method name in Ruby.
-  #
-  #   class Person
-  #     cattr_writer :"1_Badname "
-  #   end
-  #   # => NameError: invalid attribute name
-  #
-  # If you want to opt out the instance writer method, pass <tt>instance_writer: false</tt>
-  # or <tt>instance_accessor: false</tt>.
-  #
-  #   class Person
-  #     cattr_writer :hair_colors, instance_writer: false
-  #   end
-  #
-  #   Person.new.hair_colors = [:blonde, :red] # => NoMethodError
-  #
-  # Also, you can pass a block to set up the attribute with a default value.
-  #
-  #   class Person
-  #     cattr_writer :hair_colors do
-  #       [:brown, :black, :blonde, :red]
-  #     end
-  #   end
-  #
-  #   Person.class_variable_get("@@hair_colors") # => [:brown, :black, :blonde, :red]
-  def cattr_writer(*syms)
-    options = syms.extract_options!
-    syms.each do |sym|
-      raise NameError.new("invalid class attribute name: #{sym}") unless sym =~ /^[_A-Za-z]\w*$/
-      class_eval(<<-EOS, __FILE__, __LINE__ + 1)
-        unless defined? @@#{sym}
-          @@#{sym} = nil
-        end
-
-        def self.#{sym}=(obj)
-          @@#{sym} = obj
-        end
-      EOS
-
-      unless options[:instance_writer] == false || options[:instance_accessor] == false
-        class_eval(<<-EOS, __FILE__, __LINE__ + 1)
-          def #{sym}=(obj)
-            @@#{sym} = obj
-          end
-        EOS
-      end
-      send("#{sym}=", yield) if block_given?
-    end
-  end
-
-  # Defines both class and instance accessors for class attributes.
-  #
-  #   class Person
-  #     cattr_accessor :hair_colors
-  #   end
-  #
-  #   Person.hair_colors = [:brown, :black, :blonde, :red]
-  #   Person.hair_colors     # => [:brown, :black, :blonde, :red]
-  #   Person.new.hair_colors # => [:brown, :black, :blonde, :red]
-  #
-  # If a subclass changes the value then that would also change the value for
-  # parent class. Similarly if parent class changes the value then that would
-  # change the value of subclasses too.
-  #
-  #   class Male < Person
-  #   end
-  #
-  #   Male.hair_colors << :blue
-  #   Person.hair_colors # => [:brown, :black, :blonde, :red, :blue]
-  #
-  # To opt out of the instance writer method, pass <tt>instance_writer: false</tt>.
-  # To opt out of the instance reader method, pass <tt>instance_reader: false</tt>.
-  #
-  #   class Person
-  #     cattr_accessor :hair_colors, instance_writer: false, instance_reader: false
-  #   end
-  #
-  #   Person.new.hair_colors = [:brown]  # => NoMethodError
-  #   Person.new.hair_colors             # => NoMethodError
-  #
-  # Or pass <tt>instance_accessor: false</tt>, to opt out both instance methods.
-  #
-  #   class Person
-  #     cattr_accessor :hair_colors, instance_accessor: false
-  #   end
-  #
-  #   Person.new.hair_colors = [:brown]  # => NoMethodError
-  #   Person.new.hair_colors             # => NoMethodError
-  #
-  # Also you can pass a block to set up the attribute with a default value.
-  #
-  #   class Person
-  #     cattr_accessor :hair_colors do
-  #       [:brown, :black, :blonde, :red]
-  #     end
-  #   end
-  #
-  #   Person.class_variable_get("@@hair_colors") # => [:brown, :black, :blonde, :red]
-  def cattr_accessor(*syms, &blk)
-    cattr_reader(*syms)
-    cattr_writer(*syms, &blk)
-  end
-end
+ActiveSupport::Deprecation.warn(
+  "The cattr_* method definitions have been moved into active_support/core_ext/module/attribute_accessors. Please require that instead."
+)
diff --git a/activesupport/lib/active_support/core_ext/module/attribute_accessors.rb b/activesupport/lib/active_support/core_ext/module/attribute_accessors.rb
index 672cc0256f..f70a839074 100644
--- a/activesupport/lib/active_support/core_ext/module/attribute_accessors.rb
+++ b/activesupport/lib/active_support/core_ext/module/attribute_accessors.rb
@@ -1,10 +1,59 @@
 require 'active_support/core_ext/array/extract_options'
 
+# Extends the module object with class/module and instance accessors for
+# class/module attributes, just like the native attr* accessors for instance
+# attributes.
 class Module
+  # Defines a class attribute and creates a class and instance reader methods.
+  # The underlying the class variable is set to +nil+, if it is not previously
+  # defined.
+  #
+  #   module HairColors
+  #     mattr_reader :hair_colors
+  #   end
+  #
+  #   HairColors.hair_colors # => nil
+  #   HairColors.class_variable_set("@@hair_colors", [:brown, :black])
+  #   HairColors.hair_colors # => [:brown, :black]
+  #
+  # The attribute name must be a valid method name in Ruby.
+  #
+  #   module Foo
+  #     mattr_reader :"1_Badname "
+  #   end
+  #   # => NameError: invalid attribute name
+  #
+  # If you want to opt out the creation on the instance reader method, pass
+  # <tt>instance_reader: false</tt> or <tt>instance_accessor: false</tt>.
+  #
+  #   module HairColors
+  #     mattr_writer :hair_colors, instance_reader: false
+  #   end
+  #
+  #   class Person
+  #     include HairColors
+  #   end
+  #
+  #   Person.new.hair_colors # => NoMethodError
+  #
+  #
+  # Also, you can pass a block to set up the attribute with a default value.
+  #
+  #   module HairColors
+  #     cattr_reader :hair_colors do
+  #       [:brown, :black, :blonde, :red]
+  #     end
+  #   end
+  #
+  #   class Person
+  #     include HairColors
+  #   end
+  #
+  #   Person.hair_colors # => [:brown, :black, :blonde, :red] +  #
   def mattr_reader(*syms)
     options = syms.extract_options!
     syms.each do |sym|
-      raise NameError.new('invalid attribute name') unless sym =~ /^[_A-Za-z]\w*$/
+      raise NameError.new("invalid attribute name: #{sym}") unless sym =~ /^[_A-Za-z]\w*$/
       class_eval(<<-EOS, __FILE__, __LINE__ + 1)
         @@#{sym} = nil unless defined? @@#{sym}
 
@@ -20,14 +69,60 @@ class Module
           end
         EOS
       end
+      class_variable_set("@@#{sym}", yield) if block_given?
     end
   end
+  alias :cattr_reader :mattr_reader
 
+  # Defines a class attribute and creates a class and instance writer methods to
+  # allow assignment to the attribute.
+  #
+  #   module HairColors
+  #     mattr_writer :hair_colors
+  #   end
+  #
+  #   class Person
+  #     include HairColors
+  #   end
+  #
+  #   HairColors.hair_colors = [:brown, :black]
+  #   Person.class_variable_get("@@hair_colors") # => [:brown, :black]
+  #   Person.new.hair_colors = [:blonde, :red]
+  #   HairColors.class_variable_get("@@hair_colors") # => [:blonde, :red]
+  #
+  # If you want to opt out the instance writer method, pass
+  # <tt>instance_writer: false</tt> or <tt>instance_accessor: false</tt>.
+  #
+  #   module HairColors
+  #     mattr_writer :hair_colors, instance_writer: false
+  #   end
+  #
+  #   class Person
+  #     include HairColors
+  #   end
+  #
+  #   Person.new.hair_colors = [:blonde, :red] # => NoMethodError
+  #
+  # Also, you can pass a block to set up the attribute with a default value.
+  #
+  #   class HairColors
+  #     mattr_writer :hair_colors do
+  #       [:brown, :black, :blonde, :red]
+  #     end
+  #   end
+  #
+  #   class Person
+  #     include HairColors
+  #   end
+  #
+  #   Person.class_variable_get("@@hair_colors") # => [:brown, :black, :blonde, :red]
   def mattr_writer(*syms)
     options = syms.extract_options!
     syms.each do |sym|
-      raise NameError.new('invalid attribute name') unless sym =~ /^[_A-Za-z]\w*$/
+      raise NameError.new("invalid attribute name: #{sym}") unless sym =~ /^[_A-Za-z]\w*$/
       class_eval(<<-EOS, __FILE__, __LINE__ + 1)
+        @@#{sym} = nil unless defined? @@#{sym}
+
         def self.#{sym}=(obj)
           @@#{sym} = obj
         end
@@ -40,27 +135,78 @@ class Module
           end
         EOS
       end
+      send("#{sym}=", yield) if block_given?
     end
   end
+  alias :cattr_writer :mattr_writer
 
-  # Extends the module object with module and instance accessors for class attributes,
-  # just like the native attr* accessors for instance attributes.
+  # Defines both class and instance accessors for class attributes.
   #
-  #   module AppConfiguration
-  #     mattr_accessor :google_api_key
+  #   module HairColors
+  #     mattr_accessor :hair_colors
+  #   end
   #
-  #     self.google_api_key = "123456789"
+  #   class Person
+  #     include HairColors
   #   end
   #
-  #   AppConfiguration.google_api_key # => "123456789"
-  #   AppConfiguration.google_api_key = "overriding the api key!"
-  #   AppConfiguration.google_api_key # => "overriding the api key!"
+  #   Person.hair_colors = [:brown, :black, :blonde, :red]
+  #   Person.hair_colors     # => [:brown, :black, :blonde, :red]
+  #   Person.new.hair_colors # => [:brown, :black, :blonde, :red]
+  #
+  # If a subclass changes the value then that would also change the value for
+  # parent class. Similarly if parent class changes the value then that would
+  # change the value of subclasses too.
+  #
+  #   class Male < Person
+  #   end
+  #
+  #   Male.hair_colors << :blue
+  #   Person.hair_colors # => [:brown, :black, :blonde, :red, :blue]
   #
   # To opt out of the instance writer method, pass <tt>instance_writer: false</tt>.
   # To opt out of the instance reader method, pass <tt>instance_reader: false</tt>.
-  # To opt out of both instance methods, pass <tt>instance_accessor: false</tt>.
-  def mattr_accessor(*syms)
-    mattr_reader(*syms)
-    mattr_writer(*syms)
+  #
+  #   module HairColors
+  #     mattr_accessor :hair_colors, instance_writer: false, instance_reader: false
+  #   end
+  #
+  #   class Person
+  #     include HairColors
+  #   end
+  #
+  #   Person.new.hair_colors = [:brown]  # => NoMethodError
+  #   Person.new.hair_colors             # => NoMethodError
+  #
+  # Or pass <tt>instance_accessor: false</tt>, to opt out both instance methods.
+  #
+  #   module HairColors
+  #     mattr_accessor :hair_colors, instance_acessor: false
+  #   end
+  #
+  #   class Person
+  #     include HairColors
+  #   end
+  #
+  #   Person.new.hair_colors = [:brown]  # => NoMethodError
+  #   Person.new.hair_colors             # => NoMethodError
+  #
+  # Also you can pass a block to set up the attribute with a default value.
+  #
+  #   module HairColors
+  #     mattr_accessor :hair_colors do
+  #       [:brown, :black, :blonde, :red]
+  #     end
+  #   end
+  #
+  #   class Person
+  #     include HairColors
+  #   end
+  #
+  #   Person.class_variable_get("@@hair_colors") #=> [:brown, :black, :blonde, :red]
+  def mattr_accessor(*syms, &blk)
+    mattr_reader(*syms, &blk)
+    mattr_writer(*syms, &blk)
   end
+  alias :cattr_accessor :mattr_accessor
 end
diff --git a/activesupport/test/core_ext/class/attribute_accessor_test.rb b/activesupport/test/core_ext/class/attribute_accessor_test.rb
deleted file mode 100644
index 3bc948f3a6..0000000000
--- a/activesupport/test/core_ext/class/attribute_accessor_test.rb
+++ /dev/null
@@ -1,70 +0,0 @@
-require 'abstract_unit'
-require 'active_support/core_ext/class/attribute_accessors'
-
-class ClassAttributeAccessorTest < ActiveSupport::TestCase
-  def setup
-    @class = Class.new do
-      cattr_accessor :foo
-      cattr_accessor :bar,  :instance_writer   => false
-      cattr_reader   :shaq, :instance_reader   => false
-      cattr_accessor :camp, :instance_accessor => false
-      cattr_accessor(:defa) { 'default_accessor_value' }
-      cattr_reader(:defr) { 'default_reader_value' }
-      cattr_writer(:defw) { 'default_writer_value' }
-    end
-    @object = @class.new
-  end
-
-  def test_should_use_mattr_default
-    assert_nil @class.foo
-    assert_nil @object.foo
-  end
-
-  def test_should_set_mattr_value
-    @class.foo = :test
-    assert_equal :test, @object.foo
-
-    @object.foo = :test2
-    assert_equal :test2, @class.foo
-  end
-
-  def test_should_not_create_instance_writer
-    assert_respond_to @class, :foo
-    assert_respond_to @class, :foo=
-    assert_respond_to @object, :bar
-    assert !@object.respond_to?(:bar=)
-  end
-
-  def test_should_not_create_instance_reader
-    assert_respond_to @class, :shaq
-    assert !@object.respond_to?(:shaq)
-  end
-
-  def test_should_not_create_instance_accessors
-    assert_respond_to @class, :camp
-    assert !@object.respond_to?(:camp)
-    assert !@object.respond_to?(:camp=)
-  end
-
-  def test_should_raise_name_error_if_attribute_name_is_invalid
-    exception = assert_raises NameError do
-      Class.new do
-        cattr_reader "1nvalid"
-      end
-    end
-    assert_equal "invalid class attribute name: 1nvalid", exception.message
-
-    exception = assert_raises NameError do
-      Class.new do
-        cattr_writer "1nvalid"
-      end
-    end
-    assert_equal "invalid class attribute name: 1nvalid", exception.message
-  end
-
-  def test_should_use_default_value_if_block_passed
-    assert_equal 'default_accessor_value', @class.defa
-    assert_equal 'default_reader_value', @class.defr
-    assert_equal 'default_writer_value', @class.class_variable_get('@@defw')
-  end
-end
diff --git a/activesupport/test/core_ext/module/attribute_accessor_test.rb b/activesupport/test/core_ext/module/attribute_accessor_test.rb
index a577f90bdd..48f3cc579f 100644
--- a/activesupport/test/core_ext/module/attribute_accessor_test.rb
+++ b/activesupport/test/core_ext/module/attribute_accessor_test.rb
@@ -8,6 +8,11 @@ class ModuleAttributeAccessorTest < ActiveSupport::TestCase
       mattr_accessor :bar, :instance_writer => false
       mattr_reader   :shaq, :instance_reader => false
       mattr_accessor :camp, :instance_accessor => false
+
+      cattr_accessor(:defa) { 'default_accessor_value' }
+      cattr_reader(:defr) { 'default_reader_value' }
+      cattr_writer(:defw) { 'default_writer_value' }
+      cattr_accessor(:quux) { :quux }
     end
     @class = Class.new
     @class.instance_eval { include m }
@@ -27,6 +32,11 @@ class ModuleAttributeAccessorTest < ActiveSupport::TestCase
     assert_equal :test2, @module.foo
   end
 
+  def test_cattr_accessor_default_value
+    assert_equal :quux, @module.quux
+    assert_equal :quux, @object.quux
+  end
+
   def test_should_not_create_instance_writer
     assert_respond_to @module, :foo
     assert_respond_to @module, :foo=
@@ -46,16 +56,24 @@ class ModuleAttributeAccessorTest < ActiveSupport::TestCase
   end
 
   def test_should_raise_name_error_if_attribute_name_is_invalid
-    assert_raises NameError do
+    exception = assert_raises NameError do
       Class.new do
-        mattr_reader "invalid attribute name"
+        cattr_reader "1nvalid"
       end
     end
+    assert_equal "invalid attribute name: 1nvalid", exception.message
 
-    assert_raises NameError do
+    exception = assert_raises NameError do
       Class.new do
-        mattr_writer "invalid attribute name"
+        cattr_writer "1nvalid"
       end
     end
+    assert_equal "invalid attribute name: 1nvalid", exception.message
+  end
+
+  def test_should_use_default_value_if_block_passed
+    assert_equal 'default_accessor_value', @module.defa
+    assert_equal 'default_reader_value', @module.defr
+    assert_equal 'default_writer_value', @module.class_variable_get('@@defw')
   end
 end