From 2224a0e6897ce72514d1dc3482ab0eaf70f109d4 Mon Sep 17 00:00:00 2001 From: Adam Wheeler Date: Tue, 3 Dec 2024 17:12:49 -0500 Subject: [PATCH 1/3] group UndefVarError hints by parent module --- stdlib/REPL/src/REPL.jl | 74 +++++++++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 22 deletions(-) diff --git a/stdlib/REPL/src/REPL.jl b/stdlib/REPL/src/REPL.jl index 6c3f4bd4ba73a..24f10c5b756ee 100644 --- a/stdlib/REPL/src/REPL.jl +++ b/stdlib/REPL/src/REPL.jl @@ -55,37 +55,67 @@ function UndefVarError_hint(io::IO, ex::UndefVarError) else scope = undef end - if scope !== Base && !_UndefVarError_warnfor(io, Base, var) - warned = false - for m in Base.loaded_modules_order - m === Core && continue - m === Base && continue - m === Main && continue - m === scope && continue - warned |= _UndefVarError_warnfor(io, m, var) + if scope !== Base + warned = _UndefVarError_warnfor(io, [Base], var) + + if !warned + modules_to_check = (m for m in Base.loaded_modules_order + if m !== Core && m !== Base && m !== Main && m !== scope) + warned |= _UndefVarError_warnfor(io, modules_to_check, var) end - warned || - _UndefVarError_warnfor(io, Core, var) || - _UndefVarError_warnfor(io, Main, var) + + warned || _UndefVarError_warnfor(io, [Core, Main], var) end return nothing end -function _UndefVarError_warnfor(io::IO, m::Module, var::Symbol) - Base.isbindingresolved(m, var) || return false - (Base.isexported(m, var) || Base.ispublic(m, var)) || return false +function _UndefVarError_warnfor(io::IO, modules, var::Symbol) active_mod = Base.active_module() - print(io, "\nHint: ") - if isdefined(active_mod, Symbol(m)) - print(io, "a global variable of this name also exists in $m.") - else - if Symbol(m) == var - print(io, "$m is loaded but not imported in the active module $active_mod.") + + warned = false + # collect modules which export or make public the variable by + # the parentmodule in which the variable is defined + to_warn_about = Dict{Module, Vector{Module}}() + for m in modules + # only warn if exported or public TODO why did this used to check isbindingresolved? + if !Base.isexported(m, var) && !Base.ispublic(m, var) + continue + end + warned = true + + # handle case where the undefined variable is the name of a loaded module + if Symbol(m) == var && !isdefined(active_mod, var) + print(io, "\nHint: $m is loaded but not imported in the active module $active_mod.") + continue + end + + parent_m = parentmodule(getproperty(m, var)) + if !haskey(to_warn_about, parent_m) + to_warn_about[parent_m] = [m] else - print(io, "a global variable of this name may be made accessible by importing $m in the current active module $active_mod") + push!(to_warn_about[parent_m], m) + end + end + + if !isempty(to_warn_about) + for (parent_m, modules) in pairs(to_warn_about) + print(io, "\nHint: a global variable of this name also exists in ", parent_m) + for m in modules + m == parent_m && continue + how_available = if Base.isexported(m, var) + "exported by" + elseif Base.ispublic(m, var) + "made available as public by" + end + print(io, "\n - Also $how_available $m") + if !isdefined(active_mod, Symbol(m)) + print(io, ", which would need to be imported in $active_mod") + end + print(io, ".") + end end end - return true + return warned end function __init__() From 00d518121952d40af379da26e5c26fbe39acff2c Mon Sep 17 00:00:00 2001 From: Adam Wheeler Date: Wed, 4 Dec 2024 15:42:32 -0500 Subject: [PATCH 2/3] also filter on isbindingresolved --- stdlib/REPL/src/REPL.jl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stdlib/REPL/src/REPL.jl b/stdlib/REPL/src/REPL.jl index 24f10c5b756ee..4e8b44c6ff374 100644 --- a/stdlib/REPL/src/REPL.jl +++ b/stdlib/REPL/src/REPL.jl @@ -77,8 +77,8 @@ function _UndefVarError_warnfor(io::IO, modules, var::Symbol) # the parentmodule in which the variable is defined to_warn_about = Dict{Module, Vector{Module}}() for m in modules - # only warn if exported or public TODO why did this used to check isbindingresolved? - if !Base.isexported(m, var) && !Base.ispublic(m, var) + # only warn if binding is resolved and exported or public + if !Base.isbindingresolved(m, var) || (!Base.isexported(m, var) && !Base.ispublic(m, var)) continue end warned = true @@ -109,7 +109,7 @@ function _UndefVarError_warnfor(io::IO, modules, var::Symbol) end print(io, "\n - Also $how_available $m") if !isdefined(active_mod, Symbol(m)) - print(io, ", which would need to be imported in $active_mod") + print(io, " (loaded but not imported in $active_mod)") end print(io, ".") end From 819b8b16f176e6e510b41d4f1621c5f08916d146 Mon Sep 17 00:00:00 2001 From: Adam Wheeler Date: Thu, 5 Dec 2024 10:23:12 -0500 Subject: [PATCH 3/3] use binding_module; remove whitespace --- stdlib/REPL/src/REPL.jl | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/stdlib/REPL/src/REPL.jl b/stdlib/REPL/src/REPL.jl index 4e8b44c6ff374..0a5446ef6a3b2 100644 --- a/stdlib/REPL/src/REPL.jl +++ b/stdlib/REPL/src/REPL.jl @@ -55,7 +55,7 @@ function UndefVarError_hint(io::IO, ex::UndefVarError) else scope = undef end - if scope !== Base + if scope !== Base warned = _UndefVarError_warnfor(io, [Base], var) if !warned @@ -73,11 +73,11 @@ function _UndefVarError_warnfor(io::IO, modules, var::Symbol) active_mod = Base.active_module() warned = false - # collect modules which export or make public the variable by - # the parentmodule in which the variable is defined + # collect modules which export or make public the variable by + # the module in which the variable is defined to_warn_about = Dict{Module, Vector{Module}}() for m in modules - # only warn if binding is resolved and exported or public + # only warn if binding is resolved and exported or public if !Base.isbindingresolved(m, var) || (!Base.isexported(m, var) && !Base.ispublic(m, var)) continue end @@ -89,19 +89,19 @@ function _UndefVarError_warnfor(io::IO, modules, var::Symbol) continue end - parent_m = parentmodule(getproperty(m, var)) - if !haskey(to_warn_about, parent_m) - to_warn_about[parent_m] = [m] + binding_m = Base.binding_module(m, var) + if !haskey(to_warn_about, binding_m) + to_warn_about[binding_m] = [m] else - push!(to_warn_about[parent_m], m) + push!(to_warn_about[binding_m], m) end end if !isempty(to_warn_about) - for (parent_m, modules) in pairs(to_warn_about) - print(io, "\nHint: a global variable of this name also exists in ", parent_m) + for (binding_m, modules) in pairs(to_warn_about) + print(io, "\nHint: a global variable of this name also exists in ", binding_m, ".") for m in modules - m == parent_m && continue + m == binding_m && continue how_available = if Base.isexported(m, var) "exported by" elseif Base.ispublic(m, var)