From d5339c5b4046660a6f579f45acb3777d95dc9b7e Mon Sep 17 00:00:00 2001 From: Alex Arslan Date: Wed, 13 Dec 2023 18:31:12 -0800 Subject: [PATCH 1/4] Don't suggest inaccessible bindings in REPL help Fixes 52472, which was caused by `names` being changed to also return public, unexported symbols. --- stdlib/REPL/src/docview.jl | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/stdlib/REPL/src/docview.jl b/stdlib/REPL/src/docview.jl index 1d5bd12cdda58..675f6d0f0f1b0 100644 --- a/stdlib/REPL/src/docview.jl +++ b/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 @@ -818,12 +818,15 @@ print_correction(word, mod::Module) = print_correction(stdout, word, mod) 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) + symbols = filter!(s -> !isdeprecated(mod, s), names(mod; all=true, imported=true)) + for used in moduleusings(mod) + append!(symbols, filter!(s -> !isdeprecated(used, s) && isexported(used, s), names(used))) + end + append!(symbols, keys(Base.Docs.keywords)) + unique!(symbols) + return [String(x) for x in symbols if !occursin('#', String(x))] +end function doc_completions(name, mod::Module=Main) res = fuzzysort(name, accessible(mod)) From 394d1831832c9154b11cdcb7caeb63eec79d9252 Mon Sep 17 00:00:00 2001 From: Alex Arslan Date: Thu, 14 Dec 2023 13:12:32 -0800 Subject: [PATCH 2/4] Include public, unexported names in REPL help search --- stdlib/REPL/src/docview.jl | 88 +++++++++++++++++++++++------ stdlib/REPL/test/docview.jl | 36 +++++++++++- stdlib/REPL/test/replcompletions.jl | 2 +- 3 files changed, 105 insertions(+), 21 deletions(-) diff --git a/stdlib/REPL/src/docview.jl b/stdlib/REPL/src/docview.jl index 675f6d0f0f1b0..65bb9a6928152 100644 --- a/stdlib/REPL/src/docview.jl +++ b/stdlib/REPL/src/docview.jl @@ -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,30 +868,31 @@ print_correction(word, mod::Module) = print_correction(stdout, word, mod) # Completion data - moduleusings(mod) = ccall(:jl_module_usings, Any, (Any,), mod) function accessible(mod::Module) - symbols = filter!(s -> !isdeprecated(mod, s), names(mod; all=true, imported=true)) + bindings = Set(AccessibleBinding(s) for s in names(mod; all=true, imported=true) + if !isdeprecated(mod, s)) for used in moduleusings(mod) - append!(symbols, filter!(s -> !isdeprecated(used, s) && isexported(used, s), names(used))) + union!(bindings, (AccessibleBinding(used, s) for s in names(used) + if !isdeprecated(used, s))) end - append!(symbols, keys(Base.Docs.keywords)) - unique!(symbols) - return [String(x) for x in symbols if !occursin('#', String(x))] + 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 diff --git a/stdlib/REPL/test/docview.jl b/stdlib/REPL/test/docview.jl index 897a69a9266ab..123ff820bc939 100644 --- a/stdlib/REPL/test/docview.jl +++ b/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 diff --git a/stdlib/REPL/test/replcompletions.jl b/stdlib/REPL/test/replcompletions.jl index 22ef938002514..7fd36e3d147e6 100644 --- a/stdlib/REPL/test/replcompletions.jl +++ b/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 From e479b4cf1dca6f99effd6d8704fc5f4c09a28b06 Mon Sep 17 00:00:00 2001 From: Alex Arslan Date: Thu, 14 Dec 2023 14:49:45 -0800 Subject: [PATCH 3/4] Fix use of REPL-internal function in test/docs.jl --- test/docs.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/docs.jl b/test/docs.jl index 4eab0dd36361d..36893b8614170 100644 --- a/test/docs.jl +++ b/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 """ From 278bfe09d3ad67569febc6c9f5ff1b7f09a9dc0e Mon Sep 17 00:00:00 2001 From: Alex Arslan Date: Wed, 7 Feb 2024 09:58:15 -0800 Subject: [PATCH 4/4] Thank you Jameson <3 Co-authored-by: Jameson Nash --- stdlib/REPL/src/docview.jl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stdlib/REPL/src/docview.jl b/stdlib/REPL/src/docview.jl index 65bb9a6928152..79cc6b48d2f4d 100644 --- a/stdlib/REPL/src/docview.jl +++ b/stdlib/REPL/src/docview.jl @@ -765,7 +765,7 @@ function fuzzyscore(needle::AbstractString, haystack::AccessibleBinding) end function fuzzysort(search::String, candidates::Vector{AccessibleBinding}) - scores = map(cand -> fuzzyscore(search, cand.name), candidates) + scores = map(cand -> fuzzyscore(search, cand), candidates) candidates[sortperm(scores)] |> reverse end @@ -790,8 +790,8 @@ function levenshtein(s1, s2) end function levsort(search::String, candidates::Vector{AccessibleBinding}) - scores = map(candidates) do (; name) - (Float64(levenshtein(search, name)), -fuzzyscore(search, name)) + scores = map(candidates) do cand + (Float64(levenshtein(search, cand.name)), -fuzzyscore(search, cand)) end candidates = candidates[sortperm(scores)] i = 0