Qualify public, unexported bindings in REPL help

stdlib/REPL/src/docview.jl

@@ -9,7 +9,7 @@ using Base.Docs: catdoc, modules, DocStr, Binding, MultiDoc, keywords, isfield,
 
 import Base.Docs: doc, formatdoc, parsedoc, apropos
 
-using Base: with_output_color, mapany
+using Base: with_output_color, mapany, isdeprecated, isexported
 
 import REPL
 
@@ -424,8 +424,31 @@ end
 
 # repl search and completions for help
 
+# This type is returned from `accessible` and denotes a binding that is accessible within
+# some context. It differs from `Base.Docs.Binding`, which is also used by the REPL, in
+# that it doesn't track the defining module for a symbol unless the symbol is public but
+# not exported, i.e. it's accessible but requires qualification. Using this type rather
+# than `Base.Docs.Binding` simplifies things considerably, partially because REPL searching
+# is based on `String`s, which this type stores, but `Base.Docs.Binding` stores a module
+# and symbol and does not have any notion of the context from which the binding is accessed.
+struct AccessibleBinding
+    source::Union{String,Nothing}
+    name::String
+end
+
+function AccessibleBinding(mod::Module, name::Symbol)
+    m = isexported(mod, name) ? nothing : String(nameof(mod))
+    return AccessibleBinding(m, String(name))
+end
+AccessibleBinding(name::Symbol) = AccessibleBinding(nothing, String(name))
+
+function Base.show(io::IO, b::AccessibleBinding)
+    b.source === nothing || print(io, b.source, '.')
+    print(io, b.name)
+end
 
 quote_spaces(x) = any(isspace, x) ? "'" * x * "'" : x
+quote_spaces(x::AccessibleBinding) = AccessibleBinding(x.source, quote_spaces(x.name))
 
 function repl_search(io::IO, s::Union{Symbol,String}, mod::Module)
     pre = "search:"
@@ -669,6 +692,9 @@ function matchinds(needle, haystack; acronym::Bool = false)
     return is
 end
 
+matchinds(needle, (; name)::AccessibleBinding; acronym::Bool=false) =
+    matchinds(needle, name; acronym)
+
 longer(x, y) = length(x) ≥ length(y) ? (x, true) : (y, false)
 
 bestmatch(needle, haystack) =
@@ -728,8 +754,18 @@ function fuzzyscore(needle::AbstractString, haystack::AbstractString)
     1 - (string_distance(needle, lena, haystack, lenb) / max(lena, lenb))
 end
 
-function fuzzysort(search::String, candidates::Vector{String})
-    scores = map(cand -> fuzzyscore(search, cand), candidates)
+function fuzzyscore(needle::AbstractString, haystack::AccessibleBinding)
+    score = fuzzyscore(needle, haystack.name)
+    haystack.source === nothing && return score
+    # Apply a "penalty" of half an edit if the comparator binding is public but not
+    # exported so that exported/local names that exactly match the search query are
+    # listed first
+    penalty = 1 / (2 * max(length(needle), length(haystack.name)))
+    return max(score - penalty, 0)
+end
+
+function fuzzysort(search::String, candidates::Vector{AccessibleBinding})
+    scores = map(cand -> fuzzyscore(search, cand.name), candidates)
     candidates[sortperm(scores)] |> reverse
 end
 
@@ -753,12 +789,14 @@ function levenshtein(s1, s2)
     return d[m+1, n+1]
 end
 
-function levsort(search::String, candidates::Vector{String})
-    scores = map(cand -> (Float64(levenshtein(search, cand)), -fuzzyscore(search, cand)), candidates)
+function levsort(search::String, candidates::Vector{AccessibleBinding})
+    scores = map(candidates) do (; name)
+        (Float64(levenshtein(search, name)), -fuzzyscore(search, name))
+    end
     candidates = candidates[sortperm(scores)]
     i = 0
     for outer i = 1:length(candidates)
-        levenshtein(search, candidates[i]) > 3 && break
+        levenshtein(search, candidates[i].name) > 3 && break
     end
     return candidates[1:i]
 end
@@ -776,24 +814,39 @@ function printmatch(io::IO, word, match)
     end
 end
 
+function printmatch(io::IO, word, match::AccessibleBinding)
+    match.source === nothing || print(io, match.source, '.')
+    printmatch(io, word, match.name)
+end
+
+function matchlength(x::AccessibleBinding)
+    n = length(x.name)
+    if x.source !== nothing
+        n += length(x.source) + 1  # the +1 is for the `.` separator
+    end
+    return n
+end
+matchlength(x) = length(x)
+
 function printmatches(io::IO, word, matches; cols::Int = _displaysize(io)[2])
     total = 0
     for match in matches
-        total + length(match) + 1 > cols && break
+        ml = matchlength(match)
+        total + ml + 1 > cols && break
         fuzzyscore(word, match) < 0.5 && break
         print(io, " ")
         printmatch(io, word, match)
-        total += length(match) + 1
+        total += ml + 1
     end
 end
 
 printmatches(args...; cols::Int = _displaysize(stdout)[2]) = printmatches(stdout, args..., cols = cols)
 
-function print_joined_cols(io::IO, ss::Vector{String}, delim = "", last = delim; cols::Int = _displaysize(io)[2])
+function print_joined_cols(io::IO, ss::Vector{AccessibleBinding}, delim = "", last = delim; cols::Int = _displaysize(io)[2])
     i = 0
     total = 0
     for outer i = 1:length(ss)
-        total += length(ss[i])
+        total += matchlength(ss[i])
         total + max(i-2,0)*length(delim) + (i>1 ? 1 : 0)*length(last) > cols && (i-=1; break)
     end
     join(io, ss[1:i], delim, last)
