Skip to content

Harden rendered API docs and JET defaults #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ChrisRackauckas merged 1 commit into from
Aug 15, 2026
Merged

Harden rendered API docs and JET defaults #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 80 additions & 29 deletions src/SciMLTesting.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,9 +749,11 @@ Each tool runs if it is both available and enabled:
* `Aqua` + `aqua` ⇒ `Aqua.test_all(pkg; aqua_kwargs...)`,
* `JET` + `jet` ⇒ `JET.test_package(pkg; jet_kwargs...)`, where `jet_kwargs` is merged
over the standard `(; target_modules = (pkg,), mode = :typo)` rather than replacing
it, so a partial override keeps the standard setting for every key it omits. Passing
JET's deprecated `target_defined_modules` suppresses the `target_modules` default,
since the two configure the same thing.
it, so a partial override keeps the standard setting for every key it omits.
`target_defined_modules = true` asks JET to derive its target modules and replaces
the default target; `false` leaves the default `target_modules` filter in place.
Newer JET versions have removed this deprecated option, so callers should prefer
`target_modules` when configuring JET directly.
* `ExplicitImports` + `explicit_imports` ⇒ ExplicitImports' standard + public-API
checks (see [`run_explicit_imports`](@ref)).
* `api_docs` ⇒ the public-API documentation check (see [`run_api_docs`](@ref)): every
Expand Down Expand Up @@ -997,11 +999,13 @@ end
# Fill in the standard JET configuration for every key the caller did not mention. This
# must merge rather than replace: as a whole-NamedTuple default, any `jet_kwargs` at all
# dropped `mode = :typo` and silently reverted JET to its far more expensive `BasicPass`.
# `target_defined_modules` is JET's deprecated spelling of `target_modules`, so a caller
# passing it already owns that slot and must not also receive `target_modules`.
# `target_defined_modules` is a separate JET configuration: when true, JET derives its
# report target modules from the modules it analyzed. It therefore replaces the default
# target only in that case; `false` still leaves the standard target_modules filter active.
function _standard_jet_kwargs(pkg::Module, jet_kwargs)
nt = NamedTuple(jet_kwargs)
defaults = haskey(nt, :target_defined_modules) ? (; mode = :typo) :
defaults = get(nt, :target_defined_modules, false) === true && !haskey(nt, :target_modules) ?
(; mode = :typo) :
(; target_modules = (pkg,), mode = :typo)
return merge(defaults, nt)
end
Expand Down Expand Up @@ -1270,17 +1274,21 @@ function _doc_entry_name(line::AbstractString)
end

# The scope of one ```@autodocs``` block, mirroring what Documenter's `AutoDocsBlocks`
# expander does with the same settings. `nothing` means the setting could not be read
# from the text, so that dimension stays unrestricted. `modules` holds each `Modules`
# entry split into its dotted components.
# expander does with the same settings. An unknown scope is represented by `nothing` and
# never credits a name: the QA check must fail closed when it cannot prove that a name is
# rendered. `modules` holds each `Modules` entry split into its dotted components.
struct _AutoDocsBlock
modules::Union{Nothing, Vector{Vector{String}}}
pages::Union{Nothing, Vector{String}}
public::Bool
private::Bool
public::Union{Nothing, Bool}
private::Union{Nothing, Bool}
order::Union{Nothing, Vector{Symbol}}
unfiltered::Bool
end

_AutoDocsBlock() = _AutoDocsBlock(nothing, nothing, true, true)
_AutoDocsBlock() = _AutoDocsBlock(nothing, nothing, nothing, nothing, nothing, false)

const _AUTODOCS_DEFAULT_ORDER = Symbol[:module, :constant, :type, :function, :macro]

_normalize_doc_path(path::AbstractString) =
replace(String(path), '\\' => '/', "./" => "")
Expand Down Expand Up @@ -1325,12 +1333,19 @@ function _autodocs_pages(value)
return String[_normalize_doc_path(entry) for entry in entries]
end

function _autodocs_order(value)
entries = _autodocs_list(value)
entries === nothing && return nothing
all(entry -> entry isa QuoteNode && entry.value isa Symbol, entries) || return nothing
return Symbol[entry.value for entry in entries]
end

