From 4ca97dc26d38fc6d7f9b2eb7bb2d654fd781a729 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Fri, 3 Oct 2025 21:02:21 +0000 Subject: [PATCH] docs: fix crashes on searching documentation of various expressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a crash when calling `@doc` on non-document-able expressions such as `:()` so that docs can be returned for the expression itself. Expands `astname` to handle valid expression types explicitly, removes the unused, undocumented `@var` export from Base.Docs, and updates the error message to be clearer. 🤖 Generated with some assistance from Claude Code. --- base/docs/Docs.jl | 31 +++++++++++++++++++++++++------ base/docs/bindings.jl | 14 +++----------- stdlib/REPL/src/docview.jl | 11 ++++++++--- test/docs.jl | 13 +++++++++---- 4 files changed, 45 insertions(+), 24 deletions(-) diff --git a/base/docs/Docs.jl b/base/docs/Docs.jl index 13a5b35a115da..1026c70617ea1 100644 --- a/base/docs/Docs.jl +++ b/base/docs/Docs.jl @@ -295,16 +295,29 @@ catdoc(xs...) = vcat(xs...) const keywords = Dict{Symbol, DocStr}() -namify(@nospecialize x) = astname(x, isexpr(x, :macro))::Union{Symbol,Expr,GlobalRef} +namify(@nospecialize x) = astname(x, isexpr(x, :macro)) function astname(x::Expr, ismacro::Bool) head = x.head if head === :. ismacro ? macroname(x) : x - elseif head === :call && isexpr(x.args[1], :(::)) - return astname((x.args[1]::Expr).args[end], ismacro) +elseif head === :call && length(x.args) >= 1 && isexpr(x.args[1], :(::)) + # for documenting (x::y)(args...), extract the name from y + # otherwise, for documenting `x::y`, it will be extracted from x + astname((x.args[1]::Expr).args[end], ismacro) else - n = isexpr(x, (:module, :struct)) ? 2 : 1 + n = if isexpr(x, (:module, :struct)) + 2 + elseif isexpr(x, (:call, :macrocall, :function, :(=), :macro, :where, :curly, + :(::), :(<:), :(>:), :local, :global, :const, :atomic, + :copyast, :quote, :inert, :primitive, :abstract, + :escape, :var"hygienic-scope")) + # similar to is_function_def, but without -> and with various assignments, quoted statements, and miscellaneous that might be encountered in struct definitions also + 1 + else + return x # nothing to see here--bindingexpr will convert this to an error if defining a doc + end + length(x.args) < n && return x astname(x.args[n], ismacro) end end @@ -356,7 +369,7 @@ function metadata(__source__, __module__, expr, ismodule) if isa(eachex, Symbol) || isexpr(eachex, :(::)) # a field declaration if last_docstr !== nothing - push!(fields, P(namify(eachex::Union{Symbol,Expr}), last_docstr)) + push!(fields, P(namify(eachex), last_docstr)) last_docstr = nothing end elseif isexpr(eachex, :function) || isexpr(eachex, :(=)) @@ -610,7 +623,13 @@ function simple_lookup_doc(ex) elseif !isa(ex, Expr) && !isa(ex, Symbol) return :($(_doc)($(typeof)($(esc(ex))))) end - binding = esc(bindingexpr(namify(ex))) + name = namify(ex) + # If namify couldn't extract a meaningful name and returned an Expr + # that can't be converted to a binding, treat it like a value + if isa(name, Expr) && !isexpr(name, :(.)) + return :($(_doc)($(typeof)($(esc(ex))))) + end + binding = esc(bindingexpr(name)) if isexpr(ex, :call) || isexpr(ex, :macrocall) || isexpr(ex, :where) sig = esc(signature(ex)) :($(_doc)($binding, $sig)) diff --git a/base/docs/bindings.jl b/base/docs/bindings.jl index 34aa87bd13076..fc72375e8cebe 100644 --- a/base/docs/bindings.jl +++ b/base/docs/bindings.jl @@ -1,7 +1,5 @@ # This file is a part of Julia. License is MIT: https://julialang.org/license -export @var - struct Binding mod::Module var::Symbol @@ -20,17 +18,11 @@ defined(b::Binding) = invokelatest(isdefinedglobal, b.mod, b.var) resolve(b::Binding) = invokelatest(getglobal, b.mod, b.var) function splitexpr(x::Expr) - isexpr(x, :macrocall) ? splitexpr(x.args[1]) : - isexpr(x, :.) ? (x.args[1], x.args[2]) : - error("Invalid @var syntax `$x`.") + isexpr(x, :.) ? (x.args[1], x.args[2]) : error("Could not find something to document in `$x`.") end -splitexpr(s::Symbol) = Expr(:macrocall, getfield(Base, Symbol("@__MODULE__")), nothing), quot(s) +splitexpr(s::Symbol) = :($Base.@__MODULE__), quot(s) # this somewhat complex form allows deferring resolving the Module for module docstring until after the module is created splitexpr(r::GlobalRef) = r.mod, quot(r.name) -splitexpr(other) = error("Invalid @var syntax `$other`.") - -macro var(x) - esc(bindingexpr(x)) -end +splitexpr(other) = error("Could not find something to document in `$other`.") function Base.show(io::IO, b::Binding) if b.mod === Base.active_module() diff --git a/stdlib/REPL/src/docview.jl b/stdlib/REPL/src/docview.jl index 52153a12f606f..e05b3075d9f7c 100644 --- a/stdlib/REPL/src/docview.jl +++ b/stdlib/REPL/src/docview.jl @@ -256,7 +256,7 @@ doc(obj::UnionAll) = doc(Base.unwrap_unionall(obj)) doc(object, sig::Type = Union{}) = doc(aliasof(object, typeof(object)), sig) doc(object, sig...) = doc(object, Tuple{sig...}) -function lookup_doc(ex) +function lookup_doc(@nospecialize(ex)) if isa(ex, Expr) && ex.head !== :(.) && Base.isoperator(ex.head) # handle syntactic operators, e.g. +=, ::, .= ex = ex.head @@ -284,7 +284,13 @@ function lookup_doc(ex) end end end - binding = esc(bindingexpr(namify(ex))) + name = namify(ex) + # If namify couldn't extract a meaningful name and returned an Expr + # that can't be converted to a binding, treat it like a value + if isa(name, Expr) && !isexpr(name, :(.)) + return :($(doc)($(typeof)($(esc(ex))))) + end + binding = esc(bindingexpr(name)) if isexpr(ex, :call) || isexpr(ex, :macrocall) || isexpr(ex, :where) sig = esc(signature(ex)) :($(doc)($binding, $sig)) @@ -579,7 +585,6 @@ isregex(x) = isexpr(x, :macrocall, 3) && x.args[1] === Symbol("@r_str") && !isem repl(io::IO, ex::Expr; brief::Bool=true, mod::Module=Main, internal_accesses::Union{Nothing, Set{Pair{Module,Symbol}}}=nothing) = isregex(ex) ? :(apropos($io, $ex)) : _repl(ex, brief, mod, internal_accesses) repl(io::IO, str::AbstractString; brief::Bool=true, mod::Module=Main, internal_accesses::Union{Nothing, Set{Pair{Module,Symbol}}}=nothing) = :(apropos($io, $str)) repl(io::IO, other; brief::Bool=true, mod::Module=Main, internal_accesses::Union{Nothing, Set{Pair{Module,Symbol}}}=nothing) = esc(:(@doc $other)) # TODO: track internal_accesses -#repl(io::IO, other) = lookup_doc(other) # TODO repl(x; brief::Bool=true, mod::Module=Main) = repl(stdout, x; brief, mod) diff --git a/test/docs.jl b/test/docs.jl index 6c625735ecc7e..e1b212944016c 100644 --- a/test/docs.jl +++ b/test/docs.jl @@ -1,6 +1,10 @@ # This file is a part of Julia. License is MIT: https://julialang.org/license -import Base.Docs: meta, @var, DocStr, parsedoc +import Base.Docs: meta, DocStr, parsedoc, bindingexpr, namify + +macro var(x) # just for testing bindingexpr/nameify more conveniently + esc(bindingexpr(namify(x))) +end # check that @doc can work before REPL is loaded @test !startswith(read(`$(Base.julia_cmd()) -E '@doc sin'`, String), "nothing") @@ -58,6 +62,8 @@ macro macro_doctest() end @test (@eval @doc $(Meta.parse("``"))) == (@doc @cmd) @test (@eval @doc $(Meta.parse("123456789012345678901234567890"))) == (@doc @int128_str) @test (@eval @doc $(Meta.parse("1234567890123456789012345678901234567890"))) == (@doc @big_str) +# Test that @doc doesn't crash on empty tuple expression (issue #XXXXX) +@test (@doc :()) == (@doc Expr) # test that random stuff interpolated into docstrings doesn't break search or other methods here @doc doc""" @@ -1239,7 +1245,7 @@ end # Bindings. -import Base.Docs: @var, Binding, defined +import Base.Docs: Binding, defined let x = Binding(Base, Symbol("@inline")) @test defined(x) == true @@ -1565,8 +1571,7 @@ Base.@ccallable c51586_long()::Int = 3 @testset "Docs docstrings" begin undoc = Docs.undocumented_names(Docs) - @test_broken isempty(undoc) - @test undoc == [Symbol("@var")] + @test isempty(undoc) end # Docing the macroception macro