From 5f466d59ec39b9d7083bdf0fe8aaa9b8b60712b7 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Mon, 26 Feb 2024 01:33:42 +0100 Subject: [PATCH 1/4] Provide better error hint when UndefVarError results from name clashes --- stdlib/REPL/src/REPL.jl | 8 +++++++- stdlib/REPL/test/repl.jl | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/stdlib/REPL/src/REPL.jl b/stdlib/REPL/src/REPL.jl index 840e6daaf1909..e0ebc3f15be41 100644 --- a/stdlib/REPL/src/REPL.jl +++ b/stdlib/REPL/src/REPL.jl @@ -44,7 +44,13 @@ function UndefVarError_hint(io::IO, ex::UndefVarError) if C_NULL == owner # No global of this name exists in this module. # This is the common case, so do not print that information. - print(io, "\nSuggestion: check for spelling errors or missing imports.") + # It could be the binding was exported by two modules, which we can detect + # by the `usingfailed` flag in the binding: + if isdefined(bnd, :flags) && Bool(bnd.flags >> 4 & 1) # magic location of the `usingfailed` flag + print(io, "\nHint: It looks like this name was exported by two different modules, resulting in ambiguity. Try explicitly importing it, or qualifying the name with the module it should come from.") + else + print(io, "\nSuggestion: check for spelling errors or missing imports.") + end owner = bnd else owner = unsafe_pointer_to_objref(owner)::Core.Binding diff --git a/stdlib/REPL/test/repl.jl b/stdlib/REPL/test/repl.jl index 83df34a056578..6ec735b41829d 100644 --- a/stdlib/REPL/test/repl.jl +++ b/stdlib/REPL/test/repl.jl @@ -1696,6 +1696,41 @@ finally empty!(Base.Experimental._hint_handlers) end +try # test the functionality of `UndefVarError_hint` against import clashes + @assert isempty(Base.Experimental._hint_handlers) + Base.Experimental.register_error_hint(REPL.UndefVarError_hint, UndefVarError) + + fake_repl() do stdin_write, stdout_read, repl + backend = REPL.REPLBackend() + repltask = @async REPL.run_repl(repl; backend) + write(stdin_write, """ + module X + + module A + export x + x = 1 + end # A + + module B + export x + x = 2 + end # B + + using .A, .B + + end # X + """) + write(stdin_write, + "\nX.x\n\"ZZZZZ\"\n") + txt = readuntil(stdout_read, "ZZZZZ") + write(stdin_write, '\x04') + wait(repltask) + @test occursin("Hint: It looks like this name was exported by two different modules, resulting in ambiguity. Try explicitly importing it, or qualifying the name with the module it should come from.", txt) + end +finally + empty!(Base.Experimental._hint_handlers) +end + # Hints for tab completes fake_repl() do stdin_write, stdout_read, repl From 0094c79c7b9222e2b66bbcf8f0f1a1ebf4e9138b Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Mon, 26 Feb 2024 20:46:43 +0100 Subject: [PATCH 2/4] use `@eval`, not fake-repl, for better test --- stdlib/REPL/test/repl.jl | 41 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/stdlib/REPL/test/repl.jl b/stdlib/REPL/test/repl.jl index 6ec735b41829d..32d0003f3598c 100644 --- a/stdlib/REPL/test/repl.jl +++ b/stdlib/REPL/test/repl.jl @@ -1700,33 +1700,30 @@ try # test the functionality of `UndefVarError_hint` against import clashes @assert isempty(Base.Experimental._hint_handlers) Base.Experimental.register_error_hint(REPL.UndefVarError_hint, UndefVarError) - fake_repl() do stdin_write, stdout_read, repl - backend = REPL.REPLBackend() - repltask = @async REPL.run_repl(repl; backend) - write(stdin_write, """ - module X + @eval module X - module A - export x - x = 1 - end # A + module A + export x + x = 1 + end # A - module B - export x - x = 2 - end # B + module B + export x + x = 2 + end # B - using .A, .B + using .A, .B - end # X - """) - write(stdin_write, - "\nX.x\n\"ZZZZZ\"\n") - txt = readuntil(stdout_read, "ZZZZZ") - write(stdin_write, '\x04') - wait(repltask) - @test occursin("Hint: It looks like this name was exported by two different modules, resulting in ambiguity. Try explicitly importing it, or qualifying the name with the module it should come from.", txt) + end # X + + str = try + X.x + @test false # should not reach this + catch e + sprint(Base.showerror, e) end + + @test contains(str, "Hint: It looks like this name was exported by two different modules, resulting in ambiguity. Try explicitly importing it, or qualifying the name with the module it should come from.") finally empty!(Base.Experimental._hint_handlers) end From c0a6cb7b2af467a978d595ba15b426ec128136fa Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Tue, 27 Feb 2024 19:23:35 +0100 Subject: [PATCH 3/4] Update stdlib/REPL/test/repl.jl Co-authored-by: Jameson Nash --- stdlib/REPL/test/repl.jl | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/stdlib/REPL/test/repl.jl b/stdlib/REPL/test/repl.jl index 32d0003f3598c..899bd590c5c23 100644 --- a/stdlib/REPL/test/repl.jl +++ b/stdlib/REPL/test/repl.jl @@ -1716,14 +1716,7 @@ try # test the functionality of `UndefVarError_hint` against import clashes end # X - str = try - X.x - @test false # should not reach this - catch e - sprint(Base.showerror, e) - end - - @test contains(str, "Hint: It looks like this name was exported by two different modules, resulting in ambiguity. Try explicitly importing it, or qualifying the name with the module it should come from.") + @test_throws "Hint: It looks like this name was exported by two different modules, resulting in ambiguity. Try explicitly importing it, or qualifying the name with the module it should come from." X.x finally empty!(Base.Experimental._hint_handlers) end From f0827ee3a7627872a5b5296fb8d105f866c37708 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Wed, 28 Feb 2024 01:11:14 +0100 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Alex Arslan --- stdlib/REPL/src/REPL.jl | 5 ++++- stdlib/REPL/test/repl.jl | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/stdlib/REPL/src/REPL.jl b/stdlib/REPL/src/REPL.jl index e0ebc3f15be41..d1d4681b0a7c6 100644 --- a/stdlib/REPL/src/REPL.jl +++ b/stdlib/REPL/src/REPL.jl @@ -47,7 +47,10 @@ function UndefVarError_hint(io::IO, ex::UndefVarError) # It could be the binding was exported by two modules, which we can detect # by the `usingfailed` flag in the binding: if isdefined(bnd, :flags) && Bool(bnd.flags >> 4 & 1) # magic location of the `usingfailed` flag - print(io, "\nHint: It looks like this name was exported by two different modules, resulting in ambiguity. Try explicitly importing it, or qualifying the name with the module it should come from.") + print(io, "\nHint: 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.") else print(io, "\nSuggestion: check for spelling errors or missing imports.") end diff --git a/stdlib/REPL/test/repl.jl b/stdlib/REPL/test/repl.jl index 899bd590c5c23..12f3f8956122e 100644 --- a/stdlib/REPL/test/repl.jl +++ b/stdlib/REPL/test/repl.jl @@ -1716,7 +1716,11 @@ try # test the functionality of `UndefVarError_hint` against import clashes end # X - @test_throws "Hint: It looks like this name was exported by two different modules, resulting in ambiguity. Try explicitly importing it, or qualifying the name with the module it should come from." X.x + expected_message = string("\nHint: 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.") + @test_throws expected_message X.x finally empty!(Base.Experimental._hint_handlers) end