# The scope declared by a ```@autodocs``` block body (Julia `Key = value` assignments),
# or `nothing` for a block that renders nothing: Documenter refuses a block with no
# `Modules` (`'@autodocs' missing 'Modules = ...'`). Anything not decidable from the text
# — a body that does not parse, a non-literal `Modules`/`Pages` entry, or the
# `Filter`/`Order` settings, which narrow by evaluated value and by category — leaves
# that dimension unrestricted, so a block read only partly still credits everything.
# — a body that does not parse, a non-literal `Modules`/`Pages`/`Order`/`Public`/`Private`
# entry, or any `Filter` — is treated as rendering no names. This deliberately fails
# closed rather than allowing a partial parser to hide undocumented public API.
function _parse_autodocs_block(body::AbstractString)
parsed = try
Meta.parseall(body; filename = "autodocs")
Expand All @@ -1339,9 +1354,11 @@ function _parse_autodocs_block(body::AbstractString)
end
has_modules = false
modules = nothing
pages = nothing
pages = String[]
public = true
private = true
order = _AUTODOCS_DEFAULT_ORDER
unfiltered = true
for ex in parsed.args
(ex isa Expr && ex.head === :(=) && ex.args[1] isa Symbol) || continue
key = ex.args[1]::Symbol
Expand All @@ -1351,14 +1368,18 @@ function _parse_autodocs_block(body::AbstractString)
modules = _autodocs_modules(value)
elseif key === :Pages
pages = _autodocs_pages(value)
elseif key === :Order
order = _autodocs_order(value)
elseif key === :Public
public = value !== false
public = value isa Bool ? value : nothing
elseif key === :Private
private = value !== false
private = value isa Bool ? value : nothing
elseif key === :Filter
unfiltered = false
end
end
has_modules || return nothing
return _AutoDocsBlock(modules, pages, public, private)
return _AutoDocsBlock(modules, pages, public, private, order, unfiltered)
end

function _push_autodocs_block!(blocks::Vector{_AutoDocsBlock}, body::AbstractString)
Expand Down Expand Up @@ -1424,6 +1445,20 @@ function _doc_binding(pkg::Module, name::Symbol)
end
end

function _doc_category(binding)
startswith(String(binding.var), "@") && return :macro
value = try
getfield(binding.mod, binding.var)
catch
return nothing
end
value isa Function && return :function
value isa DataType && return :type
value isa UnionAll && return :type
value isa Module && return :module
return :constant
end

# Documenter decides `Public`/`Private` membership with `Base.ispublic` where it exists
# and `Base.isexported` before that (`DocSystem.APIStatus`), against the module whose
# docstring table the entry came from.
Expand All @@ -1440,7 +1475,7 @@ end
# — still lets a `Modules`-scoped block credit the name, while a `Pages`-scoped block,
# which needs a real path to match, does not.
function _docstring_entries(pkg::Module, name::Symbol)
entries = Tuple{Module, String}[]
entries = Tuple{Module, String, Union{Nothing, Symbol}}[]
binding = _doc_binding(pkg, name)
if binding !== nothing
owner = binding.mod
Expand All @@ -1453,30 +1488,47 @@ function _docstring_entries(pkg::Module, name::Symbol)
meta === nothing && continue
multidoc = get(meta, binding, nothing)
multidoc === nothing && continue
category = _doc_category(binding)
for docstr in values(multidoc.docs)
path = get(docstr.data, :path, nothing)
push!(entries, (mod, path === nothing ? "" : _normalize_doc_path(string(path))))
push!(
entries,
(mod, path === nothing ? "" : _normalize_doc_path(string(path)), category),
)
end
end
end
isempty(entries) && push!(entries, (pkg, ""))
isempty(entries) && push!(
entries, (pkg, "", binding === nothing ? nothing : _doc_category(binding))
)
return entries
end

# A module's dotted name as the manual would write it. Comparison is by name, so a
# `Modules` entry written through an import alias is not recognized as that module.
_module_components(mod::Module) = _strip_main(String[String(c) for c in fullname(mod)])

function _autodocs_renders(block::_AutoDocsBlock, entry::Tuple{Module, String}, name::Symbol)
(mod, path) = entry
(_doc_is_public(mod, name) ? block.public : block.private) || return false
function _autodocs_renders(
block::_AutoDocsBlock,
entry::Tuple{Module, String, Union{Nothing, Symbol}},
name::Symbol,
)
(mod, path, category) = entry
block.modules === nothing && return false
block.pages === nothing && return false
block.order === nothing && return false
block.unfiltered || return false
included = _doc_is_public(mod, name) ? block.public : block.private
included === true || return false
category === nothing && return false
category in block.order || return false
# Documenter collects docstrings from the listed modules themselves, so a submodule
# of a listed module is not covered unless it is listed too.
if block.modules !== nothing
components = _module_components(mod)
any(==(components), block.modules) || return false
end
if block.pages !== nothing
if !isempty(block.pages)
(!isempty(path) && any(page -> endswith(path, page), block.pages)) || return false
end
return true
Expand Down Expand Up @@ -1572,9 +1624,8 @@ Two checks, each its own nested `@testset`:
as literals, never evaluated, and follow Documenter: a block with no `Modules` renders
nothing, `Modules` entries name the modules whose docstrings are collected and do not
extend to their submodules, and `Pages` entries match a docstring's source file by
suffix. A setting that cannot be read as a literal, and the `Filter`/`Order` settings,
which narrow by evaluated value and by category, leave that dimension unrestricted, so
a block the check cannot fully read still credits everything. Names a block does not
suffix. A setting that cannot be read as a literal, or a `Filter`/`Order` setting that
the check cannot prove to include a name, credits no names. Names a block does not
cover, e.g. because the manual writes a module under an import alias, go in
`rendered_ignore`.

Expand Down
60 changes: 49 additions & 11 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,11 @@ module ScopedAutoDocsFixture
struct ScopedDocumentedType end
end

module RealJETFixture
export real_jet_identity
real_jet_identity(x) = x
end

