Stop exiting early with --fail-on-warnings; add --exception-on-warning (#12743)

Co-authored-by: Jeremy Maitin-Shepard <[email protected]>

Author: AA-Turner

.github/workflows/builddoc.yml

@@ -44,4 +44,3 @@ jobs:
         --jobs=auto
         --show-traceback
         --fail-on-warning
-        --keep-going

CHANGES.rst

@@ -30,6 +30,14 @@ Features added
   output files.
 * #12474: Support type-dependent search result highlighting via CSS.
   Patch by Tim Hoffmann.
+* #12743: No longer exit on the first warning when
+  :option:`--fail-on-warning <sphinx-build --fail-on-warning>` is used.
+  Instead, exit with a non-zero status if any warnings were generated
+  during the build.
+  Patch by Adam Turner.
+* #12743: Add :option:`sphinx-build --exception-on-warning`,
+  to raise an exception when warnings are emitted during the build.
+  Patch by Adam Turner and Jeremy Maitin-Shepard.
 
 Bugs fixed
 ----------

doc/internals/contributing.rst

@@ -263,7 +263,7 @@ To build the documentation, run the following command:
 
 .. code-block:: shell
 
-   sphinx-build -M html ./doc ./build/sphinx --fail-on-warning --keep-going
+   sphinx-build -M html ./doc ./build/sphinx --fail-on-warning
 
 This will parse the Sphinx documentation's source files and generate HTML for
 you to preview in :file:`build/sphinx/html`.

doc/man/sphinx-build.rst

@@ -43,7 +43,7 @@ Options
       the source and output directories, before any other options are passed.
       For example::
 
-          sphinx-build -M html ./source ./build --fail-on-warning --keep-going
+          sphinx-build -M html ./source ./build --fail-on-warning
 
    The *make-mode* provides the same build functionality as
    a default :ref:`Makefile or Make.bat <makefile_options>`,
@@ -253,20 +253,35 @@ Options
 
 .. option:: -W, --fail-on-warning
 
-   Turn warnings into errors.  This means that the build stops at the first
-   warning and ``sphinx-build`` exits with exit status 1.
+   Turn warnings into errors.
+   This means that :program:`sphinx-build` exits with exit status 1
+   if any warnings are generated during the build.
 
    .. versionchanged:: 7.3
       Add ``--fail-on-warning`` long option.
+   .. versionchanged:: 8.1
+      :program:`sphinx-build` no longer exits on the first warning,
+      but instead runs the entire build and exits with exit status 1
+      if any warnings were generated.
+      This behaviour was previously enabled with :option:`--keep-going`.
 
 .. option:: --keep-going
 
-   Only applicable whilst using :option:`--fail-on-warning`,
-   which by default exits :program:`sphinx-build` on the first warning.
+   From Sphinx 8.1, :option:`!--keep-going` is always enabled.
+   Previously, it was only applicable whilst using :option:`--fail-on-warning`,
+   which by default exited :program:`sphinx-build` on the first warning.
    Using :option:`!--keep-going` runs :program:`!sphinx-build` to completion
    and exits with exit status 1 if errors are encountered.
 
    .. versionadded:: 1.8
+   .. versionchanged:: 8.1
+      :program:`sphinx-build` no longer exits on the first warning,
+      meaning that in effect :option:`!--fail-on-warning` is always enabled.
+      The option is retained for compatibility, but may be removed at some
+      later date.
+
+   .. xref RemovedInSphinx10Warning: deprecate this option in Sphinx 10
+                                     or no earlier than 2026-01-01.
 
 .. option:: -T, --show-traceback
 
@@ -287,6 +302,13 @@ Options
    .. versionchanged:: 7.3
       Add ``--pdb`` long option.
 
+.. option:: --exception-on-warning
+
+   Raise an exception when a warning is emitted during the build.
+   This can be useful in combination with :option:`--pdb` to debug warnings.
+
+   .. versionadded:: 8.1
+
 .. option:: -h, --help, --version
 
    Display usage summary or Sphinx version.

doc/usage/extensions/autodoc.rst

@@ -770,6 +770,10 @@ There are also config values that you can set:
    If ``False`` is given, autodoc forcedly suppresses the error if the imported
    module emits warnings.  By default, ``True``.
 
+   .. versionchanged:: 8.1
+      This option now has no effect as :option:`!--fail-on-warning`
+      no longer exits early.
+
 .. confval:: autodoc_inherit_docstrings
 
    This value controls the docstrings inheritance.

sphinx/application.py

@@ -41,6 +41,8 @@
 from sphinx.util.tags import Tags
 
 if TYPE_CHECKING:
+    from typing import Final
+
     from docutils import nodes
     from docutils.nodes import Element, Node
     from docutils.parsers import Parser
@@ -134,7 +136,7 @@ class Sphinx:
     :ivar outdir: Directory for storing build documents.
     """
 
-    warningiserror: bool
+    warningiserror: Final = False
     _warncount: int
 
     def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[str] | None,
@@ -144,7 +146,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st
                  freshenv: bool = False, warningiserror: bool = False,
                  tags: Sequence[str] = (),
                  verbosity: int = 0, parallel: int = 0, keep_going: bool = False,
-                 pdb: bool = False) -> None:
+                 pdb: bool = False, exception_on_warning: bool = False) -> None:
         """Initialize the Sphinx application.
 
         :param srcdir: The path to the source directory.
@@ -163,8 +165,9 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st
         :param verbosity: The verbosity level.
         :param parallel: The maximum number of parallel jobs to use
             when reading/writing documents.
-        :param keep_going: If true, continue processing when an error occurs.
+        :param keep_going: Unused.
         :param pdb: If true, enable the Python debugger on an exception.
+        :param exception_on_warning: If true, raise an exception on warnings.
         """
         self.phase = BuildPhase.INITIALIZATION
         self.verbosity = verbosity
@@ -203,12 +206,10 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st
         else:
             self._warning = warning
         self._warncount = 0
-        self.keep_going = warningiserror and keep_going
-        if self.keep_going:
-            self.warningiserror = False
-        else:
-            self.warningiserror = warningiserror
+        self.keep_going = bool(warningiserror)  # Unused
+        self._fail_on_warnings = bool(warningiserror)
         self.pdb = pdb
+        self._exception_on_warning = exception_on_warning
         logging.setup(self, self._status, self._warning)
 
         self.events = EventManager(self)
@@ -386,26 +387,31 @@ def build(self, force_all: bool = False, filenames: list[str] | None = None) ->
             self.events.emit('build-finished', err)
             raise
 
-        if self._warncount and self.keep_going:
-            self.statuscode = 1
-
-        status = (__('succeeded') if self.statuscode == 0
-                  else __('finished with problems'))
-        if self._warncount:
-            if self.warningiserror:
-                if self._warncount == 1:
-                    msg = __('build %s, %s warning (with warnings treated as errors).')
-                else:
-                    msg = __('build %s, %s warnings (with warnings treated as errors).')
+        if self._warncount == 0:
+            if self.statuscode != 0:
+                logger.info(bold(__('build finished with problems.')))
             else:
-                if self._warncount == 1:
-                    msg = __('build %s, %s warning.')
-                else:
-                    msg = __('build %s, %s warnings.')
-
-            logger.info(bold(msg), status, self._warncount)
+                logger.info(bold(__('build succeeded.')))
+        elif self._warncount == 1:
+            if self._fail_on_warnings:
+                self.statuscode = 1
+                msg = __('build finished with problems, 1 warning '
+                         '(with warnings treated as errors).')
+            elif self.statuscode != 0:
+                msg = __('build finished with problems, 1 warning.')
+            else:
+                msg = __('build succeeded, 1 warning.')
+            logger.info(bold(msg))
         else:
-            logger.info(bold(__('build %s.')), status)
+            if self._fail_on_warnings:
+                self.statuscode = 1
+                msg = __('build finished with problems, %s warnings '
+                         '(with warnings treated as errors).')
+            elif self.statuscode != 0:
+                msg = __('build finished with problems, %s warnings.')
+            else:
+                msg = __('build succeeded, %s warnings.')
+            logger.info(bold(msg), self._warncount)
 
         if self.statuscode == 0 and self.builder.epilog:
             logger.info('')

sphinx/builders/__init__.py

@@ -6,6 +6,7 @@
 import pickle
 import re
 import time
+from contextlib import nullcontext
 from os import path
 from typing import TYPE_CHECKING, Any, Literal, final
 
@@ -327,7 +328,7 @@ def build(
             logger.info(bold(__('building [%s]: ')) + summary, self.name)
 
         # while reading, collect all warnings from docutils
-        with logging.pending_warnings():
+        with nullcontext() if self.app._exception_on_warning else logging.pending_warnings():
             updated_docnames = set(self.read())
 
         doccount = len(updated_docnames)
@@ -627,7 +628,7 @@ def write(
             self._write_serial(sorted(docnames))
 
     def _write_serial(self, docnames: Sequence[str]) -> None:
-        with logging.pending_warnings():
+        with nullcontext() if self.app._exception_on_warning else logging.pending_warnings():
             for docname in status_iterator(docnames, __('writing output... '), "darkgreen",
                                            len(docnames), self.app.verbosity):
                 self.app.phase = BuildPhase.RESOLVING

sphinx/builders/linkcheck.py

@@ -106,7 +106,7 @@ def process_result(self, result: CheckResult) -> None:
         elif result.status == 'working':
             logger.info(darkgreen('ok        ') + result.uri + result.message)
         elif result.status == 'timeout':
-            if self.app.quiet or self.app.warningiserror:
+            if self.app.quiet:
                 logger.warning('timeout   ' + result.uri + result.message,
                                location=(result.docname, result.lineno))
             else:
@@ -115,7 +115,7 @@ def process_result(self, result: CheckResult) -> None:
                              result.uri + ': ' + result.message)
             self.timed_out_hyperlinks += 1
         elif result.status == 'broken':
-            if self.app.quiet or self.app.warningiserror:
+            if self.app.quiet:
                 logger.warning(__('broken link: %s (%s)'), result.uri, result.message,
                                location=(result.docname, result.lineno))
             else:

sphinx/cmd/build.py

@@ -201,12 +201,14 @@ def get_parser() -> argparse.ArgumentParser:
                        help=__('write warnings (and errors) to given file'))
     group.add_argument('--fail-on-warning', '-W', action='store_true', dest='warningiserror',
                        help=__('turn warnings into errors'))
-    group.add_argument('--keep-going', action='store_true', dest='keep_going',
-                       help=__("with --fail-on-warning, keep going when getting warnings"))
+    group.add_argument('--keep-going', action='store_true', help=argparse.SUPPRESS)
     group.add_argument('--show-traceback', '-T', action='store_true', dest='traceback',
                        help=__('show full traceback on exception'))
     group.add_argument('--pdb', '-P', action='store_true', dest='pdb',
                        help=__('run Pdb on exception'))
+    group.add_argument('--exception-on-warning', action='store_true',
+                       dest='exception_on_warning',
+                       help=__('raise an exception on warnings'))
 
     return parser
 
@@ -329,11 +331,16 @@ def build_main(argv: Sequence[str]) -> int:
     try:
         confdir = args.confdir or args.sourcedir
         with patch_docutils(confdir), docutils_namespace():
-            app = Sphinx(args.sourcedir, args.confdir, args.outputdir,
-                         args.doctreedir, args.builder, args.confoverrides, args.status,
-                         args.warning, args.freshenv, args.warningiserror,
-                         args.tags, args.verbosity, args.jobs, args.keep_going,
-                         args.pdb)
+            app = Sphinx(
+                srcdir=args.sourcedir, confdir=args.confdir,
+                outdir=args.outputdir, doctreedir=args.doctreedir,
+                buildername=args.builder, confoverrides=args.confoverrides,
+                status=args.status, warning=args.warning,
+                freshenv=args.freshenv, warningiserror=args.warningiserror,
+                tags=args.tags,
+                verbosity=args.verbosity, parallel=args.jobs, keep_going=False,
+                pdb=args.pdb, exception_on_warning=args.exception_on_warning,
+            )
             app.build(args.force_all, args.filenames)
             return app.statuscode
     except (Exception, KeyboardInterrupt) as exc:

sphinx/ext/autodoc/__init__.py

@@ -415,9 +415,10 @@ def import_object(self, raiseerror: bool = False) -> bool:
         """
         with mock(self.config.autodoc_mock_imports):
             try:
-                ret = import_object(self.modname, self.objpath, self.objtype,
-                                    attrgetter=self.get_attr,
-                                    warningiserror=self.config.autodoc_warningiserror)
+                ret = import_object(
+                    self.modname, self.objpath, self.objtype,
+                    attrgetter=self.get_attr,
+                )
                 self.module, self.parent, self.object_name, self.object = ret
                 if ismock(self.object):
                     self.object = undecorate(self.object)
@@ -1960,7 +1961,7 @@ def import_object(self, raiseerror: bool = False) -> bool:
             # annotation only instance variable (PEP-526)
             try:
                 with mock(self.config.autodoc_mock_imports):
-                    parent = import_module(self.modname, self.config.autodoc_warningiserror)
+                    parent = import_module(self.modname)
                     annotations = get_type_hints(parent, None,
                                                  self.config.autodoc_type_aliases,
                                                  include_extras=True)
@@ -2455,9 +2456,10 @@ def import_object(self, raiseerror: bool = False) -> bool:
         except ImportError as exc:
             try:
                 with mock(self.config.autodoc_mock_imports):
-                    ret = import_object(self.modname, self.objpath[:-1], 'class',
-                                        attrgetter=self.get_attr,  # type: ignore[attr-defined]
-                                        warningiserror=self.config.autodoc_warningiserror)
+                    ret = import_object(
+                        self.modname, self.objpath[:-1], 'class',
+                        attrgetter=self.get_attr,  # type: ignore[attr-defined]
+                    )
                     parent = ret[3]
                     if self.is_runtime_instance_attribute(parent):
                         self.object = self.RUNTIME_INSTANCE_ATTRIBUTE
@@ -2509,9 +2511,10 @@ def import_object(self, raiseerror: bool = False) -> bool:
             return super().import_object(raiseerror=True)  # type: ignore[misc]
         except ImportError as exc:
             try:
-                ret = import_object(self.modname, self.objpath[:-1], 'class',
-                                    attrgetter=self.get_attr,  # type: ignore[attr-defined]
-                                    warningiserror=self.config.autodoc_warningiserror)
+                ret = import_object(
+                    self.modname, self.objpath[:-1], 'class',
+                    attrgetter=self.get_attr,  # type: ignore[attr-defined]
+                )
                 parent = ret[3]
                 if self.is_uninitialized_instance_attribute(parent):
                     self.object = UNINITIALIZED_ATTR