Skip to content

Fix show_source command when obj.method is overrided #1111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 23, 2025

Conversation

tompng
Copy link
Member

@tompng tompng commented Jul 22, 2025

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)
                                              ^^^^^^

@@ -114,7 +114,7 @@ def method_target(owner_receiver, super_level, method, type)
when "owner"
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
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
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
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants