Backport #8284

Expected behavior of Module#public_class_method

Added by Jack Nagel over 2 years ago. Updated about 2 years ago.

[ruby-core:54404]
Status:Assigned
Priority:Normal
Assignee:Usaku NAKAMURA

Description

It was pointed out by Myron Marston (here: https://gist.github.com/myronmarston/5401829) that calling public_class_method on an anonymous module makes the named method public on Object, etc.

Object.define_method => NoMethodError
Module.new.public_class_method(:define_method)
Object.define_method => ArgumentError

Also of note, the same is not true when calling it on a class:

Object.define_method => NoMethodError
Class.new.public_class_method(:define_method)
Object.define_method => NoMethodError

which seems inconsistent given:

Module.new.singleton_class.ancestors => [Module, Object, Kernel, BasicObject]
Class.new.singleton_class.ancestors => [Class, Module, Object, Kernel, BasicObject]

Is this a bug or expected behavior? If the latter, what is the correct explanation?

Associated revisions

Revision 40346
Added by Nobuyoshi Nakada over 2 years ago

vm_method.c: fix visibility on anonymous module

  • vm_method.c (rb_mod_public_method): fix visibility on anonymous module. set visibility of singleton method, not method in base class. [Bug #8284]

Revision 40371
Added by Nobuyoshi Nakada over 2 years ago

test_module.rb: rename

  • test/ruby/test_module.rb (test_visibility_by_public_class_method): rename because anonymousness is not a point. [Bug #8284]

History

#1 Updated by Nobuyoshi Nakada over 2 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r40346.
Jack, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


vm_method.c: fix visibility on anonymous module

  • vm_method.c (rb_mod_public_method): fix visibility on anonymous module. set visibility of singleton method, not method in base class. [Bug #8284]

#2 Updated by Nobuyoshi Nakada over 2 years ago

  • Project changed from Ruby trunk to Backport200
  • Assignee set to Tomoyuki Chikanaga
  • Status changed from Closed to Assigned
  • Tracker changed from Bug to Backport

This is a very very longstanding, 16 years old bug, since 1.0.

#3 Updated by Andri Möll over 2 years ago

Well, it was actually reported by me to the RSpec project (https://github.com/rspec/rspec-core/pull/873) and the example I gave there involved a regular, not an anonymous module. The patch you made, nobu, tests only with an anonymous module, but does the fix also solve the visibility leakage for non anonymous modules?

With Ruby v1.9.3p385:

Object.define_method
NoMethodError: private method define_method' called for Object:Class
from (irb):1
from /usr/local/bin/irb:12:in
'
module Foo
public_class_method :define_method
end
=> Foo
Object.define_method
ArgumentError: wrong number of arguments (0 for 1)
from (irb):3:in define_method'
from (irb):3
from /usr/local/bin/irb:12:in
'

#4 Updated by Jack Nagel over 2 years ago

It does:

irb(main):001:0> RUBY_DESCRIPTION
=> "ruby 2.1.0dev (2013-04-19 trunk 40359) [i386-darwin10.8.0]"
irb(main):002:0> Object.define_method
NoMethodError: private method define_method' called for Object:Class
from (irb):2
from /Users/jacknagel/.rubies/ruby-2.1.0-dev/bin/irb:12:in
'
irb(main):003:0> module Foo; public_class_method :define_method; end
=> Foo
irb(main):004:0> Object.define_method
NoMethodError: private method define_method' called for Object:Class
from (irb):4
from /Users/jacknagel/.rubies/ruby-2.1.0-dev/bin/irb:12:in
'

#5 Updated by Jack Nagel over 2 years ago

As an aside, I see that this is now marked Backport 200, but it would be great if it was backported to 1.9.3 as well.

#6 Updated by Marc-Andre Lafortune over 2 years ago

jacknagel (Jack Nagel) wrote:

As an aside, I see that this is now marked Backport 200, but it would be great if it was backported to 1.9.3 as well.

I don't know if it will be, but in any case there is always singleton_class.send :public, :define_method instead or equivalent.

It's a bit more clear, IMO, as using "public_class_method" on a Module is a bit strange... Modules don't really have "class methods".

FWIW, there are no specs in RubySpec about "public_class_method" on a Module.

#7 Updated by Andri Möll over 2 years ago

jacknagel (Jack Nagel) wrote:

It does:

That's good. Now there's only a test missing for that. :)

#8 Updated by Tomoyuki Chikanaga over 2 years ago

I'll backport r40346 and r40371, and then move this ticket to Backport93.

#9 Updated by Tomoyuki Chikanaga over 2 years ago

  • Status changed from Assigned to Closed

This issue was solved with changeset r40427.
Jack, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


merge revision(s) 40346: [Backport #8284]

* vm_method.c (rb_mod_public_method): fix visibility on anonymous
  module. set visibility of singleton method, not method in base
  class.   [Bug #8284]

#10 Updated by Tomoyuki Chikanaga over 2 years ago

  • Assignee changed from Tomoyuki Chikanaga to Usaku NAKAMURA
  • Project changed from Backport200 to Backport193
  • Status changed from Closed to Assigned

#11 Updated by Usaku NAKAMURA about 2 years ago

Something is missing to backport the patch to ruby_1_9_3.
I'll check it later.

Also available in: Atom PDF