[ruby-core:101635] [Ruby master Bug#17428] Method#inspect bad output for class methods

[ruby-core:101635] [Ruby master Bug#17428] Method#inspect bad output for class methods

[marcandre-ruby-core]

Issue #17428 has been reported by marcandre (Marc-Andre Lafortune).

----------------------------------------
Bug #17428: Method#inspect bad output for class methods
https://bugs.ruby-lang.org/issues/17428

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: jeremyevans0 (Jeremy Evans)
* ruby -v: ruby 3.0.0dev (2020-12-22T17:37:59Z master 487355873a) [x86_64-darwin18]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
```
$ $ ruby -e 'p String.method(:prepend)'
# 2.7.0:
#<Method: String.prepend(*)>
# 3.0.0:
#<Method: #<Class:Object>(Module)#prepend(*)>
```

@jeremyevans found it shows the method as pertaining to one level too high (which is good for objects as we don't want to show the singleton class there, but not for classes).

Probably due to #15608



-- 
https://bugs.ruby-lang.org/

[ruby-core:101636] [Ruby master Bug#17428] Method#inspect bad output for class methods

[marcandre-ruby-core]

Issue #17428 has been updated by marcandre (Marc-Andre Lafortune).


Correct output would be:

```
#<Method: #<Class:String>(Module)#prepend(*)>
# or even better imo to reuse the '.' notation here:
#<Method: String(Module).prepend(*)>
```

----------------------------------------
Bug #17428: Method#inspect bad output for class methods
https://bugs.ruby-lang.org/issues/17428#change-89426

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: jeremyevans0 (Jeremy Evans)
* ruby -v: ruby 3.0.0dev (2020-12-22T17:37:59Z master 487355873a) [x86_64-darwin18]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
```
$ $ ruby -e 'p String.method(:prepend)'
# 2.7.0:
#<Method: String.prepend(*)>
# 3.0.0:
#<Method: #<Class:Object>(Module)#prepend(*)>
```

@jeremyevans found it shows the method as pertaining to one level too high (which is good for objects as we don't want to show the singleton class there, but not for classes).

Probably due to #15608



-- 
https://bugs.ruby-lang.org/

[ruby-core:101641] [Ruby master Bug#17428] Method#inspect bad output for class methods

[merch-redmine]

Issue #17428 has been updated by jeremyevans0 (Jeremy Evans).


I submitted a pull request to fix this issue: https://github.com/ruby/ruby/pull/3984

----------------------------------------
Bug #17428: Method#inspect bad output for class methods
https://bugs.ruby-lang.org/issues/17428#change-89432

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: jeremyevans0 (Jeremy Evans)
* ruby -v: ruby 3.0.0dev (2020-12-22T17:37:59Z master 487355873a) [x86_64-darwin18]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
```
$ $ ruby -e 'p String.method(:prepend)'
# 2.7.0:
#<Method: String.prepend(*)>
# 3.0.0:
#<Method: #<Class:Object>(Module)#prepend(*)>
```

@jeremyevans found it shows the method as pertaining to one level too high (which is good for objects as we don't want to show the singleton class there, but not for classes).

Probably due to #15608



-- 
https://bugs.ruby-lang.org/

[ruby-core:101647] [Ruby master Bug#17428] Method#inspect bad output for class methods

[eregontp]

Issue #17428 has been updated by Eregon (Benoit Daloze).


FWIW, current output on TruffleRuby is:
```
ruby -e 'p String.method(:prepend)'
#<Method: Class(Module)#prepend(*modules) <internal:core> core/module.rb:92>
```
Probably it should show `#<Class:String>` instead of `String` though.

```
#<Method: String(Module).prepend(*)>
```
doesn't seem correct, prepend is not a class method of `Module`, it's an instance method.

----------------------------------------
Bug #17428: Method#inspect bad output for class methods
https://bugs.ruby-lang.org/issues/17428#change-89437

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: jeremyevans0 (Jeremy Evans)
* ruby -v: ruby 3.0.0dev (2020-12-22T17:37:59Z master 487355873a) [x86_64-darwin18]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
```
$ $ ruby -e 'p String.method(:prepend)'
# 2.7.0:
#<Method: String.prepend(*)>
# 3.0.0:
#<Method: #<Class:Object>(Module)#prepend(*)>
```

@jeremyevans found it shows the method as pertaining to one level too high (which is good for objects as we don't want to show the singleton class there, but not for classes).

Probably due to #15608



-- 
https://bugs.ruby-lang.org/

[ruby-core:101648] [Ruby master Bug#17428] Method#inspect bad output for class methods

[eregontp]

Issue #17428 has been updated by Eregon (Benoit Daloze).


With a trivial fix (using the actual class/rb_class_of(), not just `.class`), I get this in TruffleRuby, which I think is the correct output:
```
$ ruby -e 'p String.method(:prepend)'
#<Method: #<Class:String>(Module)#prepend(*modules) <internal:core> core/module.rb:92>

$ ruby -e 'p String.method(:prepend).unbind'
#<UnboundMethod: #<Class:String>(Module)#prepend(*modules) <internal:core> core/module.rb:92>
```

You'll notice it's quite close to MRI's output for an UnboundMethod:
```
# #<Class:Object> instead of #<Class:String> is the bug, the rest is fine
$ ruby-master -e 'p String.method(:prepend).unbind'
#<UnboundMethod: #<Class:Object>(Module)#prepend(*)>

$ ruby-2.7.2 -e 'p String.method(:prepend).unbind'
#<UnboundMethod: #<Class:String>#prepend(*)>
```

The actual logic for Method#inspect and and UnboundMethod#inspect should be similar, and I think the logic in TruffleRuby is correct and makes the most sense:
https://github.com/oracle/truffleruby/blob/0aeb60024d025e3d3d836bd08ff89479d4292bb7/src/main/ruby/truffleruby/core/truffle/method_operations.rb#L37-L42

The crucial part for this case: only use the `.` notation if the owner of the method (Method#owner) is a singleton class, *not* if the receiver has a singleton class.
```
# All Rubies agree on:
$ ruby -e 'p String.method(:prepend).owner.singleton_class?'
false
```


----------------------------------------
Bug #17428: Method#inspect bad output for class methods
https://bugs.ruby-lang.org/issues/17428#change-89438

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: jeremyevans0 (Jeremy Evans)
* ruby -v: ruby 3.0.0dev (2020-12-22T17:37:59Z master 487355873a) [x86_64-darwin18]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
```
$ $ ruby -e 'p String.method(:prepend)'
# 2.7.0:
#<Method: String.prepend(*)>
# 3.0.0:
#<Method: #<Class:Object>(Module)#prepend(*)>
```

@jeremyevans found it shows the method as pertaining to one level too high (which is good for objects as we don't want to show the singleton class there, but not for classes).

Probably due to #15608



-- 
https://bugs.ruby-lang.org/

[ruby-core:101650] [Ruby master Bug#17428] Method#inspect bad output for class methods

[marcandre-ruby-core]

Issue #17428 has been updated by marcandre (Marc-Andre Lafortune).


Eregon (Benoit Daloze) wrote in #note-3:
> ```
> #<Method: String(Module).prepend(*)>
> ```
> doesn't seem correct, prepend is not a class method of `Module`, it's an instance method.

When `(X)` appears, it means `actually an instance method of X`, even in the case where `.` appears later. So not incorrect, but I agree it might be confusing. How about this other possibility?

```
#<Method: String.(Module#)prepend(*)>
```


----------------------------------------
Bug #17428: Method#inspect bad output for class methods
https://bugs.ruby-lang.org/issues/17428#change-89440

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: jeremyevans0 (Jeremy Evans)
* ruby -v: ruby 3.0.0dev (2020-12-22T17:37:59Z master 487355873a) [x86_64-darwin18]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
```
$ $ ruby -e 'p String.method(:prepend)'
# 2.7.0:
#<Method: String.prepend(*)>
# 3.0.0:
#<Method: #<Class:Object>(Module)#prepend(*)>
```

@jeremyevans found it shows the method as pertaining to one level too high (which is good for objects as we don't want to show the singleton class there, but not for classes).

Probably due to #15608



-- 
https://bugs.ruby-lang.org/

[ruby-core:101652] [Ruby master Bug#17428] Method#inspect bad output for class methods

[eregontp]

Issue #17428 has been updated by Eregon (Benoit Daloze).


For me, `Foo.bar` immediately means "singleton method of Foo", which is not the case here.
So I think we should only use the `.` form when Method#owner is a singleton class.

I recommend simplicity and consistency (between object singleton classes and metaclasses).
The output of Method#inspect has been confusing in the past and sometimes wrong, the way to solve it IMHO is a simple rule and a consistent output.
So I'm against making it more complicated / more edge cases.

----------------------------------------
Bug #17428: Method#inspect bad output for class methods
https://bugs.ruby-lang.org/issues/17428#change-89441

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: jeremyevans0 (Jeremy Evans)
* ruby -v: ruby 3.0.0dev (2020-12-22T17:37:59Z master 487355873a) [x86_64-darwin18]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
```
$ $ ruby -e 'p String.method(:prepend)'
# 2.7.0:
#<Method: String.prepend(*)>
# 3.0.0:
#<Method: #<Class:Object>(Module)#prepend(*)>
```

@jeremyevans found it shows the method as pertaining to one level too high (which is good for objects as we don't want to show the singleton class there, but not for classes).

Probably due to #15608



-- 
https://bugs.ruby-lang.org/

[ruby-core:101656] [Ruby master Bug#17428] Method#inspect bad output for class methods

[merch-redmine]

Issue #17428 has been updated by jeremyevans0 (Jeremy Evans).


Using `#<Class:String>(Module)#` instead of `String(Module).` is a trivial change to make, and I have no preference between the two. `String.(Module#)` is a significantly more invasive change and I don't think we should go that route. It would be great to get feedback from other committers regarding this today if we want this fix to make 3.0.

----------------------------------------
Bug #17428: Method#inspect bad output for class methods
https://bugs.ruby-lang.org/issues/17428#change-89445

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: jeremyevans0 (Jeremy Evans)
* ruby -v: ruby 3.0.0dev (2020-12-22T17:37:59Z master 487355873a) [x86_64-darwin18]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
```
$ $ ruby -e 'p String.method(:prepend)'
# 2.7.0:
#<Method: String.prepend(*)>
# 3.0.0:
#<Method: #<Class:Object>(Module)#prepend(*)>
```

@jeremyevans found it shows the method as pertaining to one level too high (which is good for objects as we don't want to show the singleton class there, but not for classes).

Probably due to #15608



-- 
https://bugs.ruby-lang.org/

[ruby-core:101659] [Ruby master Bug#17428] Method#inspect bad output for class methods

[marcandre-ruby-core]

Issue #17428 has been updated by marcandre (Marc-Andre Lafortune).

Assignee changed from jeremyevans0 (Jeremy Evans) to matz (Yukihiro Matsumoto)

jeremyevans0 (Jeremy Evans) wrote in #note-7:
> `String.(Module#)` is a significantly more invasive change

I prefer to decide "what do we want" than "what is easier". Change was actually a few lines changes for that version: https://github.com/ruby/ruby/pull/3985

Matz, which do you prefer:

```
$ ruby -e 'p String.method(:prepend)'
#<Method: String(Module).prepend(*modules)...>
#<Method: String.(Module#)prepend(*modules)...>
#<Method: #<Class:String>(Module)#prepend(*modules)...>
```



----------------------------------------
Bug #17428: Method#inspect bad output for class methods
https://bugs.ruby-lang.org/issues/17428#change-89448

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* ruby -v: ruby 3.0.0dev (2020-12-22T17:37:59Z master 487355873a) [x86_64-darwin18]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
```
$ $ ruby -e 'p String.method(:prepend)'
# 2.7.0:
#<Method: String.prepend(*)>
# 3.0.0:
#<Method: #<Class:Object>(Module)#prepend(*)>
```

@jeremyevans found it shows the method as pertaining to one level too high (which is good for objects as we don't want to show the singleton class there, but not for classes).

Probably due to #15608



-- 
https://bugs.ruby-lang.org/

[ruby-core:101686] [Ruby master Bug#17428] Method#inspect bad output for class methods

[eregontp]

Issue #17428 has been updated by Eregon (Benoit Daloze).


marcandre (Marc-Andre Lafortune) wrote in #note-8:
> I prefer to decide "what do we want" than "what is easier".

I'm not matz, but what I want for Method#inspect is consistency and a clear rule for formatting.
(https://github.com/oracle/truffleruby/blob/0aeb60024d025e3d3d836bd08ff89479d4292bb7/src/main/ruby/truffleruby/core/truffle/method_operations.rb#L37-L42 seems a good one)
IMHO more special cases just lead to bugs like this, confusion as before #15608, or harder-to-understand output.

Method#inspect doesn't need to be "as nice as possible", I think it needs to be "easy to understand and consistent to help comparison and avoid ambiguity".

I think this is a good example why the behavior of current master with Jeremy's fix is better:
```
$ ruby -e 'p Module.method(:prepend)'

2.7.2, confusing, prepend is not a singleton method
#<Method: Module.prepend(*)>

CRuby master with Jeremy's fix (same on TruffleRuby): clear, simple, consistent
#<Method: #<Class:Module>(Module)#prepend(*)>

the proposals above, IMHO both are unclear and confusing:
#<Method: Module(Module).prepend(*)>
#<Method: Module.(Module#)prepend(*)>
```

----------------------------------------
Bug #17428: Method#inspect bad output for class methods
https://bugs.ruby-lang.org/issues/17428#change-89510

* Author: marcandre (Marc-Andre Lafortune)
* Status: Closed
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* ruby -v: ruby 3.0.0dev (2020-12-22T17:37:59Z master 487355873a) [x86_64-darwin18]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
```
$ $ ruby -e 'p String.method(:prepend)'
# 2.7.0:
#<Method: String.prepend(*)>
# 3.0.0:
#<Method: #<Class:Object>(Module)#prepend(*)>
```

@jeremyevans found it shows the method as pertaining to one level too high (which is good for objects as we don't want to show the singleton class there, but not for classes).

Probably due to #15608



-- 
https://bugs.ruby-lang.org/

[ruby-core:101690] [Ruby master Bug#17428] Method#inspect bad output for class methods

[merch-redmine]

Issue #17428 has been updated by jeremyevans0 (Jeremy Evans).


After thinking about this more, I would prefer :

```ruby
Module.method(:prepend)
# => #<Method: Module.prepend(Module#prepend)(*)>
```

Basically, show the method based on the receiver, then, if it is different, the method based on the owner.  I think this is conceptually simpler.  The only issue it will result in the method name showing up twice even if the method has not been aliased.

Additional examples:

```ruby
class A
  def self.b; end
end
class C < A;
end
C.method(:b)
# => #<Method: C.b(A.b)(*)>

module M
  def b; end
end
C.extend M
C.method(:b)
# => #<Method: C.b(M#b)(*)>

String.new.method(:to_s)
# => #<Method: String#to_s(*)>

class S < String
end
S.new.method(:to_s)
# => #<Method: S#to_s(String#to_s)(*)>
```

----------------------------------------
Bug #17428: Method#inspect bad output for class methods
https://bugs.ruby-lang.org/issues/17428#change-89514

* Author: marcandre (Marc-Andre Lafortune)
* Status: Closed
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* ruby -v: ruby 3.0.0dev (2020-12-22T17:37:59Z master 487355873a) [x86_64-darwin18]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
```
$ $ ruby -e 'p String.method(:prepend)'
# 2.7.0:
#<Method: String.prepend(*)>
# 3.0.0:
#<Method: #<Class:Object>(Module)#prepend(*)>
```

@jeremyevans found it shows the method as pertaining to one level too high (which is good for objects as we don't want to show the singleton class there, but not for classes).

Probably due to #15608



-- 
https://bugs.ruby-lang.org/

[ruby-core:101692] [Ruby master Bug#17428] Method#inspect bad output for class methods

[eregontp]

Issue #17428 has been updated by Eregon (Benoit Daloze).

Backport changed from 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN to 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: REQUIRED

Agreed that looks nice, but what should be shown from the example in #15608?
```
p obj.method(:foo)
#<Method: C#foo>
vs
#<Method: #<C:0x000055668ebef268>.foo(C#foo)>
```
based on whether the instance has a singleton class doesn't seem nice.

I wonder if always using the `#` notation wouldn't be simpler and more consistent (we'd always see the module in which the method is defined).
If it's a singleton class method, we might as well show the singleton class.

I think a new feature to discuss it would be best, the bug that @marcandre found has been fixed now.

----------------------------------------
Bug #17428: Method#inspect bad output for class methods
https://bugs.ruby-lang.org/issues/17428#change-89517

* Author: marcandre (Marc-Andre Lafortune)
* Status: Closed
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* ruby -v: ruby 3.0.0dev (2020-12-22T17:37:59Z master 487355873a) [x86_64-darwin18]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: REQUIRED
----------------------------------------
```
$ $ ruby -e 'p String.method(:prepend)'
# 2.7.0:
#<Method: String.prepend(*)>
# 3.0.0:
#<Method: #<Class:Object>(Module)#prepend(*)>
```

@jeremyevans found it shows the method as pertaining to one level too high (which is good for objects as we don't want to show the singleton class there, but not for classes).

Probably due to #15608



-- 
https://bugs.ruby-lang.org/

[ruby-core:102952] [Ruby master Bug#17428] Method#inspect bad output for class methods

[nagachika00]

Issue #17428 has been updated by nagachika (Tomoyuki Chikanaga).


MEMO: Though there should be some preceding changesets, I gave up to find it for the time being.

----------------------------------------
Bug #17428: Method#inspect bad output for class methods
https://bugs.ruby-lang.org/issues/17428#change-91017

* Author: marcandre (Marc-Andre Lafortune)
* Status: Closed
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* ruby -v: ruby 3.0.0dev (2020-12-22T17:37:59Z master 487355873a) [x86_64-darwin18]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: REQUIRED
----------------------------------------
```
$ $ ruby -e 'p String.method(:prepend)'
# 2.7.0:
#<Method: String.prepend(*)>
# 3.0.0:
#<Method: #<Class:Object>(Module)#prepend(*)>
```

@jeremyevans found it shows the method as pertaining to one level too high (which is good for objects as we don't want to show the singleton class there, but not for classes).

Probably due to #15608



-- 
https://bugs.ruby-lang.org/