# Reexports an undocumented external module and an undocumented external function
# next to an undocumented local name, so the docstrings check can be shown to exempt
# the module and only the module.
Expand Down Expand Up @@ -563,14 +568,27 @@ end
SciMLTesting, (; target_modules = (Base,))
).target_modules == (Base,)

# `target_defined_modules` is JET's deprecated spelling of `target_modules`, so it
# owns that slot: the default must not also inject `target_modules` alongside it.
deprecated = SciMLTesting._standard_jet_kwargs(
# `target_defined_modules` is a separate JET configuration. `false` does not
# replace the standard target_modules filter; `true` asks JET to derive its own
# target set and therefore replaces only the default target.
disabled = SciMLTesting._standard_jet_kwargs(
SciMLTesting, (; target_defined_modules = false)
)
@test disabled.mode === :typo
@test disabled.target_defined_modules === false
@test disabled.target_modules == (SciMLTesting,)

derived = SciMLTesting._standard_jet_kwargs(
SciMLTesting, (; target_defined_modules = true)
)
@test deprecated.mode === :typo
@test deprecated.target_defined_modules === true
@test !haskey(deprecated, :target_modules)
@test derived.mode === :typo
@test derived.target_defined_modules === true
@test !haskey(derived, :target_modules)

explicit = SciMLTesting._standard_jet_kwargs(
SciMLTesting, (; target_defined_modules = true, target_modules = (Base,))
)
@test explicit.target_modules == (Base,)

# Helpful errors when an enable flag is forced on but the module is unavailable.
@test_throws ArgumentError run_qa(
Expand All @@ -589,6 +607,18 @@ end
)
end

@testset "real JET target_defined_modules precedence" begin
# Newer supported JET versions removed the legacy keyword, so FakeJET checks
# forwarding of that key above while real JET checks the resulting target path.
effective = SciMLTesting._standard_jet_kwargs(
RealJETFixture, (; target_defined_modules = false)
)
@test effective.target_modules == (RealJETFixture,)
JET.test_package(
RealJETFixture; mode = effective.mode, target_modules = effective.target_modules
)
end

@testset "Aqua ambiguity subprocess resolves SciMLTesting dependencies" begin
original_load_path = copy(LOAD_PATH)
original_project = Base.active_project()
Expand Down Expand Up @@ -1242,12 +1272,14 @@ end
@test blocks[1].modules == [["MyPkg"], ["MyPkg", "Inner"]]
@test blocks[1].pages == ["systems/analysis_points.jl"]
@test blocks[1].public && !blocks[1].private
@test blocks[1].order == [:module, :constant, :type, :function, :macro]
@test blocks[1].unfiltered
@test blocks[2].modules == [["MyPkg"]]
@test blocks[2].pages === nothing
@test isempty(blocks[2].pages)
@test blocks[2].public && blocks[2].private

# Settings that cannot be read as literals leave the block unrestricted, keeping
# the pre-scoping "renders everything" behaviour rather than evaluating them.
# Settings that cannot be read as literals are retained for diagnostics but cannot
# satisfy the rendered check.
uroot = mktempdir()
usrc = joinpath(uroot, "src"); mkpath(usrc)
write(
Expand All @@ -1257,7 +1289,8 @@ end
(_, unreadable) = SciMLTesting._rendered_doc_names(usrc)
@test length(unreadable) == 1
@test unreadable[1].modules === nothing
@test unreadable[1].pages === nothing
@test isempty(unreadable[1].pages)
@test !unreadable[1].unfiltered

# Documenter renders nothing for a block with no `Modules`, so it is dropped
# instead of being read as unrestricted coverage.
Expand Down Expand Up @@ -1550,7 +1583,7 @@ end
"Modules = [Main.ScopedAutoDocsFixture]\n",
"Modules = [ScopedAutoDocsFixture]\nPages = [\"runtests.jl\"]\n",
"Modules = [ScopedAutoDocsFixture]\nPrivate = false\n",
"Modules = [ScopedAutoDocsFixture]\nFilter = t -> false\n",
"Modules = [ScopedAutoDocsFixture]\nOrder = [:function, :type]\n",
)
c = autodocs_counts(settings)
@test c[:fail] == 0 && c[:error] == 0 && c[:pass] == 1
Expand All @@ -1564,6 +1597,11 @@ end
"Modules = [Outer.Nested.ScopedAutoDocsFixture]\n",
"Modules = [ScopedAutoDocsFixture]\nPages = [\"systems/analysis_points.jl\"]\n",
"Modules = [ScopedAutoDocsFixture]\nPublic = false\n",
"Modules = [ScopedAutoDocsFixture]\nFilter = t -> false\n",
"Modules = [ScopedAutoDocsFixture]\nFilter = dynamic_filter\n",
"Modules = [ScopedAutoDocsFixture]\nOrder = [:type]\n",
"Modules = [ScopedAutoDocsFixture]\nOrder = dynamic_order\n",
"Modules = filter(_ -> false, [ScopedAutoDocsFixture])\n",
)
c = autodocs_counts(settings)
@test c[:fail] == 1
Expand Down
Loading