Audit public reexports and inherited module docs

Project.toml

@@ -1,7 +1,7 @@
 name = "SciMLTesting"
 uuid = "09d9d899-5365-40a9-917a-5f67fddea283"
 authors = ["SciML"]
-version = "2.3.0"
+version = "2.4.0"
 
 [deps]
 Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"

README.md

@@ -50,9 +50,10 @@ test = ["Test", "SciMLTesting", ...]
 | `current_group(; env = "GROUP", default = "All")` | Read the test-group env var, defaulting to `"All"` (empty string also normalizes to the default). |
 | `activate_group_env(group_dir; parent, develop, instantiate, develop_sources)` | `Pkg.activate` a per-group `Project.toml`, `develop` the parent package(s) by path, backport `[sources]`, `instantiate`. |
 | `develop_sources!(group_dir; parent)` | On Julia < 1.11, `Pkg.develop` the env's `[sources]` path graph (recursively); a no-op on 1.11+. |
-| `run_qa(pkg; Aqua, JET, ExplicitImports, aqua, jet, explicit_imports, api_docs, aqua_broken, jet_broken, ei_broken, ...)` | Run the standard Aqua/JET/ExplicitImports QA body, plus the public-API documentation check. Aqua and ExplicitImports run by default; `using JET` registers JET via its weakdep extension and turns the JET check on. `api_docs` is **on by default** and runs `run_api_docs`. The `*_broken` kwargs mark known-broken findings as `@test_broken`. |
-| `run_api_docs(pkg; docstrings = true, rendered = true, docs_src, ignore, rendered_ignore, docstrings_broken, rendered_broken)` | Assert every exported/`public` name of `pkg` has a docstring and is rendered in a `@docs` block under `docs/src`. The shared replacement for per-repo `test/QA/public_api_docs.jl` files. |
+| `run_qa(pkg; Aqua, JET, ExplicitImports, aqua, jet, explicit_imports, api_docs, check_reexports, aqua_broken, jet_broken, ei_broken, ...)` | Run the standard Aqua/JET/ExplicitImports QA body, plus the public-API documentation check. Aqua and ExplicitImports run by default; `using JET` registers JET via its weakdep extension and turns the JET check on. `api_docs` is **on by default** and runs `run_api_docs`. `check_reexports` is opt-in and rejects public dependency bindings except `reexports_allow`. The `*_broken` kwargs mark known-broken findings as `@test_broken`. |
+| `run_api_docs(pkg; docstrings = true, rendered = true, docs_src, ignore, rendered_ignore, docstrings_broken, rendered_broken)` | Assert every exported/`public` name of `pkg` has a docstring and every locally rendered name appears in a `@docs` block under `docs/src`. Re-exported dependency modules inherit their defining package's rendering. |
 | `public_api_names(pkg)` | The sorted public API of `pkg` (exported names, plus `public` names on Julia ≥ 1.11), with the module's own name dropped. |
+| `public_reexports(pkg; allow = ())` | Public names imported from outside the package module hierarchy, plus aliases with reflectable external module ownership, excluding intentional names in `allow`. |
 | `detect_sublibrary_group(group, lib_dir; default_group = "Core")` | Map a `GROUP` value to a `(sublibrary, test_group)` pair for a monorepo. |
 
 All are documented with full docstrings; `?run_tests` etc. at the REPL.
@@ -330,18 +331,18 @@ run_qa(MyPackage)
 run_qa(MyPackage; api_docs_kwargs = (; rendered = false))
 
 # Standalone (outside run_qa), e.g. as its own QA file:
-run_api_docs(MyPackage)                     # every public name has a docstring and is rendered
+run_api_docs(MyPackage)                     # every public name is documented
 run_api_docs(MyPackage; rendered = false)   # docstrings only
 ```
 
   * **`docstrings`** (default `true`) — every name in `public_api_names(pkg)` has a
     docstring. A re-exported name documented in its defining package counts as
     documented (the check follows the binding), so you are not forced to redocument
     dependency re-exports.
-  * **`rendered`** (default `true`) — every public name appears in a
-    ` ```@docs ` block under `docs_src` (defaults to `<pkgroot>/docs/src`). A
-    ` ```@autodocs ` block satisfies it wholesale. Packages without a resolvable local
-    manual must explicitly pass `rendered = false`.
+  * **`rendered`** (default `true`) — every public name except re-exported dependency
+    modules appears in a ` ```@docs ` block under `docs_src` (defaults to
+    `<pkgroot>/docs/src`). A ` ```@autodocs ` block satisfies it wholesale. Packages
+    without a resolvable local manual must explicitly pass `rendered = false`.
   * **`ignore` / `rendered_ignore`** — names to exclude (e.g. an un-documentable
     re-export), with a comment pointing at the tracking issue.
   * **`docstrings_broken` / `rendered_broken`** — mark the check `@test_broken` for a
@@ -350,6 +351,21 @@ run_api_docs(MyPackage; rendered = false)   # docstrings only
 On the Julia 1.10 LTS `public_api_names` returns only the exported names (the `public`
 keyword is 1.11+), so no per-repo `if VERSION` guards are needed.
 
+### Public reexport audit (`check_reexports = true`)
+
+The reexport audit is opt-in so adopting this SciMLTesting release does not immediately
+break existing facade packages. Enable it while cleaning each package's public surface:
+
+```julia
+run_qa(MyPackage; check_reexports = true)
+run_qa(MyFacade; check_reexports = true, reexports_allow = (:solve, :remake))
+```
+
+It detects imported functions, types, modules, scalar constants, macros, and operators.
+It also detects local aliases when their values expose module ownership, such as
+ordinary functions, declared types, and modules. Keep `reexports_allow` limited to
+deliberate facade API; ordinary packages should use qualified dependency names instead.
+
 ### Known-broken findings (`aqua_broken`, `jet_broken`, `ei_broken`)
 
 When converting a hand-rolled `qa.jl` to `run_qa` would otherwise re-red a repo that

src/SciMLTesting.jl

@@ -56,7 +56,7 @@ import REPL
 
 export current_group, activate_group_env, run_qa, run_explicit_imports, detect_sublibrary_group,
     develop_sources!, run_tests, run_everything, read_test_groups,
-    with_clean_persistent_tasks_sources, run_api_docs, public_api_names
+    with_clean_persistent_tasks_sources, run_api_docs, public_api_names, public_reexports
 
 # Group names that are never sublibraries and never named functional groups: the
 # routing keywords `run_tests` and `detect_sublibrary_group` reserve.
@@ -609,10 +609,10 @@ end
 """
     run_qa(pkg; Aqua = Aqua, JET = ..., ExplicitImports = ExplicitImports,
            aqua = Aqua !== nothing, jet = JET !== nothing,
-           explicit_imports = true, api_docs = true,
+           explicit_imports = true, api_docs = true, check_reexports = false,
            clean_sources = true,
            aqua_kwargs = (;), jet_kwargs = (; target_modules = (pkg,), mode = :typo),
-           ei_kwargs = (;), api_docs_kwargs = (;),
+           ei_kwargs = (;), api_docs_kwargs = (;), reexports_allow = (),
            aqua_broken = (), jet_broken = false, ei_broken = (),
            testset = "Quality Assurance")
 
@@ -633,8 +633,10 @@ Each tool runs if it is both available and enabled:
   * `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
-    exported/`public` name has a docstring and is rendered in the manual unless
-    `api_docs_kwargs` explicitly sets `rendered = false`.
+    exported/`public` name has a docstring and every locally rendered name appears in
+    the manual unless `api_docs_kwargs` explicitly sets `rendered = false`.
+  * `check_reexports` ⇒ fail when the package publicly exposes a binding or alias
+    owned outside its module hierarchy, except names listed in `reexports_allow`.
 
 Enable-flag defaults: `aqua` defaults to `Aqua !== nothing` (on — Aqua is always
 available; pass `Aqua = nothing` or `aqua = false` to skip it), and `jet` defaults to
@@ -723,6 +725,10 @@ run_qa(MyPackage)
 using SciMLTesting, MyPackage
 run_qa(MyPackage)
 
+# Reject accidental public reexports. Facade packages can allow intentional names:
+run_qa(MyPackage; check_reexports = true)
+run_qa(MyFacade; check_reexports = true, reexports_allow = (:solve, :remake))
+
 # Explicitly threading the modules in still works (backward-compatible):
 using SciMLTesting, Aqua, JET
 run_qa(MyPackage; Aqua = Aqua, JET = JET, jet = true)
