Don't display singleton class in Method#inspect unless method defined there#2949
Conversation
… there Previously, if an object has a singleton class, and you call Object#method on the object, the resulting string would include the object's singleton class, even though the method was not defined in the singleton class. Change this so the we only show the singleton class if the method is defined in the singleton class. Fixes [Bug ruby#15608]
|
Thanks for the PR! We should add proper specs for this explaining the intended behavior, the code is pretty obscure and so are the MRI tests (no description of the behavior). |
|
It should have the same behavior. Basically, just because the singleton class exists does not mean the method is defined in it. Method#inspect should only display the singleton class if that is where the method is defined. Briefly: class A; def a; end end
A.new.method(:a)
# => #<Method: A#a() (irb):1>
# correct, class A is where method a is defined
A.new.tap{|x| def x.a; end}.method(:a)
#=> #<Method: #<A:0x00001b32d37e0178>.a() (irb):5>
# correct, singleton class of instance is where method a is defined
A.new.tap(&:singleton_class).method(:a)
# => #<Method: #<A:0x00001b32759cd9a8>.a() (irb):1>
# incorrect, singleton class of instance exists, but that is not where method a is definedI'll push up some specs for this, hopefully today if I can get to it. |
|
That sounds great :) |
| obj.singleton_class | ||
| @m = obj.method(:bar) | ||
| @string = @m.send(@method).sub(/0x\w+/, '0xXXXXXX') | ||
| @string.should =~ /MethodSpecs::MySub\(MethodSpecs::MyMod\)/ |
There was a problem hiding this comment.
Could we be more precise and just check the exact result/full string with ==? (same below)
There was a problem hiding this comment.
Sure, I'll make that change. It will make the new specs inconsistent with the existing specs, though.
There was a problem hiding this comment.
I tried that, but the path to the file and line number is included in the spec output, which would have to be masked out similar to the object id masking. I really don't think it is a good idea to do that. I will add a commit that uses a more complete regexp, though.
There was a problem hiding this comment.
Thanks, it looks good.
I'll try to make a pass over the specs in this file to improve them.
* Fix gem pristine not accounting for user installed gems. Pull request
ruby#2914 by Luis Sagastume.
* Refactor keyword argument test for Ruby 2.7. Pull request ruby#2947 by
SHIBATA Hiroshi.
* Fix errors at frozen Gem::Version. Pull request ruby#2949 by Nobuyoshi
Nakada.
* Remove taint usage on Ruby 2.7+. Pull request ruby#2951 by Jeremy Evans.
* Check Manifest.txt is up to date. Pull request ruby#2953 by David Rodríguez.
* Clarify symlink conditionals in tests. Pull request ruby#2962 by David
Rodríguez.
* Update command line parsing to work under ps. Pull request ruby#2966 by
David Rodríguez.
* Properly test `Gem::Specifications.stub_for`. Pull request ruby#2970 by
David Rodríguez.
* Fix Gem::LOADED_SPECS_MUTEX handling for recursive locking. Pull request
ruby#2985 by MSP-Greg.
Previously, if an object has a singleton class, and you call
Object#method on the object, the resulting string would include
the object's singleton class, even though the method was not
defined in the singleton class.
Change this so the we only show the singleton class if the method
is defined in the singleton class.
Fixes [Bug #15608]