Overhaul documentation (#250)

* Small docs adjustments

* Split docs into one file per test

* Enable Documenter checks

* Add more docs for `ambiguities`

* Move parts of docstring of `test_persistent_tasks` to docs

* Add docs for `deps_compat`

* Move parts of docstring of `test_unbound_args` to docs

* Add docs for `project_extras`

* Add docs for `piracies`

* Add more docs

* Add changelgog

* Apply suggestions from code review

Co-authored-by: Yuto Horikawa <[email protected]>

* Apply suggestions from code review

* Update docs/src/unbound_args.md

* Update docs/src/unbound_args.md

---------

Co-authored-by: Yuto Horikawa <[email protected]>

Author: lgoettgens

CHANGELOG.md

@@ -7,11 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+
 ## [0.8.3] - 2023-11-29
 
-## Changed
+### Changed
 
 - `test_persistent_tasks` is now less noisy. ([#256](https://github.com/JuliaTesting/Aqua.jl/pull/256))
+- Completely overhauled the documentation. Every test now has its dedicated page. ([#250](https://github.com/JuliaTesting/Aqua.jl/pull/250))
 
 
 ## [0.8.2] - 2023-11-16

docs/make.jl

@@ -2,14 +2,26 @@ using Documenter, Aqua
 
 makedocs(;
     modules = [Aqua],
-    pages = ["Home" => "index.md"],
+    pages = [
+        "Home" => "index.md",
+        "Tests" => [
+            "test_all.md",
+            "ambiguities.md",
+            "unbound_args.md",
+            "exports.md",
+            "project_extras.md",
+            "stale_deps.md",
+            "deps_compat.md",
+            "piracies.md",
+            "persistent_tasks.md",
+        ],
+    ],
     sitename = "Aqua.jl",
     format = Documenter.HTML(;
         repolink = "https://github.com/JuliaTesting/Aqua.jl",
         assets = ["assets/favicon.ico"],
     ),
     authors = "Takafumi Arakaki",
-    warnonly = true,
 )
 
 deploydocs(; repo = "github.com/JuliaTesting/Aqua.jl", push_preview = true)

docs/src/ambiguities.md

@@ -0,0 +1,22 @@
+# Ambiguities
+
+Method ambiguities are cases where multiple methods are applicable to a given set of arguments, without having a most specific method.
+
+## Examples
+One easy example is the following:
+```@repl
+f(x::Int, y::Integer) = 1
+f(x::Integer, y::Int) = 2
+
+println(f(1, 2))
+```
+This will throw an `MethodError` because both methods are equally specific. The solution is to add a third method:
+```julia
+f(x::Int, y::Int) = ? # `?` is dependent on the use case, most times it will be `1` or `2`
+```
+
+## [Test function](@id test_ambiguities)
+
+```@docs
+Aqua.test_ambiguities
+```

docs/src/deps_compat.md

@@ -0,0 +1,14 @@
+# Compat entries
+
+In your `Project.toml` you can (and should) use compat entries to specify
+with which versions of Julia and your dependencies your package is compatible with.
+This is important to ease the installation and upgrade of your package for users,
+and to keep everything working in the case of breaking changes in Julia or your dependencies.
+
+For more details, see the [Pkg docs](https://julialang.github.io/Pkg.jl/v1/compatibility/).
+
+## [Test function](@id test_deps_compat)
+
+```@docs
+Aqua.test_deps_compat
+```

docs/src/exports.md

@@ -0,0 +1,7 @@
+# Undefined exports
+
+## [Test function](@id test_undefined_exports)
+
+```@docs
+Aqua.test_undefined_exports
+```

docs/src/index.md

@@ -1,5 +1,4 @@
-# Aqua.jl:
-## *A*uto *QU*ality *A*ssurance for Julia packages
+# Aqua.jl: *A*uto *QU*ality *A*ssurance for Julia packages
 
 Aqua.jl provides functions to run a few automatable checks for Julia packages:
 
@@ -84,20 +83,10 @@ end
 ```
 Note, that for all tests with no explicit options provided, the default options are used.
 
-For more details on the options, see the respective functions [below](@ref test_functions).
+For more details on the options, see the respective functions [here](@ref test_all).
 
-### Example uses
+## Examples
 The following is a small selection of packages that use Aqua.jl:
 - [GAP.jl](https://github.com/oscar-system/GAP.jl)
 - [Hecke.jl](https://github.com/thofma/Hecke.jl)
 - [Oscar.jl](https://github.com/oscar-system/Oscar.jl)
-
-## [Test functions](@id test_functions)
-```@docs
-Aqua.test_all
-```
-
-```@autodocs
-Modules = [Aqua]
-Filter = t -> (startswith(String(nameof(t)), "test_") && t != Aqua.test_all) || t == Aqua.find_persistent_tasks_deps
-```

docs/src/persistent_tasks.md

@@ -0,0 +1,65 @@
+# Persistent Tasks
+
+## Motivation
+
+Julia 1.10 and higher wait for all running `Task`s to finish
+before writing out the precompiled (cached) version of the package.
+One consequence is that a package that launches
+`Task`s in its `__init__` function may precompile successfully,
+but block precompilation of any packages that depend on it.
+
+## Example
+
+Let's create a dummy package, `PkgA`, that launches a persistent `Task`:
+
+```julia
+module PkgA
+const t = Ref{Any}()   # to prevent the Timer from being garbage-collected
+__init__() = t[] = Timer(0.1; interval=1)   # create a persistent `Timer` `Task`
+end
+```
+
+`PkgA` will precompile successfully, because `PkgA.__init__()` does not
+run when `PkgA` is precompiled. However,
+
+```julia
+module PkgB
+using PkgA
+end
+```
+
+fails to precompile: `using PkgA` runs `PkgA.__init__()`, which
+leaves the `Timer` `Task` running, and that causes precompilation
+of `PkgB` to hang.
+
+## How the test works
+
+This test works by launching a Julia process that tries to precompile a
+dummy package similar to `PkgB` above, modified to signal back to Aqua when
+`PkgA` has finished loading. The test fails if the gap between loading `PkgA`
+and finishing precompilation exceeds time `tmax`.
+
+## How to fix failing packages
+
+Often, the easiest fix is to modify the `__init__` function to check whether the
+Julia process is precompiling some other package; if so, don't launch the
+persistent `Task`s.
+
+```julia
+function __init__()
+    # Other setup code here
+    if ccall(:jl_generating_output, Cint, ()) == 0   # if we're not precompiling...
+        # launch persistent tasks here
+    end
+end
+```
+
+In more complex cases, you may need to set up independently-callable functions
+to launch the tasks and set conditions that allow them to cleanly exit.
+
+## [Test functions](@id test_persistent_tasks)
+
+```@docs
+Aqua.test_persistent_tasks
+Aqua.find_persistent_tasks_deps
+```

docs/src/piracies.md

@@ -0,0 +1,62 @@
+# Type piracy
+
+Type piracy is a term used to describe adding methods to a foreign function
+with only foreign arguments.
+This is considered bad practice because it can cause unexpected behavior
+when the function is called, in particular, it can change the behavior of
+one of your dependencies depending on if your package is loaded or not.
+This makes it hard to reason about the behavior of your code, and may
+introduce bugs that are hard to track down.
+
+See [Julia documentation](https://docs.julialang.org/en/v1/manual/style-guide/#Avoid-type-piracy) for more information about type piracy.
+
+## Examples
+
+Say that `PkgA` is foreign, and let's look at the different ways that `PkgB` extends its function `bar`.
+
+```julia
+module PkgA
+    struct C end
+    bar(x::C) = 42
+    bar(x::Vector) = 43
+end
+
+module PkgB 
+    import PkgA: bar, C
+    struct D end
+    bar(x::C) = 1
+    bar(xs::D...) = 2
+    bar(x::Vector{<:D}) = 3
+    bar(x::Vector{D}) = 4 # slightly bad (may cause invalidations)
+    bar(x::Union{C,D}) = 5 # slightly bad (a change in PkgA may turn it into piracy)
+    #                        (for example changing bar(x::C) = 1 to bar(x::Union{C,Int}) = 1)
+end
+```
+
+The following cases are enumerated by the return values in the example above:
+1. This is the worst case of type piracy. The value of `bar(C())` can be
+   either `1` or `42` and will depend on whether `PkgB` is loaded or not.
+2. This is also a bad case of type piracy. `bar()` throws a `MethodError` with
+   only `PkgA` available, and returns `2` with `PkgB` loaded. `PkgA` may add
+   a method for `bar()` that takes no arguments in the future, and then this
+   is equivalent to case 1.
+3. This is a moderately bad case of type piracy. `bar(Union{}[])` returns `3`
+   when `PkgB` is loaded, and `43` when `PkgB` is not loaded, although neither
+   of the occurring types are defined in `PkgB`. This case is not as bad as
+   cases 1 and 2, because it is only about behavior around `Union{}`, which has
+   no instances.
+4. Depending on ones understanding of type piracy, this could be considered piracy
+   as well. In particular, this may cause invalidations.
+5. This is a slightly bad case of type piracy. In the current form, `bar(C())`
+   returns `42` as the dispatch on `Union{C,D}` is less specific. However, a
+   future change in `PkgA` may change this behavior, e.g. by changing `bar(x::C)`
+   to `bar(x::Union{C,Int})` the call `bar(C())` would become ambiguous.
+
+!!! note
+    The test function below currently only checks for cases 1 and 2.
+
+## [Test function](@id test_piracies)
+
+```@docs
+Aqua.test_piracies
+```

docs/src/project_extras.md

@@ -0,0 +1,16 @@
+# Project.toml extras
+
+There are two different ways to specify test-only dependencies (see [the Pkg docs](https://julialang.github.io/Pkg.jl/v1/creating-packages/#Test-specific-dependencies)):
+1. Add the test-only dependencies to the `[extras]` section of your `Project.toml` file
+   and use a test target.
+2. Add the test-only dependencies to the `[deps]` section of your `test/Project.toml` file.
+   This is only available in Julia 1.2 and later.
+
+This test checks checks that, in case you use both methods, the set of test-only dependencies
+is the same in both ways.
+
+## [Test function](@id test_project_extras)
+
+```@docs
+Aqua.test_project_extras
+```

docs/src/stale_deps.md

@@ -0,0 +1,7 @@
+# Stale dependencies
+
+## [Test function](@id test_stale_deps)
+
+```@docs
+Aqua.test_stale_deps
+```

docs/src/test_all.md

@@ -0,0 +1,9 @@
+# [Test everything](@id test_all)
+
+This test runs most of the other tests in this module.
+The defaults should be fine for most packages.
+If you have a package that needs to customize the test, you can do so by providing appropriate keyword arguments to `Aqua.test_all()` (see below)
+
+```@docs
+Aqua.test_all
+```

docs/src/unbound_args.md

@@ -0,0 +1,42 @@
+# Unbound Type Parameters
+
+An unbound type parameter is a type parameter with a `where`,
+that does not occur in the signature of some dispatch of the method.
+
+## Examples
+
+The following methods each have `T` as an unbound type parameter:
+
+```@repl unbound
+f(x::Int) where {T} = do_something(x)
+g(x::T...) where {T} = println(T)
+```
+
+In the cases of `f` above, the unbound type parameter `T` is neither
+present in the signature of the methods nor as a bound of another type parameter.
+Here, the type parameter `T` can be removed without changing any semantics.
+
+For signatures with `Vararg` (cf. `g` above), the type parameter is unbound for the 
+zero-argument case (e.g. `g()`).
+
+```@repl unbound
+g(1.0, 2.0)
+g(1)
+g()
+```
+
+A possible fix would be to replace `g` by two methods.
+
+```@repl unbound2
+g() = println(Int)  # Defaults to `Int`
+g(x1::T, x2::T...) where {T} = println(T)
+g(1.0, 2.0)
+g(1)
+g()
+```
+
+## [Test function](@id test_unbound_args)
+
+```@docs
+Aqua.test_unbound_args
+```

src/Aqua.jl

@@ -27,7 +27,7 @@ include("persistent_tasks.jl")
 """
     test_all(testtarget::Module)
 
-Run following tests in isolated testset:
+Run the following tests:
 
 * [`test_ambiguities([testtarget, Base, Core])`](@ref test_ambiguities)
 * [`test_unbound_args(testtarget)`](@ref test_unbound_args)

src/ambiguities.jl

@@ -4,7 +4,7 @@
 
 Test that there is no method ambiguities in given package(s).  It
 calls `Test.detect_ambiguities` in a separated clean process to avoid
-false-positive.
+false-positives.
 
 # Keyword Arguments
 - `broken::Bool = false`: If true, it uses `@test_broken` instead of

src/exports.jl

@@ -12,9 +12,6 @@ function walkmodules(f, x::Module)
     end
 end
 
-"""
-    undefined_exports(m::Module) :: Vector{Symbol}
-"""
 function undefined_exports(m::Module)
     undefined = Symbol[]
     walkmodules(m) do x