Skip to content

Commit 1431bec

Browse files
KristofferCKristofferC
authored
Avoid requiring the REPL to be loaded to show error hints for undefined variables (#57576)
For some reason, the nice error hints for undefined variables are put in the REPL module, requiring it to be loaded to get good error messages. There are many cases where code runs outside of the REPL but where good error messages are still useful (PkgEval, `Pkg.test`, Pluto, IJulia). As an example, this is what I got when running `Pkg.test` for a package with a regression: ``` Testing Running tests... ERROR: LoadError: UndefVarError: `decompress_symmetric` not defined in `Main` Stacktrace: [1] top-level scope @ ~/JuliaPkgs/NonconvexSemidefinite.jl/test/runtests.jl:6 [2] macro expansion ``` But if I paste the code into the REPL ``` julia> decompress_symmetric ERROR: UndefVarError: `decompress_symmetric` not defined in `Main` Hint: It looks like two or more modules export different bindings with this name, resulting in ambiguity. Try explicitly importing it from a particular module, or qualifying the name with the module it should come from. Hint: a global variable of this name also exists in NonconvexCore. Hint: a global variable of this name also exists in NonconvexSemidefinite. Hint: a global variable of this name also exists in NonconvexIpopt. ``` which makes me immediately understand what is going on. This just moves the code for this error hint to live beside all the other hints, I see no reason for keeping this special.
1 parent b9ac28a commit 1431bec

File tree

4 files changed

+153
-166
lines changed

4 files changed

+153
-166
lines changed

‎base/errorshow.jl‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,6 +1143,96 @@ end
11431143

11441144
Experimental.register_error_hint(fielderror_listfields_hint_handler, FieldError)
11451145

1146+
function UndefVarError_hint(io::IO, ex::UndefVarError)
1147+
var = ex.var
1148+
if isdefined(ex, :scope)
1149+
scope = ex.scope
1150+
if scope isa Module
1151+
bpart = Base.lookup_binding_partition(ex.world, GlobalRef(scope, var))
1152+
kind = Base.binding_kind(bpart)
1153+
if kind === Base.PARTITION_KIND_GLOBAL || kind === Base.PARTITION_KIND_UNDEF_CONST || kind == Base.PARTITION_KIND_DECLARED
1154+
print(io, "\nSuggestion: add an appropriate import or assignment. This global was declared but not assigned.")
1155+
elseif kind === Base.PARTITION_KIND_FAILED
1156+
print(io, "\nHint: It looks like two or more modules export different ",
1157+
"bindings with this name, resulting in ambiguity. Try explicitly ",
1158+
"importing it from a particular module, or qualifying the name ",
1159+
"with the module it should come from.")
1160+
elseif kind === Base.PARTITION_KIND_GUARD
1161+
print(io, "\nSuggestion: check for spelling errors or missing imports.")
1162+
elseif Base.is_some_imported(kind)
1163+
print(io, "\nSuggestion: this global was defined as `$(Base.partition_restriction(bpart).globalref)` but not assigned a value.")
1164+
end
1165+
elseif scope === :static_parameter
1166+
print(io, "\nSuggestion: run Test.detect_unbound_args to detect method arguments that do not fully constrain a type parameter.")
1167+
elseif scope === :local
1168+
print(io, "\nSuggestion: check for an assignment to a local variable that shadows a global of the same name.")
1169+
end
1170+
else
1171+
scope = undef
1172+
end
1173+
if scope !== Base
1174+
warned = _UndefVarError_warnfor(io, [Base], var)
1175+
1176+
if !warned
1177+
modules_to_check = (m for m in Base.loaded_modules_order
1178+
if m !== Core && m !== Base && m !== Main && m !== scope)
1179+
warned |= _UndefVarError_warnfor(io, modules_to_check, var)
1180+
end
1181+
1182+
warned || _UndefVarError_warnfor(io, [Core, Main], var)
1183+
end
1184+
return nothing
1185+
end
1186+
1187+
function _UndefVarError_warnfor(io::IO, modules, var::Symbol)
1188+
active_mod = Base.active_module()
1189+
1190+
warned = false
1191+
# collect modules which export or make public the variable by
1192+
# the module in which the variable is defined
1193+
to_warn_about = Dict{Module, Vector{Module}}()
1194+
for m in modules
1195+
# only include in info if binding has a value and is exported or public
1196+
if !Base.isdefined(m, var) || (!Base.isexported(m, var) && !Base.ispublic(m, var))
1197+
continue
1198+
end
1199+
warned = true
1200+
1201+
# handle case where the undefined variable is the name of a loaded module
1202+
if Symbol(m) == var && !isdefined(active_mod, var)
1203+
print(io, "\nHint: $m is loaded but not imported in the active module $active_mod.")
1204+
continue
1205+
end
1206+
1207+
binding_m = Base.binding_module(m, var)
1208+
if !haskey(to_warn_about, binding_m)
1209+
to_warn_about[binding_m] = [m]
1210+
else
1211+
push!(to_warn_about[binding_m], m)
1212+
end
1213+
end
1214+
1215+
for (binding_m, modules) in pairs(to_warn_about)
1216+
print(io, "\nHint: a global variable of this name also exists in ", binding_m, ".")
1217+
for m in modules
1218+
m == binding_m && continue
1219+
how_available = if Base.isexported(m, var)
1220+
"exported by"
1221+
elseif Base.ispublic(m, var)
1222+
"declared public in"
1223+
end
1224+
print(io, "\n - Also $how_available $m")
1225+
if !isdefined(active_mod, nameof(m)) || (getproperty(active_mod, nameof(m)) !== m)
1226+
print(io, " (loaded but not imported in $active_mod)")
1227+
end
1228+
print(io, ".")
1229+
end
1230+
end
1231+
return warned
1232+
end
1233+
1234+
Base.Experimental.register_error_hint(UndefVarError_hint, UndefVarError)
1235+
11461236
# ExceptionStack implementation
11471237
size(s::ExceptionStack) = size(s.stack)
11481238
getindex(s::ExceptionStack, i::Int) = s.stack[i]

‎stdlib/REPL/src/REPL.jl‎

Lines changed: 2 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ module REPL
1717
Base.Experimental.@optlevel 1
1818
Base.Experimental.@max_methods 1
1919

20-
function UndefVarError_hint(io::IO, ex::UndefVarError)
20+
function UndefVarError_REPL_hint(io::IO, ex::UndefVarError)
2121
var = ex.var
2222
if var === :or
2323
print(io, "\nSuggestion: Use `||` for short-circuiting boolean OR.")
@@ -30,95 +30,11 @@ function UndefVarError_hint(io::IO, ex::UndefVarError)
3030
elseif var === :quit
3131
print(io, "\nSuggestion: To exit Julia, use Ctrl-D, or type exit() and press enter.")
3232
end
33-
if isdefined(ex, :scope)
34-
scope = ex.scope
35-
if scope isa Module
36-
bpart = Base.lookup_binding_partition(ex.world, GlobalRef(scope, var))
37-
kind = Base.binding_kind(bpart)
38-
if kind === Base.PARTITION_KIND_GLOBAL || kind === Base.PARTITION_KIND_UNDEF_CONST || kind == Base.PARTITION_KIND_DECLARED
39-
print(io, "\nSuggestion: add an appropriate import or assignment. This global was declared but not assigned.")
40-
elseif kind === Base.PARTITION_KIND_FAILED
41-
print(io, "\nHint: It looks like two or more modules export different ",
42-
"bindings with this name, resulting in ambiguity. Try explicitly ",
43-
"importing it from a particular module, or qualifying the name ",
44-
"with the module it should come from.")
45-
elseif kind === Base.PARTITION_KIND_GUARD
46-
print(io, "\nSuggestion: check for spelling errors or missing imports.")
47-
elseif Base.is_some_imported(kind)
48-
print(io, "\nSuggestion: this global was defined as `$(Base.partition_restriction(bpart).globalref)` but not assigned a value.")
49-
end
50-
elseif scope === :static_parameter
51-
print(io, "\nSuggestion: run Test.detect_unbound_args to detect method arguments that do not fully constrain a type parameter.")
52-
elseif scope === :local
53-
print(io, "\nSuggestion: check for an assignment to a local variable that shadows a global of the same name.")
54-
end
55-
else
56-
scope = undef
57-
end
58-
if scope !== Base
59-
warned = _UndefVarError_warnfor(io, [Base], var)
60-
61-
if !warned
62-
modules_to_check = (m for m in Base.loaded_modules_order
63-
if m !== Core && m !== Base && m !== Main && m !== scope)
64-
warned |= _UndefVarError_warnfor(io, modules_to_check, var)
65-
end
66-
67-
warned || _UndefVarError_warnfor(io, [Core, Main], var)
68-
end
69-
return nothing
70-
end
71-
72-
function _UndefVarError_warnfor(io::IO, modules, var::Symbol)
73-
active_mod = Base.active_module()
74-
75-
warned = false
76-
# collect modules which export or make public the variable by
77-
# the module in which the variable is defined
78-
to_warn_about = Dict{Module, Vector{Module}}()
79-
for m in modules
80-
# only include in info if binding has a value and is exported or public
81-
if !Base.isdefined(m, var) || (!Base.isexported(m, var) && !Base.ispublic(m, var))
82-
continue
83-
end
84-
warned = true
85-
86-
# handle case where the undefined variable is the name of a loaded module
87-
if Symbol(m) == var && !isdefined(active_mod, var)
88-
print(io, "\nHint: $m is loaded but not imported in the active module $active_mod.")
89-
continue
90-
end
91-
92-
binding_m = Base.binding_module(m, var)
93-
if !haskey(to_warn_about, binding_m)
94-
to_warn_about[binding_m] = [m]
95-
else
96-
push!(to_warn_about[binding_m], m)
97-
end
98-
end
99-
100-
for (binding_m, modules) in pairs(to_warn_about)
101-
print(io, "\nHint: a global variable of this name also exists in ", binding_m, ".")
102-
for m in modules
103-
m == binding_m && continue
104-
how_available = if Base.isexported(m, var)
105-
"exported by"
106-
elseif Base.ispublic(m, var)
107-
"declared public in"
108-
end
109-
print(io, "\n - Also $how_available $m")
110-
if !isdefined(active_mod, nameof(m)) || (getproperty(active_mod, nameof(m)) !== m)
111-
print(io, " (loaded but not imported in $active_mod)")
112-
end
113-
print(io, ".")
114-
end
115-
end
116-
return warned
11733
end
11834

11935
function __init__()
12036
Base.REPL_MODULE_REF[] = REPL
121-
Base.Experimental.register_error_hint(UndefVarError_hint, UndefVarError)
37+
Base.Experimental.register_error_hint(UndefVarError_REPL_hint, UndefVarError)
12238
return nothing
12339
end
12440

‎stdlib/REPL/test/repl.jl‎

Lines changed: 0 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -1832,85 +1832,6 @@ fake_repl() do stdin_write, stdout_read, repl
18321832
@test contains(txt, "Some type information was truncated. Use `show(err)` to see complete types.")
18331833
end
18341834

1835-
try # test the functionality of `UndefVarError_hint`
1836-
@assert isempty(Base.Experimental._hint_handlers)
1837-
Base.Experimental.register_error_hint(REPL.UndefVarError_hint, UndefVarError)
1838-
1839-
fake_repl() do stdin_write, stdout_read, repl
1840-
backend = REPL.REPLBackend()
1841-
repltask = @async REPL.run_repl(repl; backend)
1842-
write(stdin_write, """
1843-
module A53000
1844-
export f
1845-
f() = 0.0
1846-
end
1847-
1848-
module C_outer_53000
1849-
import ..A53000: f
1850-
public f
1851-
1852-
module C_inner_53000
1853-
import ..C_outer_53000: f
1854-
export f
1855-
end
1856-
end
1857-
1858-
module D_53000
1859-
public f
1860-
f() = 1.0
1861-
end
1862-
1863-
C_inner_53000 = "I'm a decoy with the same name as C_inner_53000!"
1864-
1865-
append!(Base.loaded_modules_order, [A53000, C_outer_53000, C_outer_53000.C_inner_53000, D_53000])
1866-
f
1867-
"""
1868-
)
1869-
write(stdin_write, "\nZZZZZ\n")
1870-
txt = readuntil(stdout_read, "ZZZZZ")
1871-
write(stdin_write, '\x04')
1872-
wait(repltask)
1873-
@test occursin("Hint: a global variable of this name also exists in Main.A53000.", txt)
1874-
@test occursin("Hint: a global variable of this name also exists in Main.D_53000.", txt)
1875-
@test occursin("- Also declared public in Main.C_outer_53000.", txt)
1876-
@test occursin("- Also exported by Main.C_outer_53000.C_inner_53000 (loaded but not imported in Main).", txt)
1877-
end
1878-
catch e
1879-
# fail test if error
1880-
@test false
1881-
finally
1882-
empty!(Base.Experimental._hint_handlers)
1883-
end
1884-
1885-
try # test the functionality of `UndefVarError_hint` against import clashes
1886-
@assert isempty(Base.Experimental._hint_handlers)
1887-
Base.Experimental.register_error_hint(REPL.UndefVarError_hint, UndefVarError)
1888-
1889-
@eval module X
1890-
1891-
module A
1892-
export x
1893-
x = 1
1894-
end # A
1895-
1896-
module B
1897-
export x
1898-
x = 2
1899-
end # B
1900-
1901-
using .A, .B
1902-
1903-
end # X
1904-
1905-
expected_message = string("\nHint: It looks like two or more modules export different ",
1906-
"bindings with this name, resulting in ambiguity. Try explicitly ",
1907-
"importing it from a particular module, or qualifying the name ",
1908-
"with the module it should come from.")
1909-
@test_throws expected_message X.x
1910-
finally
1911-
empty!(Base.Experimental._hint_handlers)
1912-
end
1913-
19141835
# Hints for tab completes
19151836

19161837
fake_repl() do stdin_write, stdout_read, repl

‎test/errorshow.jl‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ Base.Experimental.register_error_hint(Base.methods_on_iterable, MethodError)
1212
Base.Experimental.register_error_hint(Base.nonsetable_type_hint_handler, MethodError)
1313
Base.Experimental.register_error_hint(Base.fielderror_listfields_hint_handler, FieldError)
1414
Base.Experimental.register_error_hint(Base.fielderror_dict_hint_handler, FieldError)
15-
1615
@testset "SystemError" begin
1716
err = try; systemerror("reason", Cint(0)); false; catch ex; ex; end::SystemError
1817
errs = sprint(Base.showerror, err)
@@ -878,6 +877,67 @@ end
878877
@test occursin(hintExpected, errorMsg)
879878
end
880879

880+
# UndefVar error hints
881+
module A53000
882+
export f
883+
f() = 0.0
884+
end
885+
886+
module C_outer_53000
887+
import ..A53000: f
888+
public f
889+
890+
module C_inner_53000
891+
import ..C_outer_53000: f
892+
export f
893+
end
894+
end
895+
896+
module D_53000
897+
public f
898+
f() = 1.0
899+
end
900+
901+
C_inner_53000 = "I'm a decoy with the same name as C_inner_53000!"
902+
903+
Base.Experimental.register_error_hint(Base.UndefVarError_hint, UndefVarError)
904+
905+
@testset "undefvar error hints" begin
906+
old_modules_order = Base.loaded_modules_order
907+
append!(Base.loaded_modules_order, [A53000, C_outer_53000, C_outer_53000.C_inner_53000, D_53000])
908+
test = @test_throws UndefVarError f
909+
ex = test.value::UndefVarError
910+
errormsg = sprint(Base.showerror, ex)
911+
mod = @__MODULE__
912+
@test occursin("Hint: a global variable of this name also exists in $mod.A53000.", errormsg)
913+
@test occursin("Hint: a global variable of this name also exists in $mod.D_53000.", errormsg)
914+
@test occursin("- Also declared public in $mod.C_outer_53000", errormsg)
915+
@test occursin("- Also exported by $mod.C_outer_53000.C_inner_53000 (loaded but not imported in Main).", errormsg)
916+
copy!(Base.loaded_modules_order, old_modules_order)
917+
end
918+
@testset " test the functionality of `UndefVarError_hint` against import clashes" begin
919+
@eval module X
920+
module A
921+
export x
922+
x = 1
923+
end # A
924+
925+
module B
926+
export x
927+
x = 2
928+
end # B
929+
930+
using .A, .B
931+
932+
end # X
933+
934+
expected_message = string("\nHint: It looks like two or more modules export different ",
935+
"bindings with this name, resulting in ambiguity. Try explicitly ",
936+
"importing it from a particular module, or qualifying the name ",
937+
"with the module it should come from.")
938+
@test_throws expected_message X.x
939+
end
940+
881941
# test showing MethodError with type argument
882942
struct NoMethodsDefinedHere; end
883943
let buf = IOBuffer()

0 commit comments

Comments
 (0)