Skip to content

Fix show_source command when obj.method is overrided#1111

Merged
tompng merged 1 commit into
ruby:masterfrom
tompng:show_source_method
Jul 23, 2025
Merged

Fix show_source command when obj.method is overrided#1111
tompng merged 1 commit into
ruby:masterfrom
tompng:show_source_method

Conversation

@tompng

@tompng tompng commented Jul 22, 2025

Copy link
Copy Markdown
Member

method method is sometimes overridden.

req = Net::HTTP::Post.new('/path')
req.method #=> "POST"
req.path #=> "/path"

Fixes this error

irb(main):001> require 'net/http'
=> true
irb(main):002> req = Net::HTTP::Post.new('/path')
=> #<Net::HTTP::Post POST>
irb(main):003> $ req.path
lib/irb/source_finder.rb:117:in 'IRB::SourceFinder#method_target': wrong number of arguments (given 1, expected 0) (ArgumentError)

        target_method = owner_receiver.method(method)
                                              ^^^^^^
Comment thread lib/irb/source_finder.rb Outdated
target_method = owner_receiver.instance_method(method)
when "receiver"
target_method = owner_receiver.method(method)
target_method = Object.instance_method(:method).bind_call(owner_receiver, method)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we already store the method method in KERNEL_METHOD under Context. Perhaps we can create a module under IRB, like IRB::ORIGINAL_METHODS::KERNEL_METHOD, to host these method objects and reuse them in different places?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be good to introduce something like IRB::ORIGINAL_METHODS 👍

In context.rb and in completion.rb, method is called in each keystroke. But in SourceFinder, it is only called once per show_source. Needs for storing is not that high here, so I want to pend doing it for now in this pull request.

Anyway, thanks for finding KERNEL_METHOD. I updated Object.instance_method(:method) to Kernel.instance_method(:method) since def method; end in top level overrides Object's method.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair. Feel free to ship it then!

@st0012 st0012 added the bug Something isn't working label Jul 22, 2025
`method` method is sometimes overridden. (example: Net::HTTPRequest)
To handle this case, use `Object.instance_method(:method).bind_call` instead of `obj.method(name)`
@tompng tompng force-pushed the show_source_method branch from d6b51ef to 2785cbe Compare July 22, 2025 19:36
@tompng tompng merged commit 6e88aef into ruby:master Jul 23, 2025
31 of 33 checks passed
@tompng tompng deleted the show_source_method branch July 23, 2025 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

2 participants