Add MethodError hints for functions in other modules #58715
Conversation
When a MethodError occurs, check if functions with the same name exist in other modules (particularly those of the argument types). This helps users discover that they may need to import a function or ensure multiple functions are the same generic function. - For Base functions: suggests importing (e.g., "You may have intended to import Base.length") - For other modules: suggests they may be intended as the same generic function - Shows all matches from relevant modules in sorted order - Uses modulesof! to properly handle all type structures including unions Fixes #58682
|
(Written by claude code per #58666 - we should update AGENTS.md to tell the AI to do that). |
| print(io, "\nYou may have intended to import ") | ||
| show_unquoted(io, Expr(:., :Base, QuoteNode(name))) | ||
| # Check for functions with the same name in other modules | ||
| if f_is_function && isdefined(ft.name, :singletonname) |
There was a problem hiding this comment.
| if f_is_function && isdefined(ft.name, :singletonname) | |
| if f_is_function |
There was a problem hiding this comment.
Yes, yes, I'm on it - give a minute ;)
| for mod in sorted_modules | ||
| if isdefined(mod, name) | ||
| candidate = getfield(mod, name) | ||
| if candidate !== f && (isa(candidate, Function) || isa(candidate, Type)) && hasmethod(candidate, arg_types) |
There was a problem hiding this comment.
| if candidate !== f && (isa(candidate, Function) || isa(candidate, Type)) && hasmethod(candidate, arg_types) | |
| if candidate !== f && ex.world != typemax(UInt) && hasmethod(candidate, arg_types; world=ex.world) |
There was a problem hiding this comment.
Changes current behavior, but I agree that it should have checked in ex.world
| show_unquoted(io, Expr(:., nameof(f_module), QuoteNode(name))) | ||
| print(io, " and ") | ||
| show_unquoted(io, Expr(:., nameof(mod), QuoteNode(name))) |
There was a problem hiding this comment.
These look like they would print submodules very weirdly. I think you want
| show_unquoted(io, Expr(:., nameof(f_module), QuoteNode(name))) | |
| print(io, " and ") | |
| show_unquoted(io, Expr(:., nameof(mod), QuoteNode(name))) | |
| show_unquoted(io, Expr(:., f_module, QuoteNode(name))) | |
| print(io, " and ") | |
| show_unquoted(io, Expr(:., mod, QuoteNode(name))) |
There was a problem hiding this comment.
I guess we can print the full path. I figured you'd already know the full module path since it'd be in one the types that printed in the error.
There was a problem hiding this comment.
I can just imagine some weird cases though, like "You may have intended Artifacts and Artifacts to be the same", neglecting to mention that one of those is Pkg.Artifacts 😂
There was a problem hiding this comment.
Yep, that sounds convincing to me. I'll switch it to the full path.
| print(io, "\nYou may have intended to import ") | ||
| show_unquoted(io, Expr(:., :Base, QuoteNode(name))) | ||
| else | ||
| print(io, "\nYou may have intended for ") |
There was a problem hiding this comment.
I know the use of the presumptive author "you" is pre-existing, but I don't think it is right to assert that the "you" here who might want this to have worked and the package author who intended for it to work are the same person. Should we word these with the package name as the subject instead?
| print(io, "\nYou may have intended for ") | |
| print(io, "\n$mod may have intended for ") |
Or rather "The intent in $mod may have been to import $f_module: $name"
There was a problem hiding this comment.
"The intent in $mod may have been to import $f_module: $name"
I think we've generally been try to warn people away from import. Maybe
"The definition in $mod by have intended to extend $f_module.$name"?
There was a problem hiding this comment.
@vtjnash Thoughts on that wording?
There was a problem hiding this comment.
Seems fine to me. I tend to think import is fine and would be clear here, but not enough to try to convince other people that they need to use it
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
When a MethodError occurs, check if functions with the same name exist
in other modules (particularly those of the argument types). This helps
users discover that they may need to import a function or ensure multiple
functions are the same generic function.
Fixes #58682