Skip to content
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

Weird method location shouldn't match unknown location #2244

Merged
merged 1 commit into from
Apr 9, 2022

Conversation

jcoleman
Copy link
Contributor

@jcoleman jcoleman commented Apr 7, 2022

Methods that don't have a source location (e.g., C methods, or methods
created via metaprogramming or even alias_method) are not reasonable
possible matching methods for a "weird method" we need to locate.

In this case renamed_method_source_location can also return nil if
the actual code in question is a bare script (i.e., no methods). If that script
is loaded via eval then we'll end up in the weird method path in the first
place, but no method matching can be found, and if a no-source-location
method exists, we'll return that.

Down the line that's particularly painful because the source loading thinks
it's a C method, but it can actually be from metaprogramming (and alias_method!),
and then Pry::Method#pry_doc_info raises an error, and wherami breaks even
though we already have valid __FILE__ and __LINE__ values.

@andrehjr
Copy link
Member

andrehjr commented Apr 7, 2022

Hi @jcoleman thanks for the PR!

Can you add a spec for this scenario?

@jcoleman
Copy link
Contributor Author

jcoleman commented Apr 7, 2022

I’m certainly happy to try to add a test; I red-greened this in a live system hence why I don’t have one yet.

@jcoleman jcoleman force-pushed the patch-1 branch 2 times, most recently from dcc1f7f to 45ae63e Compare April 8, 2022 14:05
@jcoleman
Copy link
Contributor Author

jcoleman commented Apr 8, 2022

@andrehjr I've added a test (for interest's sake: the naming in the test was mostly to match the source of finding this error, which was in a Capistrano 2 task).

spec/method_spec.rb Outdated Show resolved Hide resolved
Methods that don't have a source location (e.g., C methods, or methods
created via metaprogramming or even `alias_method` to C methods) are not
reasonable possible matching methods for a "weird method" we need to
locate.

In this case `renamed_method_source_location` can also return `nil` if
the actual code in question is a bare script (i.e., no methods). If that
script is loaded via `eval` then we'll end up in the weird method path
in the first place, but no method matching can be found, and if a
no-source-location method exists, we'll return that.

Down the line that's particularly painful because the source loading
thinks it's a C method, but it can actually be from metaprogramming (and
`alias_method`!), and then `Pry::Method#pry_doc_info` raises an error,
and `wherami` breaks even though we already have valid `__FILE__` and
`__LINE__` values.
@jcoleman
Copy link
Contributor Author

@andrehjr Thanks for merging this! Anyway you'd be able to release a 0.14.2 soon so that I can easily install the fix on our systems?

@jcoleman
Copy link
Contributor Author

@andrehjr Bump on my question about whether you'd be able to release an updated version.

@jcoleman
Copy link
Contributor Author

@andrehjr Any chance you have a timeline for release a new minor revision?

@andrehjr
Copy link
Member

Soon, hopefully. I don't currently have a timeline(which depends on other maintainers), there's another important fix I want to work on first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants