Merge pull request #6638 from frodsan/config_accessor

add instance_accessor option to config_accessor

3 files changed, 82 insertions, 9 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index af9e8c27c7..c778647ea5 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,5 +1,17 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Add `:instance_accessor` option for `config_accessor`.
+
+        class User
+          include ActiveSupport::Configurable
+          config_accessor :allowed_access, instance_accessor: false
+        end
+
+        User.new.allowed_access = true # => NoMethodError
+        User.new.allowed_access        # => NoMethodError
+
+    *Francesco Rodriguez*
+
 *   ActionView::Helpers::NumberHelper methods have been moved to ActiveSupport::NumberHelper and are now available via
     Numeric#to_s.  Numeric#to_s now accepts the formatting  options :phone, :currency, :percentage, :delimited,
     :rounded, :human, and :human_size. *Andrew Mutz*
diff --git a/activesupport/lib/active_support/configurable.rb b/activesupport/lib/active_support/configurable.rb
index a8aa53a80f..4fb8c7af3f 100644
--- a/activesupport/lib/active_support/configurable.rb
+++ b/activesupport/lib/active_support/configurable.rb
@@ -37,29 +37,77 @@ module ActiveSupport
         yield config
       end
 
-      # Allows you to add shortcut so that you don't have to refer to attribute through config.
-      # Also look at the example for config to contrast.
+      # Allows you to add shortcut so that you don't have to refer to attribute
+      # through config. Also look at the example for config to contrast.
+      # 
+      # Defines both class and instance config accessors.
       #
       #   class User
       #     include ActiveSupport::Configurable
       #     config_accessor :allowed_access
       #   end
       #
+      #   User.allowed_access # => nil 
+      #   User.allowed_access = false
+      #   User.allowed_access # => false   
+      # 
       #   user = User.new
+      #   user.allowed_access # => false
       #   user.allowed_access = true
       #   user.allowed_access # => true
       #
+      #   User.allowed_access # => false  
+      #
+      # The attribute name must be a valid method name in Ruby.
+      #
+      #   class User
+      #     include ActiveSupport::Configurable
+      #     config_accessor :"1_Badname"
+      #   end
+      #   # => NameError: invalid config attribute name
+      #
+      # 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 User
+      #     include ActiveSupport::Configurable
+      #     config_accessor :allowed_access, instance_reader: false, instance_writer: false
+      #   end
+      #
+      #   User.allowed_access = false
+      #   User.allowed_access # => false
+      #
+      #   User.new.allowed_access = true # => NoMethodError
+      #   User.new.allowed_access        # => NoMethodError
+      #
+      # Or pass <tt>instance_accessor: false</tt>, to opt out both instance methods.
+      #
+      #   class User
+      #     include ActiveSupport::Configurable
+      #     config_accessor :allowed_access, instance_accessor: false
+      #   end
+      #
+      #   User.allowed_access = false
+      #   User.allowed_access # => false
+      #
+      #   User.new.allowed_access = true # => NoMethodError
+      #   User.new.allowed_access        # => NoMethodError  
       def config_accessor(*names)
         options = names.extract_options!
 
         names.each do |name|
+          raise NameError.new('invalid config attribute name') unless name =~ /^[_A-Za-z]\w*$/
+
           reader, line = "def #{name}; config.#{name}; end", __LINE__
           writer, line = "def #{name}=(value); config.#{name} = value; end", __LINE__
 
           singleton_class.class_eval reader, __FILE__, line
           singleton_class.class_eval writer, __FILE__, line
-          class_eval reader, __FILE__, line unless options[:instance_reader] == false
-          class_eval writer, __FILE__, line unless options[:instance_writer] == false
+
+          unless options[:instance_accessor] == false
+            class_eval reader, __FILE__, line unless options[:instance_reader] == false
+            class_eval writer, __FILE__, line unless options[:instance_writer] == false
+          end
         end
       end
     end
@@ -79,7 +127,6 @@ module ActiveSupport
     #
     #   user.config.allowed_access # => true
     #   user.config.level          # => 1
-    #
     def config
       @_config ||= self.class.config.inheritable_copy
     end
diff --git a/activesupport/test/configurable_test.rb b/activesupport/test/configurable_test.rb
index 2e5ea2c360..da7729d066 100644
--- a/activesupport/test/configurable_test.rb
+++ b/activesupport/test/configurable_test.rb
@@ -5,7 +5,8 @@ class ConfigurableActiveSupport < ActiveSupport::TestCase
   class Parent
     include ActiveSupport::Configurable
     config_accessor :foo
-    config_accessor :bar, :instance_reader => false, :instance_writer => false
+    config_accessor :bar, instance_reader: false, instance_writer: false
+    config_accessor :baz, instance_accessor: false
   end
 
   class Child < Parent
@@ -19,13 +20,13 @@ class ConfigurableActiveSupport < ActiveSupport::TestCase
   end
 
   test "adds a configuration hash" do
-    assert_equal({ :foo => :bar }, Parent.config)
+    assert_equal({ foo: :bar }, Parent.config)
   end
 
   test "adds a configuration hash to a module as well" do
     mixin = Module.new { include ActiveSupport::Configurable }
     mixin.config.foo = :bar
-    assert_equal({ :foo => :bar }, mixin.config)
+    assert_equal({ foo: :bar }, mixin.config)
   end
 
   test "configuration hash is inheritable" do
@@ -39,8 +40,12 @@ class ConfigurableActiveSupport < ActiveSupport::TestCase
 
   test "configuration accessors is not available on instance" do
     instance = Parent.new
+
     assert !instance.respond_to?(:bar)
     assert !instance.respond_to?(:bar=)
+
+    assert !instance.respond_to?(:baz)
+    assert !instance.respond_to?(:baz=)
   end
 
   test "configuration hash is available on instance" do
@@ -71,6 +76,15 @@ class ConfigurableActiveSupport < ActiveSupport::TestCase
     assert_method_defined child.new.config, :bar
   end
 
+  test "should raise name error if attribute name is invalid" do
+    assert_raises NameError do
+      Class.new do 
+        include ActiveSupport::Configurable
+        config_accessor "invalid attribute name"
+      end
+    end
+  end
+
   def assert_method_defined(object, method)
     methods = object.public_methods.map(&:to_s)
     assert methods.include?(method.to_s), "Expected #{methods.inspect} to include #{method.to_s.inspect}"
@@ -80,4 +94,4 @@ class ConfigurableActiveSupport < ActiveSupport::TestCase
     methods = object.public_methods.map(&:to_s)
     assert !methods.include?(method.to_s), "Expected #{methods.inspect} to not include #{method.to_s.inspect}"
   end
-end
+end
\ No newline at end of file