Replace strict with warnonly and make makedocs builds fail by d…

…efault (#2194)

CHANGELOG.md

@@ -11,6 +11,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Breaking
 
+* The `strict` keyword argument to `makedocs` has been removed and replaced with `warnonly`. `makedocs` now **fails builds by default** if any of the document checks fails (previously it only issued warnings by default). ([#2051], [#2194])
+
+  **For upgrading:** If you are running with `strict = true`, you can just drop the `strict` option.
+  If you are currently being strict about specific error classes, you can invert the list of error classes with `Documenter.except`.
+
+  If you were not setting the `strict` keyword, but your build is failing now, you should first endeavor to fix the errors that are causing the build to fail.
+  If that is not feasible, you can exclude specific error categories from failing the build (e.g. `warnonly = [:footnotes, :cross_references]`).
+  Finally, setting `warnonly = true` can be used to recover the old `strict = false` default behavior, turning all errors back into warnings.
+
 * The Markdown backend has been fully removed from the Documenter package, in favor of the external [DocumenterMarkdown package](https://github.com/JuliaDocs/DocumenterMarkdown.jl). This includes the removal of the exported `Deps` module. ([#1826])
 
   **For upgrading:** To keep using the Markdown backend, refer to the [DocumenterMarkdown package](https://github.com/JuliaDocs/DocumenterMarkdown.jl). That package might not immediately support the latest Documenter version, however.
@@ -1593,6 +1602,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#2018]: https://github.com/JuliaDocs/Documenter.jl/issues/2018
 [#2019]: https://github.com/JuliaDocs/Documenter.jl/issues/2019
 [#2027]: https://github.com/JuliaDocs/Documenter.jl/issues/2027
+[#2051]: https://github.com/JuliaDocs/Documenter.jl/issues/2051
 [#2066]: https://github.com/JuliaDocs/Documenter.jl/issues/2066
 [#2067]: https://github.com/JuliaDocs/Documenter.jl/issues/2067
 [#2070]: https://github.com/JuliaDocs/Documenter.jl/issues/2070
@@ -1613,6 +1623,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#2170]: https://github.com/JuliaDocs/Documenter.jl/issues/2170
 [#2181]: https://github.com/JuliaDocs/Documenter.jl/issues/2181
 [#2187]: https://github.com/JuliaDocs/Documenter.jl/issues/2187
+[#2194]: https://github.com/JuliaDocs/Documenter.jl/issues/2194
 [JuliaLang/julia#36953]: https://github.com/JuliaLang/julia/issues/36953
 [JuliaLang/julia#38054]: https://github.com/JuliaLang/julia/issues/38054
 [JuliaLang/julia#39841]: https://github.com/JuliaLang/julia/issues/39841

docs/make.jl

@@ -74,7 +74,7 @@ makedocs(
         "contributing.md",
         "release-notes.md",
     ],
-    strict = !("strict=false" in ARGS),
+    warnonly = ("strict=false" in ARGS),
     doctest = ("doctest=only" in ARGS) ? :only : true,
 )
 

docs/src/man/doctests.md

@@ -3,7 +3,8 @@
 Documenter will, by default, run `jldoctest` code blocks that it finds and makes sure that
 the actual output matches what's in the doctest. This can help to avoid documentation
 examples from becoming outdated, incorrect, or misleading. It is recommended that as many of
-a package's examples as possible be runnable by Documenter's doctest. Doctest failures during [`makedocs`](@ref) are printed as logging statements by default, but can be made fatal by passing `strict=true` or `strict=:doctest` to `makedocs`.
+a package's examples as possible be runnable by Documenter's doctest.
+Doctest failures during [`makedocs`](@ref) are fatal by default, but can be turned into just warnings by passing `warnonly=:doctest` to `makedocs`.
 
 
 This section of the manual outlines how to go about enabling doctests for code blocks in

src/Builder.jl

@@ -218,7 +218,7 @@ function Selectors.runner(::Type{Doctest}, doc::Documenter.Document)
         @info "Doctest: running doctests."
         DocTests.doctest(doc.blueprint, doc)
         num_errors = length(doc.internal.errors)
-        if (doc.user.doctest === :only || is_strict(doc.user.strict, :doctest)) && num_errors > 0
+        if (doc.user.doctest === :only || is_strict(doc, :doctest)) && num_errors > 0
             error("`makedocs` encountered $(num_errors > 1 ? "$(num_errors) doctest errors" : "a doctest error"). Terminating build")
         end
     else
@@ -256,12 +256,12 @@ end
 function Selectors.runner(::Type{RenderDocument}, doc::Documenter.Document)
     is_doctest_only(doc, "RenderDocument") && return
     # How many fatal errors
-    fatal_errors = filter(is_strict(doc.user.strict), doc.internal.errors)
+    fatal_errors = filter(is_strict(doc), doc.internal.errors)
     c = length(fatal_errors)
     if c > 0
-        error("`makedocs` encountered $(c > 1 ? "errors" : "an error") ("
+        error("`makedocs` encountered $(c > 1 ? "errors" : "an error") ["
         * join(Ref(":") .* string.(fatal_errors), ", ")
-        * "). Terminating build before rendering.")
+        * "] -- terminating build before rendering.")
     else
         @info "RenderDocument: rendering document."
         Documenter.render(doc)

src/documents.jl

@@ -300,7 +300,7 @@ struct User
     linkcheck_timeout::Real   # ..but only wait this many seconds for each one.
     checkdocs::Symbol         # Check objects missing from `@docs` blocks. `:none`, `:exports`, or `:all`.
     doctestfilters::Vector{Regex} # Filtering for doctests
-    strict::Union{Bool,Symbol,Vector{Symbol}} # Throw an exception when any warnings are encountered.
+    warnonly::Vector{Symbol}  # List of docerror groups that should only warn, rather than cause a build failure
     pages   :: Vector{Any}    # Ordering of document pages specified by the user.
     pagesonly :: Bool         # Discard any .md pages from processing that are not in .pages
     expandfirst::Vector{String} # List of pages that get "expanded" before others
@@ -371,7 +371,7 @@ function Document(plugins = nothing;
         linkcheck_timeout :: Real    = 10,
         checkdocs::Symbol            = :all,
         doctestfilters::Vector{Regex}= Regex[],
-        strict::Union{Bool,Symbol,Vector{Symbol}} = false,
+        warnonly :: Union{Bool,Symbol,Vector{Symbol}} = Symbol[],
         modules  :: ModVec = Module[],
         pages    :: Vector           = Any[],
         pagesonly:: Bool             = false,
@@ -386,7 +386,7 @@ function Document(plugins = nothing;
         others...
     )
 
-    check_strict_kw(strict)
+    warnonly = reduce_warnonly(warnonly) # convert warnonly to Symbol[]
     check_kwargs(others)
 
     if !isa(format, AbstractVector)
@@ -428,7 +428,7 @@ function Document(plugins = nothing;
         linkcheck_timeout,
         checkdocs,
         doctestfilters,
-        strict,
+        warnonly,
         pages,
         pagesonly,
         expandfirst,
@@ -631,6 +631,35 @@ function interpret_repo_and_remotes(; root, repo, remotes)
     return (makedocs_root_remote, remotes_checked)
 end
 
+# Converts the warnonly keyword argument to a Vector{Symbol}
+reduce_warnonly(warnonly::Bool) = reduce_warnonly(warnonly ? ERROR_NAMES : Symbol[])
+reduce_warnonly(s::Symbol) = reduce_warnonly(Symbol[s])
+function reduce_warnonly(warnonly)
+    warnonly = Symbol[s for s in warnonly]
+    extra_names = setdiff(warnonly, ERROR_NAMES)
+    if !isempty(extra_names)
+        throw(ArgumentError("""
+        Keyword argument `warnonly` given invalid values: $(extra_names)
+        Valid options are: $(ERROR_NAMES)
+        """))
+    end
+    return warnonly
+end
+
+"""
+    is_strict(::Document, val::Symbol) -> Bool
+
+Internal function to check if Documenter should throw an error or simply
+print a warning when hitting error condition.
+
+Single-argument `is_strict(strict)` provides a curried function.
+"""
+function is_strict(doc::Document, val::Symbol)
+    val in ERROR_NAMES || throw(ArgumentError("Invalid val in is_strict: $val"))
+    return !(val ∈ doc.user.warnonly)
+end
+is_strict(doc::Document) = Base.Fix1(is_strict, doc)
+
 function addremote!(remotes::Vector{RemoteRepository}, remoteref::RemoteRepository)
     for ref in remotes
         if ref.root == remoteref.root

src/makedocs.jl

@@ -67,7 +67,7 @@ produce the expected results. See the [Doctests](@ref) manual section for detail
 running doctests.
 
 Setting `doctest` to `:only` allows for doctesting without a full build. In this mode, most
-build stages are skipped and the `strict` keyword is ignored (a doctesting error will always
+build stages are skipped and the `warnonly` keyword is ignored (a doctesting error will always
 make `makedocs` throw an error in this mode).
 
 **`modules`** specifies a vector of modules that should be documented in `source`. If any
@@ -174,15 +174,15 @@ by setting `Draft = true` in an `@meta` block.
 defined in the `modules` keyword that have a docstring attached have the docstring also
 listed in the manual (e.g. there's a `@docs` block with that docstring). Possible values
 are `:all` (check all names; the default), `:exports` (check only exported names) and
-`:none` (no checks are performed). If `strict=true` (or `strict=:missing_docs` or
-`strict=[:missing_docs, ...]`) is also set then the build will fail if any missing
-docstrings are encountered.
+`:none` (no checks are performed).
+
+By default, if the document check detect any errors, it will fail the documentation build.
+This behavior can be relaxed with the `warnonly` keyword.
 
 **`linkcheck`** -- if set to `true` [`makedocs`](@ref) uses `curl` to check the status codes
 of external-pointing links, to make sure that they are up-to-date. The links and their
-status codes are printed to the standard output. If `strict` is also set to `true`
-(or `:linkcheck` or a `Vector` including `:linkcheck`) then the build will fail if there
-are any broken (400+ status code) links. Default: `false`.
+status codes are printed to the standard output. When enabled, any detected errors will fail
+the build, but this can be overridden by passing `:linkcheck` to `warnonly`. Default: `false`.
 
 **`linkcheck_ignore`** allows certain URLs to be ignored in `linkcheck`. The values should
 be a list of strings (which get matched exactly) or `Regex` objects. By default nothing is
@@ -191,11 +191,17 @@ ignored.
 **`linkcheck_timeout`** configures how long `curl` waits (in seconds) for a link request to
 return a response before giving up. The default is 10 seconds.
 
-**`strict`** -- if set to `true`, [`makedocs`](@ref) fails the build right before rendering
-if it encountered any errors with the document in the previous build phases. The keyword
-`strict` can also be set to a `Symbol` or `Vector{Symbol}` to specify which kind of error
-(or errors) should be fatal. Options are: $(join(Ref("`:") .* string.(ERROR_NAMES) .* Ref("`"), ", ", ", and ")).
-Use [`Documenter.except`](@ref) to explicitly set non-fatal errors.
+**`warnonly`** can be used to control whether the `makedocs` build fails with an error, or
+simply prints a warning if it detects any issues with the document. Additionally, a `Symbol`
+or a `Vector` of `Symbol`s can be passed to make Documenter warn for only those specified
+error classes (see also: [`Documenter.except`](@ref)). If set to `true`, the build should
+never fail due to document checks. The keyword defaults to `false`.
+
+Note that setting `warnonly = true` in general is not recommended, since it will make it very
+easy to miss Documentation build issues, and will lead to the deployment of broken manuals.
+The only case where you may want to consider passing `true` is when you are automatically
+deploying the documentation for a package release. In that case, `warnonly` should be set
+dynamically by checking the relevant environment variables set by the CI system.
 
 **`workdir`** determines the working directory where `@example` and `@repl` code blocks are
 executed. It can be either a path or the special value `:build` (default).
@@ -242,21 +248,21 @@ end
 """
     Documenter.except(errors...)
 
-Returns the list of all valid error classes that can be passed as the `strict` argument to
+Returns the list of all valid error classes that can be passed as the `warnonly` argument of
 [`makedocs`](@ref), except for the ones specified in the `errors` argument. Each error class
 must be a `Symbol` and passed as a separate argument.
 
-Can be used to easily disable the strict checking of specific error classes while making
-sure that all other error types still lead to the Documenter build failing. E.g. to stop
-Documenter failing on footnote and linkcheck errors, one can set `strict` as
+This can be used to enable strict error checking for only the listed error classes, while having
+other error types simply print a warning. E.g. to make Documenter fail the build only for
+footnote and linkcheck errors, one can set `warnonly` as
 
 ```julia
 makedocs(...,
-    strict = Documenter.except(:linkcheck, :footnote),
+    warnonly = Documenter.except(:linkcheck, :footnote),
 )
 ```
 
-Possible valid `Symbol` values are:
+The possible `Symbol` values that can be passed to the function are:
 $(join(Ref("`:") .* string.(ERROR_NAMES) .* Ref("`"), ", ", ", and ")).
 """
 function except(errors::Symbol...)

src/utilities/utilities.jl

@@ -38,9 +38,9 @@ macro docerror(doc, tag, msg, exs...)
         end
     end
     quote
-        let
-            push!($(doc).internal.errors, $(tag))
-            if is_strict($(doc).user.strict, $(tag))
+        let doc = $(doc)
+            push!(doc.internal.errors, $(tag))
+            if is_strict(doc, $(tag))
                 @error $(msg) $(exs...)
             else
                 @warn $(msg) $(exs...)
@@ -684,45 +684,6 @@ function get_sandbox_module!(meta, prefix, name = nothing)
     end
 end
 
-"""
-    is_strict(strict, val::Symbol) -> Bool
-
-Internal function to check if `strict` is strict about `val`, i.e.
-if errors of type `val` should be fatal, according
-to the setting `strict` (as a keyword to `makedocs`).
-
-Single-argument `is_strict(strict)` provides a curried function.
-"""
-is_strict
-
-is_strict(strict::Bool, ::Symbol) = strict
-is_strict(strict::Symbol, val::Symbol) = strict === val
-is_strict(strict::Vector{Symbol}, val::Symbol) = val ∈ strict
-is_strict(strict) = Base.Fix1(is_strict, strict)
-
-"""
-    check_strict_kw(strict) -> Nothing
-
-Internal function to check if `strict` is a valid value for
-the keyword argument `strict` to `makedocs.` Throws an
-`ArgumentError` if it is not valid.
-"""
-check_strict_kw
-
-check_strict_kw(::Bool) = nothing
-check_strict_kw(s::Symbol) = check_strict_kw(tuple(s))
-function check_strict_kw(strict)
-    extra_names = setdiff(strict, ERROR_NAMES)
-    if !isempty(extra_names)
-        throw(ArgumentError("""
-        Keyword argument `strict` given unknown values: $(extra_names)
-
-        Valid options are: $(ERROR_NAMES)
-        """))
-    end
-    return nothing
-end
-
 """
 Calls `git remote show \$(remotename)` to try to determine the main (development) branch
 of the remote repository. Returns `master` and prints a warning if it was unable to figure

test/doctests/doctests.jl

@@ -146,58 +146,58 @@ rfile(filename) = joinpath(@__DIR__, "stdouts", filename)
     # with strict = true to make sure that the doctests are indeed failing.
     #
     # Some tests are broken due to https://github.com/JuliaDocs/Documenter.jl/issues/974
-    run_makedocs(["working.md"]; strict=true) do result, success, backtrace, output
+    run_makedocs(["working.md"]) do result, success, backtrace, output
         @test success
         @test is_same_as_file(output, rfile("1.stdout"))
     end
 
-    run_makedocs(["broken.md"]; strict=true) do result, success, backtrace, output
+    run_makedocs(["broken.md"]) do result, success, backtrace, output
         @test !success
         @test is_same_as_file(output, rfile("2.stdout"))
     end
 
-    run_makedocs(["working.md", "fooworking.md"]; modules=[FooWorking], strict=true) do result, success, backtrace, output
+    run_makedocs(["working.md", "fooworking.md"]; modules=[FooWorking]) do result, success, backtrace, output
         @test success
         @test is_same_as_file(output, rfile("3.stdout"))
     end
 
-    run_makedocs(["working.md", "foobroken.md"]; modules=[FooBroken], strict=true) do result, success, backtrace, output
+    run_makedocs(["working.md", "foobroken.md"]; modules=[FooBroken]) do result, success, backtrace, output
         @test !success
         @test is_same_as_file(output, rfile("4.stdout"))
     end
 
-    run_makedocs(["broken.md", "fooworking.md"]; modules=[FooWorking], strict=true) do result, success, backtrace, output
+    run_makedocs(["broken.md", "fooworking.md"]; modules=[FooWorking]) do result, success, backtrace, output
         @test !success
         @test is_same_as_file(output, rfile("5.stdout"))
     end
 
-    for strict in (true, :doctest, [:doctest])
-        run_makedocs(["broken.md", "foobroken.md"]; modules=[FooBroken], strict=strict) do result, success, backtrace, output
+    for warnonly in (false, :autodocs_block, Documenter.except(:doctest))
+        run_makedocs(["broken.md", "foobroken.md"]; modules=[FooBroken], warnonly) do result, success, backtrace, output
             @test !success
             @test is_same_as_file(output, rfile("6.stdout"))
         end
     end
 
-    run_makedocs(["fooworking.md"]; modules=[FooWorking], strict=true) do result, success, backtrace, output
+    run_makedocs(["fooworking.md"]; modules=[FooWorking]) do result, success, backtrace, output
         @test success
         @test is_same_as_file(output, rfile("7.stdout"))
     end
 
-    run_makedocs(["foobroken.md"]; modules=[FooBroken], strict=true) do result, success, backtrace, output
+    run_makedocs(["foobroken.md"]; modules=[FooBroken]) do result, success, backtrace, output
         @test !success
         @test is_same_as_file(output, rfile("8.stdout"))
     end
 
     # Here we try the default (strict = false) -- output should say that doctest failed, but
     # success should still be true.
-    run_makedocs(["working.md"]) do result, success, backtrace, output
+    run_makedocs(["working.md"]; warnonly=true) do result, success, backtrace, output
         @test success
         @test is_same_as_file(output, rfile("11.stdout"))
     end
 
     # Three options that do not strictly check doctests, including testing the default
-    for strict_kw in ((; strict=false), NamedTuple(), (; strict=[:meta_block]))
-        run_makedocs(["broken.md"]; strict_kw...) do result, success, backtrace, output
+    for warnonly_kw in ((; warnonly=true), (; warnonly=Documenter.except(:meta_block)))
+        run_makedocs(["broken.md"]; warnonly_kw...) do result, success, backtrace, output
             @test success
             @test is_same_as_file(output, rfile("12.stdout"))
         end
@@ -224,8 +224,8 @@ rfile(filename) = joinpath(@__DIR__, "stdouts", filename)
         @test !success
         @test is_same_as_file(output, rfile("24.stdout"))
     end
-    # strict gets ignored with doctest = :only
-    run_makedocs(["broken.md"]; modules=[FooBroken], doctest = :only, strict=false) do result, success, backtrace, output
+    # warnonly gets ignored with doctest = :only
+    run_makedocs(["broken.md"]; modules=[FooBroken], doctest = :only, warnonly=true) do result, success, backtrace, output
         @test !success
         @test is_same_as_file(output, rfile("25.stdout"))
     end
@@ -243,7 +243,7 @@ rfile(filename) = joinpath(@__DIR__, "stdouts", filename)
     end
 
     # Tests for special REPL softscope
-    run_makedocs(["softscope.md"]) do result, success, backtrace, output
+    run_makedocs(["softscope.md"]; warnonly=true) do result, success, backtrace, output
         @test success
         @test is_same_as_file(output, rfile("41.stdout"))
     end