From 0771bd4b346eb5ecdc7ce6dd1f40c4325e8aac2e Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Thu, 12 Mar 2020 14:20:21 -0500 Subject: [PATCH 1/8] Allow packages to provide custom hints for MethodErrors Package authors may be able to predict likely user errors, and it can be nice to create an informative hint. For MethodErrors, we've done this repeatedly for Base; this PR makes it possible for packages to get in on the fun. The procedure is described in the docstring for `Base.methoderror_hints`. --- base/errorshow.jl | 25 ++++++++++++++++++++++++- test/errorshow.jl | 22 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/base/errorshow.jl b/base/errorshow.jl index 2327a3732c906..75478a7875595 100644 --- a/base/errorshow.jl +++ b/base/errorshow.jl @@ -208,6 +208,24 @@ function show_convert_error(io::IO, ex::MethodError, @nospecialize(arg_types_par end end +""" + methoderror_hints + +Packages can provide hints about appropriate corrective actions for +specific `MethodError`s with `push!(Base.methoderror_hints, hintmessage)`, where +`hintmessage` should be a function like + + function hintmessage(f, arg_types, kwargs) + # Test to see whether the call matches the specific pattern for this message + if f === myfunc && length(arg_types) == 1 && arg_types[1] <: SomeType + return "`myfunc(::SomeType)` is not defined, did you mean to call `otherfunc`?" + end + return nothing # use `nothing` to indicate that f, arg_types, kwargs didn't match + end +""" +const methoderror_hints = [] +# Note: Base should not use `methoderror_hints`, it should inline +# custom hints below to avoid the slight performance hit from `invokelatest`. function showerror(io::IO, ex::MethodError) # ex.args is a tuple type if it was thrown from `invoke` and is # a tuple of the arguments otherwise. @@ -311,6 +329,12 @@ function showerror(io::IO, ex::MethodError) "\nYou can convert to a column vector with the vec() function.") end end + for predicate in methoderror_hints + msg = Base.invokelatest(predicate, f, arg_types_param, kwargs) + if msg !== nothing + print(io, '\n', msg) + end + end try show_method_candidates(io, ex, kwargs) catch ex @@ -731,4 +755,3 @@ function show(io::IO, ip::InterpreterIP) print(io, " in $(ip.code) at statement $(Int(ip.stmt))") end end - diff --git a/test/errorshow.jl b/test/errorshow.jl index ad47dad903f81..208244c2689fa 100644 --- a/test/errorshow.jl +++ b/test/errorshow.jl @@ -586,6 +586,28 @@ end end end +# Custom hints +struct HasNoOne end +function recommend_oneunit(f, arg_types, kwargs) + if f === Base.one && length(arg_types) == 1 && arg_types[1] === HasNoOne + if isempty(kwargs) + return "HasNoOne does not support `one`; did you mean `oneunit`?" + else + return "`one` doesn't take keyword arguments, that would be silly" + end + end +end +push!(Base.methoderror_hints, recommend_oneunit) +let err_str + err_str = @except_str one(HasNoOne()) MethodError + @test occursin(r"MethodError: no method matching one\(::.*HasNoOne\)", err_str) + @test occursin("HasNoOne does not support `one`; did you mean `oneunit`?", err_str) + err_str = @except_str one(HasNoOne(); value=2) MethodError + @test occursin(r"MethodError: no method matching one\(::.*HasNoOne; value=2\)", err_str) + @test occursin("`one` doesn't take keyword arguments, that would be silly", err_str) +end +pop!(Base.methoderror_hints) + # issue #28442 @testset "Long stacktrace printing" begin f28442(c) = g28442(c + 1) From 1cb46c96a8163ce33e8219b059b5fe0c714ad299 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Fri, 13 Mar 2020 03:58:38 -0500 Subject: [PATCH 2/8] Add NEWS and compat info --- NEWS.md | 5 +++++ base/errorshow.jl | 3 +++ 2 files changed, 8 insertions(+) diff --git a/NEWS.md b/NEWS.md index 8cddb17a82558..2be1317144455 100644 --- a/NEWS.md +++ b/NEWS.md @@ -11,6 +11,11 @@ New language features macros and matrix constructors, which are whitespace sensitive, because expressions like `[a ±b]` now get parsed as `[a ±(b)]` instead of `[±(a, b)]`. ([#34200]) +* Packages can now provide custom hints for specific `MethodError`s to + help guide users. `push!` a pattern-matching function `hint(f, + argtypes, kwargs)`, returning a message string in case of a match + and `nothing` otherwise, to `Base.methoderror_hints`. ([#35094]) + Language changes ---------------- diff --git a/base/errorshow.jl b/base/errorshow.jl index 75478a7875595..e695af3f5003e 100644 --- a/base/errorshow.jl +++ b/base/errorshow.jl @@ -222,6 +222,9 @@ specific `MethodError`s with `push!(Base.methoderror_hints, hintmessage)`, where end return nothing # use `nothing` to indicate that f, arg_types, kwargs didn't match end + +!!! compat "Julia 1.5" + Custom MethodError hints are available as of Julia 1.5. """ const methoderror_hints = [] # Note: Base should not use `methoderror_hints`, it should inline From d269fd379074817ef7d0de27ffb383ea23a83070 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Fri, 13 Mar 2020 16:05:28 -0500 Subject: [PATCH 3/8] Mention push from __init__ --- NEWS.md | 3 ++- base/errorshow.jl | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 2be1317144455..c6a022ff06764 100644 --- a/NEWS.md +++ b/NEWS.md @@ -14,7 +14,8 @@ New language features * Packages can now provide custom hints for specific `MethodError`s to help guide users. `push!` a pattern-matching function `hint(f, argtypes, kwargs)`, returning a message string in case of a match - and `nothing` otherwise, to `Base.methoderror_hints`. ([#35094]) + and `nothing` otherwise, to `Base.methoderror_hints`. + See `?Base.methoderror_hints` for more information. ([#35094]) Language changes ---------------- diff --git a/base/errorshow.jl b/base/errorshow.jl index e695af3f5003e..41c7f1949d2da 100644 --- a/base/errorshow.jl +++ b/base/errorshow.jl @@ -223,6 +223,9 @@ specific `MethodError`s with `push!(Base.methoderror_hints, hintmessage)`, where return nothing # use `nothing` to indicate that f, arg_types, kwargs didn't match end +Packages should perform the `push!` onto `Base.methoderror_hints` from +their `__init__` function. + !!! compat "Julia 1.5" Custom MethodError hints are available as of Julia 1.5. """ From e9a3c40f1b0e5502c666c71bebf65c45c2cc3558 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Sat, 14 Mar 2020 08:23:05 -0500 Subject: [PATCH 4/8] Quote demo with backticks rather than indentation --- base/errorshow.jl | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/base/errorshow.jl b/base/errorshow.jl index 41c7f1949d2da..8c2c5d611430f 100644 --- a/base/errorshow.jl +++ b/base/errorshow.jl @@ -215,13 +215,15 @@ Packages can provide hints about appropriate corrective actions for specific `MethodError`s with `push!(Base.methoderror_hints, hintmessage)`, where `hintmessage` should be a function like - function hintmessage(f, arg_types, kwargs) - # Test to see whether the call matches the specific pattern for this message - if f === myfunc && length(arg_types) == 1 && arg_types[1] <: SomeType - return "`myfunc(::SomeType)` is not defined, did you mean to call `otherfunc`?" - end - return nothing # use `nothing` to indicate that f, arg_types, kwargs didn't match +```julia +function hintmessage(f, arg_types, kwargs) + # Test to see whether the call matches the specific pattern for this message + if f === myfunc && length(arg_types) == 1 && arg_types[1] <: SomeType + return "`myfunc(::SomeType)` is not defined, did you mean to call `otherfunc`?" end + return nothing # use `nothing` to indicate that f, arg_types, kwargs didn't match +end +``` Packages should perform the `push!` onto `Base.methoderror_hints` from their `__init__` function. From 70d86797b4d6ba82d1a1b1290b4fa7b808ce62f9 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Sat, 14 Mar 2020 13:22:53 -0500 Subject: [PATCH 5/8] Generalize the hint mechanism --- NEWS.md | 8 ++-- base/errorshow.jl | 111 +++++++++++++++++++++++++++++++++------------- base/exports.jl | 2 + test/errorshow.jl | 12 ++--- 4 files changed, 90 insertions(+), 43 deletions(-) diff --git a/NEWS.md b/NEWS.md index c6a022ff06764..b38a8f24ba0c7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -11,11 +11,9 @@ New language features macros and matrix constructors, which are whitespace sensitive, because expressions like `[a ±b]` now get parsed as `[a ±(b)]` instead of `[±(a, b)]`. ([#34200]) -* Packages can now provide custom hints for specific `MethodError`s to - help guide users. `push!` a pattern-matching function `hint(f, - argtypes, kwargs)`, returning a message string in case of a match - and `nothing` otherwise, to `Base.methoderror_hints`. - See `?Base.methoderror_hints` for more information. ([#35094]) +* Packages can now provide custom hints to help users resolve errors by using the + `register_error_hint` function. Packages that define custom exception types + can support hints by calling `show_error_hints` from their `showerror` method. ([#35094]) Language changes ---------------- diff --git a/base/errorshow.jl b/base/errorshow.jl index 8c2c5d611430f..85b3a6995edd0 100644 --- a/base/errorshow.jl +++ b/base/errorshow.jl @@ -29,6 +29,80 @@ ERROR: MyException: test exception """ showerror(io::IO, ex) = show(io, ex) +""" + register_error_hint(handler, exceptiontype) + +Register a "hinting" function `handler(io, exception)` that can +suggest potential ways for users to circumvent errors. `handler` +should examine `exception` to see whether the conditions appropriate +for a hint are met, and then generate output to `io`. +Packages should call `register_error_hint` from within their +[`__init__`](@ref) function. + +For specific exception types, `handler` is required to accept additional arguments: + +- `MethodError`: provide `handler(io, exc::MethodError, argtypes, kwargs)`, + which splits the combined arguments into positional and keyword arguments. + +When issuing a hint, the output should typically start with `\n`. + +If you define custom exception types, your `showerror` method can +support hints by calling [`show_error_hints`](@ref). + +# Example + +```jldoctest +julia> module Hinter + + only_int(x::Int) = 1 + any_number(x::Number) = 2 + + function __init__() + register_error_hint(MethodError) do io, exc, argtypes, kwargs + if exc.f == only_int + if get(io, :color, false) + print(io, "\nDid you mean to call ") + printstyled(io, "`any_number`?", color=:light_magenta) + else + print(io, "\nDid you mean to call `any_number`?") + end + end + end + end + + end +Main.Hinter + +julia> Hinter.only_int(1.0) +ERROR: MethodError: no method matching only_int(::Float64) +Did you mean to call `any_number`? +Closest candidates are: +[...] +``` + +!!! compat "Julia 1.5" + Custom error hints are available as of Julia 1.5. +""" +function register_error_hint(handler, exct::Type) + list = get!(()->[], _hint_handlers, exct) + push!(list, handler) + return nothing +end + +const _hint_handlers = IdDict{Type,Vector{Any}}() + +function show_error_hints(io, ex, args...) + hinters = get!(()->[], _hint_handlers, typeof(ex)) + for handler in hinters + try + Base.invokelatest(handler, io, ex, args...) + catch err + tn = typeof(handler).name + @error "Hint-handler $handler for $(typeof(ex)) in $(tn.module) caused an error" + end + end +end + function showerror(io::IO, ex::BoundsError) print(io, "BoundsError") if isdefined(ex, :a) @@ -45,6 +119,7 @@ function showerror(io::IO, ex::BoundsError) print(io, ']') end end + show_error_hints(io, ex) end function showerror(io::IO, ex::TypeError) @@ -68,6 +143,7 @@ function showerror(io::IO, ex::TypeError) end print(io, ctx, ", expected ", ex.expected, ", got ", targs...) end + show_error_hints(io, ex) end function showerror(io::IO, ex, bt; backtrace=true) @@ -106,6 +182,7 @@ function showerror(io::IO, ex::DomainError) if isdefined(ex, :msg) print(io, ":\n", ex.msg) end + show_error_hints(io, ex) nothing end @@ -161,6 +238,7 @@ function showerror(io::IO, ex::InexactError) print(io, "InexactError: ", ex.func, '(') nameof(ex.T) === ex.func || print(io, ex.T, ", ") print(io, ex.val, ')') + show_error_hints(io, ex) end typesof(args...) = Tuple{Any[ Core.Typeof(a) for a in args ]...} @@ -208,32 +286,6 @@ function show_convert_error(io::IO, ex::MethodError, @nospecialize(arg_types_par end end -""" - methoderror_hints - -Packages can provide hints about appropriate corrective actions for -specific `MethodError`s with `push!(Base.methoderror_hints, hintmessage)`, where -`hintmessage` should be a function like - -```julia -function hintmessage(f, arg_types, kwargs) - # Test to see whether the call matches the specific pattern for this message - if f === myfunc && length(arg_types) == 1 && arg_types[1] <: SomeType - return "`myfunc(::SomeType)` is not defined, did you mean to call `otherfunc`?" - end - return nothing # use `nothing` to indicate that f, arg_types, kwargs didn't match -end -``` - -Packages should perform the `push!` onto `Base.methoderror_hints` from -their `__init__` function. - -!!! compat "Julia 1.5" - Custom MethodError hints are available as of Julia 1.5. -""" -const methoderror_hints = [] -# Note: Base should not use `methoderror_hints`, it should inline -# custom hints below to avoid the slight performance hit from `invokelatest`. function showerror(io::IO, ex::MethodError) # ex.args is a tuple type if it was thrown from `invoke` and is # a tuple of the arguments otherwise. @@ -337,12 +389,7 @@ function showerror(io::IO, ex::MethodError) "\nYou can convert to a column vector with the vec() function.") end end - for predicate in methoderror_hints - msg = Base.invokelatest(predicate, f, arg_types_param, kwargs) - if msg !== nothing - print(io, '\n', msg) - end - end + show_error_hints(io, ex, arg_types_param, kwargs) try show_method_candidates(io, ex, kwargs) catch ex diff --git a/base/exports.jl b/base/exports.jl index 6754fe08b3079..2e8f2ccc3d72c 100644 --- a/base/exports.jl +++ b/base/exports.jl @@ -695,8 +695,10 @@ export backtrace, catch_backtrace, error, + register_error_hint, rethrow, retry, + show_error_hints, systemerror, # stack traces diff --git a/test/errorshow.jl b/test/errorshow.jl index 208244c2689fa..12ed50849bd26 100644 --- a/test/errorshow.jl +++ b/test/errorshow.jl @@ -588,16 +588,16 @@ end # Custom hints struct HasNoOne end -function recommend_oneunit(f, arg_types, kwargs) - if f === Base.one && length(arg_types) == 1 && arg_types[1] === HasNoOne +function recommend_oneunit(io, ex, arg_types, kwargs) + if ex.f === Base.one && length(arg_types) == 1 && arg_types[1] === HasNoOne if isempty(kwargs) - return "HasNoOne does not support `one`; did you mean `oneunit`?" + print(io, "\nHasNoOne does not support `one`; did you mean `oneunit`?") else - return "`one` doesn't take keyword arguments, that would be silly" + print(io, "\n`one` doesn't take keyword arguments, that would be silly") end end end -push!(Base.methoderror_hints, recommend_oneunit) +@test register_error_hint(recommend_oneunit, MethodError) === nothing let err_str err_str = @except_str one(HasNoOne()) MethodError @test occursin(r"MethodError: no method matching one\(::.*HasNoOne\)", err_str) @@ -606,7 +606,7 @@ let err_str @test occursin(r"MethodError: no method matching one\(::.*HasNoOne; value=2\)", err_str) @test occursin("`one` doesn't take keyword arguments, that would be silly", err_str) end -pop!(Base.methoderror_hints) +pop!(Base._hint_handlers[MethodError]) # order is undefined, don't copy this # issue #28442 @testset "Long stacktrace printing" begin From 20e2afe2fd40b0b56b036d034a0cd50102742895 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Sat, 14 Mar 2020 14:09:30 -0500 Subject: [PATCH 6/8] Don't test for color in demo --- base/errorshow.jl | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/base/errorshow.jl b/base/errorshow.jl index 85b3a6995edd0..b1cfc7386292e 100644 --- a/base/errorshow.jl +++ b/base/errorshow.jl @@ -60,12 +60,8 @@ julia> module Hinter function __init__() register_error_hint(MethodError) do io, exc, argtypes, kwargs if exc.f == only_int - if get(io, :color, false) - print(io, "\nDid you mean to call ") - printstyled(io, "`any_number`?", color=:light_magenta) - else - print(io, "\nDid you mean to call `any_number`?") - end + print(io, "\nDid you mean to call ") + printstyled(io, "`any_number`?", color=:light_magenta) end end end From fa5e92445c7f4d6a264fa6f6b9f1dfc02d8b5168 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Sat, 14 Mar 2020 14:10:14 -0500 Subject: [PATCH 7/8] Add a test for busted error-hint handlers --- test/errorshow.jl | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/errorshow.jl b/test/errorshow.jl index 12ed50849bd26..86b02dda7f4d6 100644 --- a/test/errorshow.jl +++ b/test/errorshow.jl @@ -608,6 +608,19 @@ let err_str end pop!(Base._hint_handlers[MethodError]) # order is undefined, don't copy this +function busted_hint(io, exc, notarg) # wrong number of args + print(io, "\nI don't have a hint for you, sorry") +end +@test register_error_hint(busted_hint, DomainError) === nothing +try + sqrt(-2) +catch ex + io = IOBuffer() + @test_logs (:error, "Hint-handler busted_hint for DomainError in $(@__MODULE__) caused an error") showerror(io, ex) +end +pop!(Base._hint_handlers[DomainError]) # order is undefined, don't copy this + + # issue #28442 @testset "Long stacktrace printing" begin f28442(c) = g28442(c + 1) From b775889da861e7c9a0377eae935dd746f2edb206 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Sun, 15 Mar 2020 04:53:29 -0500 Subject: [PATCH 8/8] Eliminate the doctests and fix docs --- base/errorshow.jl | 27 +++++++++++++++++++-------- doc/src/base/base.md | 2 ++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/base/errorshow.jl b/base/errorshow.jl index b1cfc7386292e..c8f745fe90a57 100644 --- a/base/errorshow.jl +++ b/base/errorshow.jl @@ -35,23 +35,23 @@ showerror(io::IO, ex) = show(io, ex) Register a "hinting" function `handler(io, exception)` that can suggest potential ways for users to circumvent errors. `handler` should examine `exception` to see whether the conditions appropriate -for a hint are met, and then generate output to `io`. +for a hint are met, and if so generate output to `io`. Packages should call `register_error_hint` from within their -[`__init__`](@ref) function. +`__init__` function. For specific exception types, `handler` is required to accept additional arguments: - `MethodError`: provide `handler(io, exc::MethodError, argtypes, kwargs)`, which splits the combined arguments into positional and keyword arguments. -When issuing a hint, the output should typically start with `\n`. +When issuing a hint, the output should typically start with `\\n`. If you define custom exception types, your `showerror` method can support hints by calling [`show_error_hints`](@ref). # Example -```jldoctest +``` julia> module Hinter only_int(x::Int) = 1 @@ -60,20 +60,24 @@ julia> module Hinter function __init__() register_error_hint(MethodError) do io, exc, argtypes, kwargs if exc.f == only_int - print(io, "\nDid you mean to call ") - printstyled(io, "`any_number`?", color=:light_magenta) + # Color is not necessary, this is just to show it's possible. + print(io, "\\nDid you mean to call ") + printstyled(io, "`any_number`?", color=:cyan) end end end end -Main.Hinter +``` + +Then if you call `Hinter.only_int` on something that isn't an `Int` (thereby triggering a `MethodError`), it issues the hint: +``` julia> Hinter.only_int(1.0) ERROR: MethodError: no method matching only_int(::Float64) Did you mean to call `any_number`? Closest candidates are: -[...] + ... ``` !!! compat "Julia 1.5" @@ -87,6 +91,13 @@ end const _hint_handlers = IdDict{Type,Vector{Any}}() +""" + show_error_hints(io, ex, args...) + +Invoke all handlers from [`register_error_hint`](@ref) for the particular +exception type `typeof(ex)`. `args` must contain any other arguments expected by +the handler for that type. +""" function show_error_hints(io, ex, args...) hinters = get!(()->[], _hint_handlers, typeof(ex)) for handler in hinters diff --git a/doc/src/base/base.md b/doc/src/base/base.md index 1035ba519c511..015b3729c6a8d 100644 --- a/doc/src/base/base.md +++ b/doc/src/base/base.md @@ -334,6 +334,8 @@ Base.backtrace Base.catch_backtrace Base.catch_stack Base.@assert +Base.register_error_hint +Base.show_error_hints Base.ArgumentError Base.AssertionError Core.BoundsError