Increase static typing strictness (#10530)

setup.cfg

@@ -26,11 +26,13 @@ python_version = 3.6
 disallow_incomplete_defs = True
 show_column_numbers = True
 show_error_context = True
+show_error_codes = true
 ignore_missing_imports = True
 follow_imports = skip
 check_untyped_defs = True
 warn_unused_ignores = True
 strict_optional = False
+no_implicit_optional = True
 
 [tool:pytest]
 filterwarnings =

sphinx/application.py

@@ -135,9 +135,6 @@ class Sphinx:
         self.phase = BuildPhase.INITIALIZATION
         self.verbosity = verbosity
         self.extensions: Dict[str, Extension] = {}
-        self.builder: Optional[Builder] = None
-        self.env: Optional[BuildEnvironment] = None
-        self.project: Optional[Project] = None
         self.registry = SphinxComponentRegistry()
 
         # validate provided directories
@@ -248,10 +245,16 @@ class Sphinx:
 
         # create the project
         self.project = Project(self.srcdir, self.config.source_suffix)
+
+        # set up the build environment
+        self.env = self._init_env(freshenv)
+
         # create the builder
         self.builder = self.create_builder(buildername)
-        # set up the build environment
-        self._init_env(freshenv)
+
+        # build environment post-initialisation, after creating the builder
+        self._post_init_env()
+
         # set up the builder
         self._init_builder()
 
@@ -283,20 +286,34 @@ class Sphinx:
             else:
                 logger.info(__('not available for built-in messages'))
 
-    def _init_env(self, freshenv: bool) -> None:
+    def _init_env(self, freshenv: bool) -> BuildEnvironment:
         filename = path.join(self.doctreedir, ENV_PICKLE_FILENAME)
         if freshenv or not os.path.exists(filename):
-            self.env = BuildEnvironment(self)
-            self.env.find_files(self.config, self.builder)
+            return self._create_fresh_env()
         else:
+            return self._load_existing_env(filename)
+
+    def _create_fresh_env(self) -> BuildEnvironment:
+        env = BuildEnvironment(self)
+        self._fresh_env_used = True
+        return env
+
+    def _load_existing_env(self, filename: str) -> BuildEnvironment:
         try:
             with progress_message(__('loading pickled environment')):
                 with open(filename, 'rb') as f:
-                        self.env = pickle.load(f)
-                        self.env.setup(self)
+                    env = pickle.load(f)
+                    env.setup(self)
+                    self._fresh_env_used = False
         except Exception as err:
             logger.info(__('failed: %s'), err)
-                self._init_env(freshenv=True)
+            env = self._create_fresh_env()
+        return env
+
+    def _post_init_env(self) -> None:
+        if self._fresh_env_used:
+            self.env.find_files(self.config, self.builder)
+        del self._fresh_env_used
 
     def preload_builder(self, name: str) -> None:
         self.registry.preload_builder(self, name)
@@ -306,9 +323,10 @@ class Sphinx:
             logger.info(__('No builder selected, using default: html'))
             name = 'html'
 
-        return self.registry.create_builder(self, name)
+        return self.registry.create_builder(self, name, self.env)
 
     def _init_builder(self) -> None:
+        if not hasattr(self.builder, "env"):
             self.builder.set_environment(self.env)
         self.builder.init()
         self.events.emit('builder-inited')
@@ -986,8 +1004,9 @@ class Sphinx:
             kwargs['defer'] = 'defer'
 
         self.registry.add_js_file(filename, priority=priority, **kwargs)
-        if hasattr(self.builder, 'add_js_file'):
-            self.builder.add_js_file(filename, priority=priority, **kwargs)  # type: ignore
+        if hasattr(self, 'builder') and hasattr(self.builder, 'add_js_file'):
+            self.builder.add_js_file(filename,  # type: ignore[attr-defined]
+                                     priority=priority, **kwargs)
 
     def add_css_file(self, filename: str, priority: int = 500, **kwargs: Any) -> None:
         """Register a stylesheet to include in the HTML output.
@@ -1047,8 +1066,9 @@ class Sphinx:
         """
         logger.debug('[app] adding stylesheet: %r', filename)
         self.registry.add_css_files(filename, priority=priority, **kwargs)
-        if hasattr(self.builder, 'add_css_file'):
-            self.builder.add_css_file(filename, priority=priority, **kwargs)  # type: ignore
+        if hasattr(self, 'builder') and hasattr(self.builder, 'add_css_file'):
+            self.builder.add_css_file(filename,  # type: ignore[attr-defined]
+                                      priority=priority, **kwargs)
 
     def add_stylesheet(self, filename: str, alternate: bool = False, title: str = None
                        ) -> None:

sphinx/builders/__init__.py

@@ -3,6 +3,7 @@
 import codecs
 import pickle
 import time
+import warnings
 from os import path
 from typing import (TYPE_CHECKING, Any, Dict, Iterable, List, Optional, Sequence, Set, Tuple,
                     Type, Union)
@@ -11,6 +12,7 @@ from docutils import nodes
 from docutils.nodes import Node
 
 from sphinx.config import Config
+from sphinx.deprecation import RemovedInSphinx70Warning
 from sphinx.environment import CONFIG_CHANGED_REASON, CONFIG_OK, BuildEnvironment
 from sphinx.environment.adapters.asset import ImageAdapter
 from sphinx.errors import SphinxError
@@ -75,7 +77,7 @@ class Builder:
     #: The builder supports data URIs or not.
     supported_data_uri_images = False
 
-    def __init__(self, app: "Sphinx") -> None:
+    def __init__(self, app: "Sphinx", env: BuildEnvironment = None) -> None:
         self.srcdir = app.srcdir
         self.confdir = app.confdir
         self.outdir = app.outdir
@@ -83,7 +85,14 @@ class Builder:
         ensuredir(self.doctreedir)
 
         self.app: Sphinx = app
-        self.env: Optional[BuildEnvironment] = None
+        if env is not None:
+            self.env: BuildEnvironment = env
+            self.env.set_versioning_method(self.versioning_method,
+                                           self.versioning_compare)
+        elif env is not Ellipsis:
+            # ... is passed by SphinxComponentRegistry.create_builder to not show two warnings.
+            warnings.warn("The 'env' argument to Builder will be required from Sphinx 7.",
+                          RemovedInSphinx70Warning, stacklevel=2)
         self.events: EventManager = app.events
         self.config: Config = app.config
         self.tags: Tags = app.tags
@@ -105,6 +114,9 @@ class Builder:
 
     def set_environment(self, env: BuildEnvironment) -> None:
         """Store BuildEnvironment object."""
+        warnings.warn("Builder.set_environment is deprecated, pass env to "
+                      "'Builder.__init__()' instead.",
+                      RemovedInSphinx70Warning, stacklevel=2)
         self.env = env
         self.env.set_versioning_method(self.versioning_method,
                                        self.versioning_compare)

sphinx/builders/html/__init__.py

@@ -26,6 +26,7 @@ from sphinx.builders import Builder
 from sphinx.config import ENUM, Config
 from sphinx.deprecation import RemovedInSphinx70Warning, deprecated_alias
 from sphinx.domains import Domain, Index, IndexEntry
+from sphinx.environment import BuildEnvironment
 from sphinx.environment.adapters.asset import ImageAdapter
 from sphinx.environment.adapters.indexentries import IndexEntries
 from sphinx.environment.adapters.toctree import TocTree
@@ -51,6 +52,17 @@ INVENTORY_FILENAME = 'objects.inv'
 logger = logging.getLogger(__name__)
 return_codes_re = re.compile('[\r\n]+')
 
+DOMAIN_INDEX_TYPE = Tuple[
+    # Index name (e.g. py-modindex)
+    str,
+    # Index class
+    Type[Index],
+    # list of (heading string, list of index entries) pairs.
+    List[Tuple[str, List[IndexEntry]]],
+    # whether sub-entries should start collapsed
+    bool
+]
+
 
 def get_stable_hash(obj: Any) -> str:
     """
@@ -197,10 +209,10 @@ class StandaloneHTMLBuilder(Builder):
     download_support = True  # enable download role
 
     imgpath: str = None
-    domain_indices: List[Tuple[str, Type[Index], List[Tuple[str, List[IndexEntry]]], bool]] = []  # NOQA
+    domain_indices: List[DOMAIN_INDEX_TYPE] = []
 
-    def __init__(self, app: Sphinx) -> None:
-        super().__init__(app)
+    def __init__(self, app: Sphinx, env: BuildEnvironment = None) -> None:
+        super().__init__(app, env)
 
         # CSS files
         self.css_files: List[Stylesheet] = []

sphinx/cmd/quickstart.py

@@ -7,11 +7,14 @@ import sys
 import time
 from collections import OrderedDict
 from os import path
-from typing import Any, Callable, Dict, List, Union
+from typing import TYPE_CHECKING, Any, Callable, Dict, List, Union
 
 # try to import readline, unix specific enhancement
 try:
     import readline
+    if TYPE_CHECKING and sys.platform == "win32":  # always false, for type checking
+        raise ImportError
+
     if readline.__doc__ and 'libedit' in readline.__doc__:
         readline.parse_and_bind("bind ^I rl_complete")
         USE_LIBEDIT = True

sphinx/registry.py

@@ -22,7 +22,7 @@ except ImportError:
 
 from sphinx.builders import Builder
 from sphinx.config import Config
-from sphinx.deprecation import RemovedInSphinx60Warning
+from sphinx.deprecation import RemovedInSphinx60Warning, RemovedInSphinx70Warning
 from sphinx.domains import Domain, Index, ObjType
 from sphinx.domains.std import GenericObject, Target
 from sphinx.environment import BuildEnvironment
@@ -153,11 +153,23 @@ class SphinxComponentRegistry:
 
             self.load_extension(app, entry_point.module)
 
-    def create_builder(self, app: "Sphinx", name: str) -> Builder:
+    def create_builder(self, app: "Sphinx", name: str,
+                       env: BuildEnvironment = None) -> Builder:
         if name not in self.builders:
             raise SphinxError(__('Builder name %s not registered') % name)
 
-        return self.builders[name](app)
+        try:
+            return self.builders[name](app, env)
+        except TypeError:
+            warnings.warn(
+                f"The custom builder {name} defines a custom __init__ method without the "
+                f"'env'argument. Report this bug to the developers of your custom builder, "
+                f"this is likely not a issue with Sphinx. The 'env' argument will be required "
+                f"from Sphinx 7.", RemovedInSphinx70Warning, stacklevel=2)
+            builder = self.builders[name](app, env=...)  # type: ignore[arg-type]
+            if env is not None:
+                builder.set_environment(env)
+            return builder
 
     def add_domain(self, domain: Type[Domain], override: bool = False) -> None:
         logger.debug('[app] adding domain: %r', domain)

sphinx/util/console.py

@@ -23,6 +23,9 @@ def terminal_safe(s: str) -> str:
 
 def get_terminal_width() -> int:
     """Borrowed from the py lib."""
+    if sys.platform == "win32":
+        # For static typing, as fcntl & termios never exist on Windows.
+        return int(os.environ.get('COLUMNS', 80)) - 1
     try:
         import fcntl
         import struct
@@ -32,7 +35,7 @@ def get_terminal_width() -> int:
         terminal_width = width
     except Exception:
         # FALLBACK
-        terminal_width = int(os.environ.get('COLUMNS', "80")) - 1
+        terminal_width = int(os.environ.get('COLUMNS', 80)) - 1
     return terminal_width
 
 

sphinx/util/logging.py

@@ -381,8 +381,8 @@ class WarningSuppressor(logging.Filter):
         super().__init__()
 
     def filter(self, record: logging.LogRecord) -> bool:
-        type = getattr(record, 'type', None)
-        subtype = getattr(record, 'subtype', None)
+        type = getattr(record, 'type', '')
+        subtype = getattr(record, 'subtype', '')
 
         try:
             suppress_warnings = self.app.config.suppress_warnings

sphinx/util/parallel.py

@@ -1,6 +1,7 @@
 """Parallel building utilities."""
 
 import os
+import sys
 import time
 import traceback
 from math import sqrt
@@ -16,6 +17,11 @@ from sphinx.util import logging
 
 logger = logging.getLogger(__name__)
 
+if sys.platform != "win32":
+    ForkProcess = multiprocessing.context.ForkProcess
+else:
+    # For static typing, as ForkProcess doesn't exist on Windows
+    ForkProcess = multiprocessing.process.BaseProcess
 
 # our parallel functionality only works for the forking Process
 parallel_available = multiprocessing and os.name == 'posix'
@@ -49,7 +55,7 @@ class ParallelTasks:
         # task arguments
         self._args: Dict[int, Optional[List[Any]]] = {}
         # list of subprocesses (both started and waiting)
-        self._procs: Dict[int, multiprocessing.context.ForkProcess] = {}
+        self._procs: Dict[int, ForkProcess] = {}
         # list of receiving pipe connections of running subprocesses
         self._precvs: Dict[int, Any] = {}
         # list of receiving pipe connections of waiting subprocesses

tests/test_application.py

@@ -1,15 +1,46 @@
 """Test the Sphinx class."""
 
+import shutil
+import sys
+from io import StringIO
+from pathlib import Path
 from unittest.mock import Mock
 
 import pytest
 from docutils import nodes
 
+import sphinx.application
 from sphinx.errors import ExtensionError
-from sphinx.testing.util import strip_escseq
+from sphinx.testing.path import path
+from sphinx.testing.util import SphinxTestApp, strip_escseq
 from sphinx.util import logging
 
 
+def test_instantiation(tmp_path_factory, rootdir: str, monkeypatch):
+    # Given
+    src_dir = tmp_path_factory.getbasetemp() / 'root'
+
+    # special support for sphinx/tests
+    if rootdir and not src_dir.exists():
+        shutil.copytree(Path(str(rootdir)) / 'test-root', src_dir)
+
+    monkeypatch.setattr('sphinx.application.abspath', lambda x: x)
+
+    syspath = sys.path[:]
+
+    # When
+    app_ = SphinxTestApp(
+        srcdir=path(src_dir),
+        status=StringIO(),
+        warning=StringIO()
+    )
+    sys.path[:] = syspath
+    app_.cleanup()
+
+    # Then
+    assert isinstance(app_, sphinx.application.Sphinx)
+
+
 def test_events(app, status, warning):
     def empty():
         pass

tests/test_environment.py

@@ -49,8 +49,7 @@ def test_images(app):
     app.build()
 
     tree = app.env.get_doctree('images')
-    htmlbuilder = StandaloneHTMLBuilder(app)
-    htmlbuilder.set_environment(app.env)
+    htmlbuilder = StandaloneHTMLBuilder(app, app.env)
     htmlbuilder.init()
     htmlbuilder.imgpath = 'dummy'
     htmlbuilder.post_process_images(tree)
@@ -59,8 +58,7 @@ def test_images(app):
     assert set(htmlbuilder.images.values()) == \
         {'img.png', 'img1.png', 'simg.png', 'svgimg.svg', 'img.foo.png'}
 
-    latexbuilder = LaTeXBuilder(app)
-    latexbuilder.set_environment(app.env)
+    latexbuilder = LaTeXBuilder(app, app.env)
     latexbuilder.init()
     latexbuilder.post_process_images(tree)
     assert set(latexbuilder.images.keys()) == \

tests/test_environment_toctree.py

@@ -156,7 +156,7 @@ def test_get_toc_for(app):
 @pytest.mark.test_params(shared_result='test_environment_toctree_basic')
 def test_get_toc_for_only(app):
     app.build()
-    builder = StandaloneHTMLBuilder(app)
+    builder = StandaloneHTMLBuilder(app, app.env)
     toctree = TocTree(app.env).get_toc_for('index', builder)
 
     assert_node(toctree,

tests/test_markup.py

@@ -106,8 +106,7 @@ def verify_re_html(app, parse):
 def verify_re_latex(app, parse):
     def verify(rst, latex_expected):
         document = parse(rst)
-        app.builder = LaTeXBuilder(app)
-        app.builder.set_environment(app.env)
+        app.builder = LaTeXBuilder(app, app.env)
         app.builder.init()
         theme = app.builder.themes.get('manual')
         latex_translator = ForgivingLaTeXTranslator(document, app.builder, theme)