@@ -815,27 +868,31 @@ print_correction(word, mod::Module) = print_correction(stdout, word, mod)
 
 # Completion data
 
-
 moduleusings(mod) = ccall(:jl_module_usings, Any, (Any,), mod)
 
-filtervalid(names) = filter(x->!occursin(r"#", x), map(string, names))
-
-accessible(mod::Module) =
-    Symbol[filter!(s -> !Base.isdeprecated(mod, s), names(mod, all=true, imported=true));
-           map(names, moduleusings(mod))...;
-           collect(keys(Base.Docs.keywords))] |> unique |> filtervalid
+function accessible(mod::Module)
+    bindings = Set(AccessibleBinding(s) for s in names(mod; all=true, imported=true)
+                   if !isdeprecated(mod, s))
+    for used in moduleusings(mod)
+        union!(bindings, (AccessibleBinding(used, s) for s in names(used)
+                          if !isdeprecated(used, s)))
+    end
+    union!(bindings, (AccessibleBinding(k) for k in keys(Base.Docs.keywords)))
+    filter!(b -> !occursin('#', b.name), bindings)
+    return collect(bindings)
+end
 
 function doc_completions(name, mod::Module=Main)
     res = fuzzysort(name, accessible(mod))
 
     # to insert an entry like `raw""` for `"@raw_str"` in `res`
-    ms = match.(r"^@(.*?)_str$", res)
+    ms = map(c -> match(r"^@(.*?)_str$", c.name), res)
     idxs = findall(!isnothing, ms)
 
     # avoid messing up the order while inserting
     for i in reverse!(idxs)
         c = only((ms[i]::AbstractMatch).captures)
-        insert!(res, i, "$(c)\"\"")
+        insert!(res, i, AccessibleBinding(res[i].source, "$(c)\"\""))
     end
     res
 end

stdlib/REPL/test/docview.jl

@@ -4,9 +4,9 @@ using Test
 import REPL, REPL.REPLCompletions
 import Markdown
 
-function get_help_io(input)
+function get_help_io(input, mod=Main)
     buf = IOBuffer()
-    eval(REPL.helpmode(buf, input))
+    eval(REPL.helpmode(buf, input, mod))
     String(take!(buf))
 end
 get_help_standard(input) = string(eval(REPL.helpmode(IOBuffer(), input)))
@@ -40,7 +40,7 @@ end
     symbols = "@" .* checks .* "_str"
     results = checks .* "\"\""
     for (i,r) in zip(symbols,results)
-        @test r ∈ REPL.doc_completions(i)
+        @test r ∈ string.(REPL.doc_completions(i))
     end
 end
 @testset "fuzzy score" begin
@@ -56,6 +56,13 @@ end
     # Unicode
     @test 1.0 > REPL.fuzzyscore("αkδψm", "αkδm") > 0.0
     @test 1.0 > REPL.fuzzyscore("αkδψm", "α") > 0.0
+
+    exact_match_export = REPL.fuzzyscore("thing", REPL.AccessibleBinding(:thing))
+    exact_match_public = REPL.fuzzyscore("thing", REPL.AccessibleBinding("A", "thing"))
+    inexact_match_export = REPL.fuzzyscore("thing", REPL.AccessibleBinding(:thang))
+    inexact_match_public = REPL.fuzzyscore("thing", REPL.AccessibleBinding("A", "thang"))
+    @test exact_match_export > exact_match_public > inexact_match_export > inexact_match_public
+    @test exact_match_export ≈ 1.0
 end
 
 @testset "Unicode doc lookup (#41589)" begin
@@ -135,3 +142,26 @@ end
 
 # Issue #51344, don't print "internal binding" warning for non-existent bindings.
 @test string(eval(REPL.helpmode("Base.no_such_symbol"))) == "No documentation found.\n\nBinding `Base.no_such_symbol` does not exist.\n"
+
+module TestSuggestPublic
+    export dingo
+    public dango
+    dingo(x) = x + 1
+    dango(x) = x = 2
+end
+using .TestSuggestPublic
+helplines(s) = map(strip, split(get_help_io(s, @__MODULE__), '\n'; keepempty=false))
+@testset "search lists public names" begin
+    lines = helplines("dango")
+    # Ensure that public names that exactly match the search query are listed first
+    # even if they aren't exported, as long as no exact exported/local match exists
+    @test startswith(lines[1], "search: TestSuggestPublic.dango dingo")
+    @test lines[2] == "Couldn't find dango"  # 🙈🍡
+    @test startswith(lines[3], "Perhaps you meant TestSuggestPublic.dango, dingo")
+end
+dango() = "🍡"
+@testset "search prioritizes exported names" begin
+    # Prioritize exported/local names if they exactly match
+    lines = helplines("dango")
+    @test startswith(lines[1], "search: dango TestSuggestPublic.dango dingo")
+end

stdlib/REPL/test/replcompletions.jl

@@ -8,7 +8,7 @@ using REPL
 @testset "Check symbols previously not shown by REPL.doc_completions()" begin
     symbols = ["?","=","[]","[","]","{}","{","}",";","","'","&&","||","julia","Julia","new","@var_str"]
     for i in symbols
-        @test i ∈ REPL.doc_completions(i, Main)
+        @test i ∈ string.(REPL.doc_completions(i, Main))
     end
 end
 

test/docs.jl

@@ -1465,7 +1465,7 @@ end
 end
 
 struct t_docs_abc end
-@test "t_docs_abc" in accessible(@__MODULE__)
+@test "t_docs_abc" in string.(accessible(@__MODULE__))
 
 # Call overloading issues #20087 and #44889
 """