Skip to content

Commit a987f56

Browse files
Liozouaviatesk
andauthored
Fix ?(#TAB method search name exploration (#52555)
Fix #52551. This PR ensures that a `SomeModule.?(...#TAB` completion can only suggests method `foo` such that `SomeModule.foo` exists (by checking `isdefined(SomeModule, :foo)`). This is equivalent, I believe, to the initial implementation of #38791, less the bug. Now that we have #51345, we may want to relax the above condition somewhat to include public names present in modules loaded into `SomeModule`, so that, for instance, a direct completion of `?(` would include `@assume_effects`. This could be good for method exploration because even though typing `@assume_effects` with no qualification in `Main` will error, the error now includes the helpful message ``` Hint: a global variable of this name also exists in Base. ``` But that can wait for a later PR anyway, this one is just the bugfix. The bug mentioned at #52551 (comment) dates from the initial #38791 so this could be backported as far back as v1.8. --------- Co-authored-by: Shuhei Kadowaki <[email protected]>
1 parent ae3c711 commit a987f56

File tree

2 files changed

+30
-27
lines changed

2 files changed

+30
-27
lines changed

stdlib/REPL/src/REPLCompletions.jl

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export completions, shell_completions, bslash_completions, completion_text
77
using Core: CodeInfo, MethodInstance, CodeInstance, Const
88
const CC = Core.Compiler
99
using Base.Meta
10-
using Base: propertynames, something
10+
using Base: propertynames, something, IdSet
1111

1212
abstract type Completion end
1313

@@ -718,6 +718,26 @@ function complete_methods(ex_org::Expr, context_module::Module=Main, shift::Bool
718718
end
719719

720720
MAX_ANY_METHOD_COMPLETIONS::Int = 10
721+
function recursive_explore_names!(seen::IdSet, callee_module::Module, initial_module::Module, exploredmodules::IdSet{Module}=IdSet{Module}())
722+
push!(exploredmodules, callee_module)
723+
for name in names(callee_module; all=true, imported=true)
724+
if !Base.isdeprecated(callee_module, name) && !startswith(string(name), '#') && isdefined(initial_module, name)
725+
func = getfield(callee_module, name)
726+
if !isa(func, Module)
727+
funct = Core.Typeof(func)
728+
push!(seen, funct)
729+
elseif isa(func, Module) && func exploredmodules
730+
recursive_explore_names!(seen, func, initial_module, exploredmodules)
731+
end
732+
end
733+
end
734+
end
735+
function recursive_explore_names(callee_module::Module, initial_module::Module)
736+
seen = IdSet{Any}()
737+
recursive_explore_names!(seen, callee_module, initial_module)
738+
seen
739+
end
740+
721741
function complete_any_methods(ex_org::Expr, callee_module::Module, context_module::Module, moreargs::Bool, shift::Bool)
722742
out = Completion[]
723743
args_ex, kwargs_ex, kwargs_flag = try
@@ -733,32 +753,8 @@ function complete_any_methods(ex_org::Expr, callee_module::Module, context_modul
733753
# semicolon for the ".?(" syntax
734754
moreargs && push!(args_ex, Vararg{Any})
735755

736-
seen = Base.IdSet()
737-
for name in names(callee_module; all=true)
738-
if !Base.isdeprecated(callee_module, name) && isdefined(callee_module, name) && !startswith(string(name), '#')
739-
func = getfield(callee_module, name)
740-
if !isa(func, Module)
741-
funct = Core.Typeof(func)
742-
if !in(funct, seen)
743-
push!(seen, funct)
744-
complete_methods!(out, funct, args_ex, kwargs_ex, MAX_ANY_METHOD_COMPLETIONS, false)
745-
end
746-
elseif callee_module === Main && isa(func, Module)
747-
callee_module2 = func
748-
for name in names(callee_module2)
749-
if !Base.isdeprecated(callee_module2, name) && isdefined(callee_module2, name) && !startswith(string(name), '#')
750-
func = getfield(callee_module, name)
751-
if !isa(func, Module)
752-
funct = Core.Typeof(func)
753-
if !in(funct, seen)
754-
push!(seen, funct)
755-
complete_methods!(out, funct, args_ex, kwargs_ex, MAX_ANY_METHOD_COMPLETIONS, false)
756-
end
757-
end
758-
end
759-
end
760-
end
761-
end
756+
for seen_name in recursive_explore_names(callee_module, callee_module)
757+
complete_methods!(out, seen_name, args_ex, kwargs_ex, MAX_ANY_METHOD_COMPLETIONS, false)
762758
end
763759

764760
if !shift

stdlib/REPL/test/replcompletions.jl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,10 @@ let ex = quote
142142
struct WeirdNames end
143143
Base.propertynames(::WeirdNames) = (Symbol("oh no!"), Symbol("oh yes!"))
144144

145+
# https://github.com/JuliaLang/julia/issues/52551#issuecomment-1858543413
146+
export exported_symbol
147+
exported_symbol(::WeirdNames) = nothing
148+
145149
end # module CompletionFoo
146150
test_repl_comp_dict = CompletionFoo.test_dict
147151
test_repl_comp_customdict = CompletionFoo.test_customdict
@@ -742,6 +746,9 @@ end
742746

743747
#TODO: @test_nocompletion("CompletionFoo.?(3; len2=5; ")
744748

749+
# https://github.com/JuliaLang/julia/issues/52551
750+
@test !isempty(test_complete("?("))
751+
745752
#################################################################
746753

747754
# Test method completion with varargs

0 commit comments

Comments
 (0)