Let Ruby deal with method visibility.

Check respond_to_without_attributes? in method_missing. If there is any
method that responds (even private), let super handle it and raise
NoMethodError if necessary.

2 files changed, 47 insertions, 13 deletions

diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb
index c5f7a21d3f..baebc91192 100644
--- a/activemodel/lib/active_model/attribute_methods.rb
+++ b/activemodel/lib/active_model/attribute_methods.rb
@@ -411,13 +411,18 @@ module ActiveModel
     # It's also possible to instantiate related objects, so a Client class
     # belonging to the clients table with a +master_id+ foreign key can
     # instantiate master through Client#master.
-    def method_missing(method_id, *args, &block)
-      method_name = method_id.to_s
-      if match = match_attribute_method?(method_name)
-        guard_private_attribute_method!(method_name, args)
-        return __send__(match.target, match.attr_name, *args, &block)
+    def method_missing(method, *args, &block)
+      if respond_to_without_attributes?(method, true)
+        super
+      else
+        match = match_attribute_method?(method.to_s)
+
+        if match
+          __send__(match.target, match.attr_name, *args, &block)
+        else
+          super
+        end
       end
-      super
     end
 
     # A Person object with a name attribute can ask <tt>person.respond_to?(:name)</tt>,
@@ -450,13 +455,6 @@ module ActiveModel
         match && attribute_method?(match.attr_name) ? match : nil
       end
 
-      # prevent method_missing from calling private methods with #send
-      def guard_private_attribute_method!(method_name, args)
-        if self.class.private_method_defined?(method_name)
-          raise NoMethodError.new("Attempt to call private method `#{method_name}'", method_name, args)
-        end
-      end
-
       def missing_attribute(attr_name, stack)
         raise ActiveModel::MissingAttributeError, "missing attribute: #{attr_name}", stack
       end
diff --git a/activemodel/test/cases/attribute_methods_test.rb b/activemodel/test/cases/attribute_methods_test.rb
index 61ff5fca7a..198e4c3c06 100644
--- a/activemodel/test/cases/attribute_methods_test.rb
+++ b/activemodel/test/cases/attribute_methods_test.rb
@@ -32,6 +32,16 @@ private
   end
 
   alias attribute_test attribute
+
+  def private_method
+    "<3 <3"
+  end
+
+protected
+
+  def protected_method
+    "O_o O_o"
+  end
 end
 
 class ModelWithAttributesWithSpaces
@@ -151,4 +161,30 @@ class AttributeMethodsTest < ActiveModel::TestCase
 
     assert_equal 'value of foo', klass.new.foo
   end
+
+  test 'should not interfere with method_missing if the attr has a private/protected method' do
+    m = ModelWithAttributes2.new
+    m.attributes = { 'private_method' => '<3', 'protected_method' => 'O_o' }
+
+    # dispatches to the *method*, not the attribute
+    assert_equal '<3 <3',   m.send(:private_method)
+    assert_equal 'O_o O_o', m.send(:protected_method)
+
+    # sees that a method is already defined, so doesn't intervene
+    assert_raises(NoMethodError) { m.private_method }
+    assert_raises(NoMethodError) { m.protected_method }
+  end
+
+  test 'should not interfere with respond_to? if the attribute has a private/protected method' do
+    m = ModelWithAttributes2.new
+    m.attributes = { 'private_method' => '<3', 'protected_method' => 'O_o' }
+
+    assert !m.respond_to?(:private_method)
+    assert m.respond_to?(:private_method, true)
+
+    # This is messed up, but it's how Ruby works at the moment. Apparently it will be changed
+    # in the future.
+    assert m.respond_to?(:protected_method)
+    assert m.respond_to?(:protected_method, true)
+  end
 end