Roger-luo / Configurations.jl

Options & Configurations made easy.
https://configurations.rogerluo.dev/stable
MIT License
80 stars 12 forks source link

better error description for positional argument #73

Open johnnychen94 opened 2 years ago

johnnychen94 commented 2 years ago
@option struct MyOption
    x::Int
    y::Int = 1
end

MyOption(2) throws some encrypted error messages besides the core error MethodError: no method matching MyOption(::Int64)

ERROR: MethodError: no method matching MyOption(::Int64)┌ Error: Error showing method candidates, aborted
│   exception =
│    could not determine location of method definition
│    Stacktrace:
│      [1] error(s::String)
│        @ Base ./error.jl:33
│      [2] functionloc
│        @ ./methodshow.jl:164 [inlined]
│      [3] show_method_candidates(io::IOContext{Base.TTY}, ex::MethodError, kwargs::Any)
│        @ Base ./errorshow.jl:499
│      [4] showerror(io::IOContext{Base.TTY}, ex::MethodError)
│        @ Base ./errorshow.jl:318
│      [5] showerror(io::IOContext{Base.TTY}, ex::MethodError, bt::Vector{Base.StackTraces.StackFrame}; backtrace::Bool)
│        @ Base ./errorshow.jl:88
│      [6] show_exception_stack(io::IOContext{Base.TTY}, stack::Vector{Any})
│        @ Base ./errorshow.jl:866
│      [7] display_error(io::IOContext{Base.TTY}, stack::Base.ExceptionStack)
│        @ Base ./client.jl:104
│      [8] #invokelatest#2
│        @ ./essentials.jl:716 [inlined]
│      [9] invokelatest
│        @ ./essentials.jl:714 [inlined]
│     [10] print_response(errio::IO, response::Any, show_value::Bool, have_color::Bool, specialdisplay::Union{Nothing, AbstractDisplay})
│        @ REPL /Applications/Julia-1.7.app/Contents/Resources/julia/share/julia/stdlib/v1.7/REPL/src/REPL.jl:288
│     [11] (::REPL.var"#45#46"{REPL.LineEditREPL, Pair{Any, Bool}, Bool, Bool})(io::Any)
│        @ REPL /Applications/Julia-1.7.app/Contents/Resources/julia/share/julia/stdlib/v1.7/REPL/src/REPL.jl:277
│     [12] with_repl_linfo(f::Any, repl::REPL.LineEditREPL)
│        @ REPL /Applications/Julia-1.7.app/Contents/Resources/julia/share/julia/stdlib/v1.7/REPL/src/REPL.jl:510
│     [13] print_response(repl::REPL.AbstractREPL, response::Any, show_value::Bool, have_color::Bool)
│        @ REPL /Applications/Julia-1.7.app/Contents/Resources/julia/share/julia/stdlib/v1.7/REPL/src/REPL.jl:275
│     [14] (::REPL.var"#do_respond#66"{Bool, Bool, REPL.var"#77#87"{REPL.LineEditREPL, REPL.REPLHistoryProvider}, REPL.LineEditREPL, REPL.LineEdit.Prompt})(s::REPL.LineEdit.MIState, buf::Any, ok::Bool)
│        @ REPL /Applications/Julia-1.7.app/Contents/Resources/julia/share/julia/stdlib/v1.7/REPL/src/REPL.jl:846
│     [15] #invokelatest#2
│        @ ./essentials.jl:716 [inlined]
│     [16] invokelatest
│        @ ./essentials.jl:714 [inlined]
│     [17] run_interface(terminal::REPL.Terminals.TextTerminal, m::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
│        @ REPL.LineEdit /Applications/Julia-1.7.app/Contents/Resources/julia/share/julia/stdlib/v1.7/REPL/src/LineEdit.jl:2493
│     [18] run_frontend(repl::REPL.LineEditREPL, backend::REPL.REPLBackendRef)
│        @ REPL /Applications/Julia-1.7.app/Contents/Resources/julia/share/julia/stdlib/v1.7/REPL/src/REPL.jl:1232
│     [19] (::REPL.var"#49#54"{REPL.LineEditREPL, REPL.REPLBackendRef})()
│        @ REPL ./task.jl:423
└ @ Base errorshow.jl:320

Stacktrace:
 [1] top-level scope

Maybe it's a good idea to eagerly generate dummy and descriptive messages here, e.g., defining

MyOption(arg0, args...) = error("Positional constructor for `@option` structs is not supported, use keyword version `MyOption(; kwargs...)` instead.")

Is it https://github.com/Roger-luo/Configurations.jl/blob/6aad922a0074d1bc9a6cb2235b2c5f76437cd1fb/src/codegen.jl?_pjax=%23js-repo-pjax-container%2C%20div%5Bitemtype%3D%22http%3A%2F%2Fschema.org%2FSoftwareSourceCode%22%5D%20main%2C%20%5Bdata-pjax-container%5D#L351-L353 that handles this codegen?

Roger-luo commented 2 years ago

Positional constructor is supported but it should only be your default constructor. You should write MyOption(1, 2) not just MyOption(1)

I can see why you think there is a constructor with vararg. But I don't think having that is very useful in more complicated case given there's already a keyword constructor and this is the same behavior of Base.@kedef

Roger-luo commented 2 years ago

For a normal struct type without such constructor I think you get a similar error isn't it?

johnnychen94 commented 2 years ago

My apologies for not describing it very clearly. I was hoping the error stack become cleaner/shorter, e.g.,:

julia> Base.@kwdef struct MyOption
           x::Int
           y::Int = 1
       end
MyOption

julia> MyOption(1)
ERROR: MethodError: no method matching MyOption(::Int64)
Closest candidates are:
  MyOption(::Int64, ::Int64) at REPL[1]:2
  MyOption(::Any, ::Any) at REPL[1]:2
  MyOption(; x, y) at /Applications/Julia-1.7.app/Contents/Resources/julia/share/julia/base/util.jl:478
Stacktrace:
 [1] top-level scope
   @ REPL[2]:1
johnnychen94 commented 2 years ago

could not determine location of method definition

Oh I guess this is a julia issue with not so good support for codegen methods?

Roger-luo commented 2 years ago

just need to insert the line number correctly in this package: https://github.com/Roger-luo/Expronicon.jl