@@ -745,6 +751,8 @@ function run_qa(
         jet::Bool = JET !== nothing,
         explicit_imports::Bool = true,
         api_docs::Bool = true,
+        check_reexports::Bool = false,
+        reexports_allow = (),
         clean_sources::Bool = true,
         aqua_kwargs = (;),
         jet_kwargs = (; target_modules = (pkg,), mode = :typo),
@@ -793,6 +801,14 @@ function run_qa(
         end
         explicit_imports && run_explicit_imports(pkg, ExplicitImports; ei_kwargs, ei_broken)
         api_docs && run_api_docs(pkg; api_docs_kwargs...)
+        if check_reexports
+            reexported = public_reexports(pkg; allow = reexports_allow)
+            @testset "No unapproved public reexports" begin
+                @test isempty(reexported)
+                isempty(reexported) ||
+                    @info "run_qa: unapproved public reexports" pkg reexported
+            end
+        end
     end
     return nothing
 end
@@ -878,7 +894,7 @@ included when they are re-exported with `export`, since `names` reports them.
 
 ```julia
 julia> public_api_names(SciMLTesting)
-11-element Vector{Symbol}:
+13-element Vector{Symbol}:
  :activate_group_env
  ⋮
 ```
@@ -888,6 +904,57 @@ function public_api_names(pkg::Module)
     return sort!(api)
 end
 
+"""
+    public_reexports(pkg; allow = ()) -> Vector{Symbol}
+
+Return public names imported from, or aliased to API owned outside, `pkg`'s module
+hierarchy. This is an opt-in QA audit: facade packages may allow deliberate reexports,
+while ordinary packages should expose only their own API.
+
+`allow` is a collection of public names that are intentional reexports. Imported
+bindings are detected regardless of value kind. Local aliases are also detected when
+their values expose module ownership, including ordinary functions, declared types,
+and modules. Julia does not retain source-binding provenance for assigned scalar or
+enum values, callable objects, or constructed `Union` types; import those bindings
+directly when they must be covered by this audit.
+
+# Examples
+
+```julia
+public_reexports(MyPackage)
+public_reexports(MyFacade; allow = (:solve, :remake))
+```
+"""
+function public_reexports(pkg::Module; allow = ())
+    allowed = Set(Symbol.(allow))
+    reexported = filter(public_api_names(pkg)) do name
+        name in allowed && return false
+        isdefined(pkg, name) || return false
+        binding_owner = try
+            which(pkg, name)
+        catch
+            pkg
+        end
+        !_is_within_module(binding_owner, pkg) && return true
+        value_owner = try
+            parentmodule(getfield(pkg, name))
+        catch
+            pkg
+        end
+        return !_is_within_module(value_owner, pkg)
+    end
+    return sort!(reexported)
+end
+
+function _is_within_module(candidate::Module, root::Module)
+    while candidate !== root
+        parent = parentmodule(candidate)
+        parent === candidate && return false
+        candidate = parent
+    end
+    return true
+end
+
 # Whether `pkg.name` resolves to a docstring. Uses `Base.Docs.doc` on the *binding*
 # (not the object), so a re-exported name that is documented in the module that
 # actually defines it counts as documented here too (the binding follows the import
@@ -900,6 +967,15 @@ function _has_docstring(pkg::Module, name::Symbol)
     return !occursin("No documentation found", doc)
 end
 
+# A re-exported module inherits its defining package's module documentation.
+# Re-exported functions and types still require an explicit local API entry.
+function _requires_local_rendering(pkg::Module, name::Symbol)
+    isdefined(pkg, name) || return true
+    value = getfield(pkg, name)
+    value isa Module || return true
+    return _is_within_module(value, pkg)
+end
+
 # The bare name referenced by one line inside a ```@docs``` fenced block. A `@docs`
 # entry is a name, optionally module-qualified and/or with a call signature, e.g.
 # `foo`, `MyPkg.foo`, `foo(x::Int)`, `MyPkg.@mac`. Take the first whitespace token,
@@ -979,8 +1055,9 @@ Two checks, each its own nested `@testset`:
     not forced to redocument names it re-exports from a dependency.
   * **rendered** (`rendered = true`, on by default): every public API name appears inside a
     ```` ```@docs ```` block somewhere under `docs_src`, so it is rendered in the
-    manual. Packages without a resolvable local manual must explicitly set
-    `rendered = false`. If any
+    manual. Re-exported modules inherit the defining package's rendered module
+    documentation, while re-exported functions and types still require a local entry.
+    Packages without a resolvable local manual must explicitly set `rendered = false`. If any
     ```` ```@autodocs ```` block is present the check passes wholesale — `@autodocs`
     renders whole modules, so anything with a docstring is already rendered.
 
@@ -1042,8 +1119,12 @@ function run_api_docs(
         if rendered
             (rendered_names, autodocs) = _rendered_doc_names(docs_src)
             skip = Set{Symbol}(Symbol.(rendered_ignore))
-            unrendered = autodocs ? Symbol[] :
-                sort!(filter(n -> !(n in skip) && !(n in rendered_names), api))
+            unrendered = autodocs ? Symbol[] : sort!(
+                    filter(
+                        n -> _requires_local_rendering(pkg, n) && !(n in skip) && !(n in rendered_names),
+                        api,
+                    )
+                )
             @testset "public API is rendered in docs" begin
                 rendered_broken ? (@test_broken isempty(unrendered)) :
                     (@test isempty(unrendered))