Test define method visibility

core/src/main/java/org/jruby/RubyModule.java

@@ -1868,10 +1868,19 @@ public IRubyObject newMethod(IRubyObject receiver, final String methodName, bool
 
     @JRubyMethod(name = "define_method", visibility = PRIVATE, reads = VISIBILITY)
     public IRubyObject define_method(ThreadContext context, IRubyObject arg0, Block block) {
-        Visibility visibility = context.getCurrentVisibility();
+        Visibility visibility = getVisibilityForDefineMethod(context);
+
         return defineMethodFromBlock(context, arg0, block, visibility);
     }
 
+    private Visibility getVisibilityForDefineMethod(ThreadContext context) {
+        Visibility visibility = PUBLIC;
+
+        // These checks are similar to rb_vm_cref_in_context from MRI.
+        if (context.getCurrentFrame().getSelf() == this) visibility = context.getCurrentVisibility();
+        return visibility;
+    }
+
     public IRubyObject defineMethodFromBlock(ThreadContext context, IRubyObject arg0, Block block, Visibility visibility) {
         final Ruby runtime = context.runtime;
         RubySymbol nameSym = TypeConverter.checkID(arg0);
@@ -1913,7 +1922,8 @@ public IRubyObject defineMethodFromBlock(ThreadContext context, IRubyObject arg0
 
     @JRubyMethod(name = "define_method", visibility = PRIVATE, reads = VISIBILITY)
     public IRubyObject define_method(ThreadContext context, IRubyObject arg0, IRubyObject arg1, Block block) {
-        Visibility visibility = context.getCurrentVisibility();
+        Visibility visibility = getVisibilityForDefineMethod(context);
+
         return defineMethodFromCallable(context, arg0, arg1, visibility);
     }
 

spec/ruby/core/module/define_method_spec.rb

@@ -73,19 +73,60 @@ def test_method
 
 describe "Module#define_method when name is not a special private name" do
   describe "given an UnboundMethod" do
-    it "sets the visibility of the method to the current visibility" do
-      m = Module.new do
-        def foo
+    describe "and called from the target module" do
+      it "sets the visibility of the method to the current visibility" do
+        klass = Class.new do
+          define_method(:bar, ModuleSpecs::EmptyFooMethod)
+          private
+          define_method(:baz, ModuleSpecs::EmptyFooMethod)
         end
-        private :foo
+
+        klass.should have_public_instance_method(:bar)
+        klass.should have_private_instance_method(:baz)
       end
-      klass = Class.new do
-        define_method(:bar, m.instance_method(:foo))
-        private
-        define_method(:baz, m.instance_method(:foo))
+    end
+
+    describe "and called from another module" do
+      it "sets the visibility of the method to public" do
+        klass = Class.new
+        Class.new do
+          klass.send(:define_method, :bar, ModuleSpecs::EmptyFooMethod)
+          private
+          klass.send(:define_method, :baz, ModuleSpecs::EmptyFooMethod)
+        end
+
+        klass.should have_public_instance_method(:bar)
+        klass.should have_public_instance_method(:baz)
+      end
+    end
+  end
+
+  describe "passed a block" do
+    describe "and called from the target module" do
+      it "sets the visibility of the method to the current visibility" do
+        klass = Class.new do
+          define_method(:bar) {}
+          private
+          define_method(:baz) {}
+        end
+
+        klass.should have_public_instance_method(:bar)
+        klass.should have_private_instance_method(:baz)
+      end
+    end
+
+    describe "and called from another module" do
+      it "sets the visibility of the method to public" do
+        klass = Class.new
+        Class.new do
+          klass.send(:define_method, :bar) {}
+          private
+          klass.send(:define_method, :baz) {}
+        end
+
+        klass.should have_public_instance_method(:bar)
+        klass.should have_public_instance_method(:baz)
       end
-      klass.should have_public_instance_method(:bar)
-      klass.should have_private_instance_method(:baz)
     end
   end
 end

spec/ruby/core/module/fixtures/classes.rb

@@ -576,6 +576,13 @@ def self.===(*)
       raise 'method contents are irrelevant to test'
     end
   end
+
+  m = Module.new do
+    def foo
+    end
+    private :foo
+  end
+  EmptyFooMethod = m.instance_method(:foo)
 end
 
 class Object

spec/truffle/tags/core/module/define_method_tags.txt

@@ -0,0 +1,2 @@
+fails:Module#define_method when name is not a special private name given an UnboundMethod and called from another module sets the visibility of the method to public
+fails:Module#define_method when name is not a special private name passed a block and called from another module sets the visibility of the method to public