From b5115375971c1483393f31a1f603c0c598704f9d Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Date: Fri, 26 Jul 2024 17:33:55 +0100 Subject: [PATCH] ``_StrPath`` is dead; long live ``_StrPath`` (#12690) --- CHANGES.rst | 9 +- doc/conf.py | 3 +- sphinx/application.py | 10 +- sphinx/builders/linkcheck.py | 6 +- sphinx/environment/__init__.py | 3 +- sphinx/project.py | 9 +- sphinx/util/_pathlib.py | 132 ++++++++++++++++++++ tests/test_builders/test_build_html.py | 7 +- tests/test_builders/test_build_linkcheck.py | 4 +- 9 files changed, 161 insertions(+), 22 deletions(-) create mode 100644 sphinx/util/_pathlib.py diff --git a/CHANGES.rst b/CHANGES.rst index 4f881ac175c..7c10a93ab08 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -68,10 +68,6 @@ Incompatible changes * #12096: Do not overwrite user-supplied files when copying assets unless forced with ``force=True``. Patch by Adam Turner. -* #12650: Remove support for string methods on :py:class:`~pathlib.Path` objects. - Use :py:func:`os.fspath` to convert :py:class:`~pathlib.Path` objects to strings, - or :py:class:`~pathlib.Path`'s methods to work with path objects. - Patch by Adam Turner. * #12646: Remove :py:func:`!sphinx.util.inspect.isNewType`. Patch by Adam Turner. * Remove the long-deprecated (since Sphinx 2) alias @@ -88,6 +84,11 @@ Deprecated to ``sphinx.ext.intersphinx.validate_intersphinx_mapping``. The old name will be removed in Sphinx 10. Patch by Adam Turner. +* #12650, #12686, #12690: Extend the deprecation for string methods on + :py:class:`~pathlib.Path` objects to Sphinx 9. + Use :py:func:`os.fspath` to convert :py:class:`~pathlib.Path` objects to strings, + or :py:class:`~pathlib.Path`'s methods to work with path objects. + Patch by Adam Turner. Features added -------------- diff --git a/doc/conf.py b/doc/conf.py index 73c904014e3..f784995453a 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -179,12 +179,12 @@ ('js:func', 'number'), ('js:func', 'string'), ('py:attr', 'srcline'), + ('py:class', '_StrPath'), # sphinx.environment.BuildEnvironment.doc2path ('py:class', 'Element'), # sphinx.domains.Domain ('py:class', 'Documenter'), # sphinx.application.Sphinx.add_autodocumenter ('py:class', 'IndexEntry'), # sphinx.domains.IndexEntry ('py:class', 'Node'), # sphinx.domains.Domain ('py:class', 'NullTranslations'), # gettext.NullTranslations - ('py:class', 'Path'), # sphinx.environment.BuildEnvironment.doc2path ('py:class', 'RoleFunction'), # sphinx.domains.Domain ('py:class', 'RSTState'), # sphinx.utils.parsing.nested_parse_to_nodes ('py:class', 'Theme'), # sphinx.application.TemplateBridge @@ -213,6 +213,7 @@ ('py:class', 'sphinx.roles.XRefRole'), ('py:class', 'sphinx.search.SearchLanguage'), ('py:class', 'sphinx.theming.Theme'), + ('py:class', 'sphinx.util._pathlib._StrPath'), # sphinx.project.Project.doc2path ('py:class', 'sphinxcontrib.websupport.errors.DocumentNotFoundError'), ('py:class', 'sphinxcontrib.websupport.errors.UserNotAuthorizedError'), ('py:exc', 'docutils.nodes.SkipNode'), diff --git a/sphinx/application.py b/sphinx/application.py index 70ec427c209..a1589fb230c 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -13,7 +13,6 @@ from collections.abc import Callable, Collection, Sequence # NoQA: TCH003 from io import StringIO from os import path -from pathlib import Path from typing import IO, TYPE_CHECKING, Any, Literal from docutils.nodes import TextElement # NoQA: TCH002 @@ -32,6 +31,7 @@ from sphinx.project import Project from sphinx.registry import SphinxComponentRegistry from sphinx.util import docutils, logging +from sphinx.util._pathlib import _StrPath from sphinx.util.build_phase import BuildPhase from sphinx.util.console import bold from sphinx.util.display import progress_message @@ -173,9 +173,9 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st self.registry = SphinxComponentRegistry() # validate provided directories - self.srcdir = Path(srcdir).resolve() - self.outdir = Path(outdir).resolve() - self.doctreedir = Path(doctreedir).resolve() + self.srcdir = _StrPath(srcdir).resolve() + self.outdir = _StrPath(outdir).resolve() + self.doctreedir = _StrPath(doctreedir).resolve() if not path.isdir(self.srcdir): raise ApplicationError(__('Cannot find source directory (%s)') % @@ -231,7 +231,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st self.confdir = self.srcdir self.config = Config({}, confoverrides or {}) else: - self.confdir = Path(confdir).resolve() + self.confdir = _StrPath(confdir).resolve() self.config = Config.read(self.confdir, confoverrides or {}, self.tags) # set up translation infrastructure diff --git a/sphinx/builders/linkcheck.py b/sphinx/builders/linkcheck.py index 4416007d5a6..5352b25936b 100644 --- a/sphinx/builders/linkcheck.py +++ b/sphinx/builders/linkcheck.py @@ -28,13 +28,13 @@ if TYPE_CHECKING: from collections.abc import Callable, Iterator - from pathlib import Path from typing import Any from requests import Response from sphinx.application import Sphinx from sphinx.config import Config + from sphinx.util._pathlib import _StrPath from sphinx.util.typing import ExtensionMetadata logger = logging.getLogger(__name__) @@ -150,7 +150,7 @@ def write_linkstat(self, data: dict[str, str | int]) -> None: self.json_outfile.write(json.dumps(data)) self.json_outfile.write('\n') - def write_entry(self, what: str, docname: str, filename: Path, line: int, + def write_entry(self, what: str, docname: str, filename: _StrPath, line: int, uri: str) -> None: self.txt_outfile.write(f'{filename}:{line}: [{what}] {uri}\n') @@ -226,7 +226,7 @@ def _add_uri(self, uri: str, node: nodes.Element) -> None: class Hyperlink(NamedTuple): uri: str docname: str - docpath: Path + docpath: _StrPath lineno: int diff --git a/sphinx/environment/__init__.py b/sphinx/environment/__init__.py index 4ef23cd8d45..20bfd86bc75 100644 --- a/sphinx/environment/__init__.py +++ b/sphinx/environment/__init__.py @@ -36,6 +36,7 @@ from sphinx.domains import Domain from sphinx.events import EventManager from sphinx.project import Project + from sphinx.util._pathlib import _StrPath logger = logging.getLogger(__name__) @@ -413,7 +414,7 @@ def path2doc(self, filename: str | os.PathLike[str]) -> str | None: """ return self.project.path2doc(filename) - def doc2path(self, docname: str, base: bool = True) -> Path: + def doc2path(self, docname: str, base: bool = True) -> _StrPath: """Return the filename for the document name. If *base* is True, return absolute path under self.srcdir. diff --git a/sphinx/project.py b/sphinx/project.py index 7cdd7f6a7c0..f1d77a351a0 100644 --- a/sphinx/project.py +++ b/sphinx/project.py @@ -9,6 +9,7 @@ from sphinx.locale import __ from sphinx.util import logging +from sphinx.util._pathlib import _StrPath from sphinx.util.matching import get_matching_files from sphinx.util.osutil import path_stabilize @@ -24,7 +25,7 @@ class Project: def __init__(self, srcdir: str | os.PathLike[str], source_suffix: Iterable[str]) -> None: #: Source directory. - self.srcdir = Path(srcdir) + self.srcdir = _StrPath(srcdir) #: source_suffix. Same as :confval:`source_suffix`. self.source_suffix = tuple(source_suffix) @@ -106,7 +107,7 @@ def path2doc(self, filename: str | os.PathLike[str]) -> str | None: # the file does not have a docname return None - def doc2path(self, docname: str, absolute: bool) -> Path: + def doc2path(self, docname: str, absolute: bool) -> _StrPath: """Return the filename for the document name. If *absolute* is True, return as an absolute path. @@ -119,5 +120,5 @@ def doc2path(self, docname: str, absolute: bool) -> Path: filename = Path(docname + self._first_source_suffix) if absolute: - return self.srcdir / filename - return filename + return _StrPath(self.srcdir / filename) + return _StrPath(filename) diff --git a/sphinx/util/_pathlib.py b/sphinx/util/_pathlib.py new file mode 100644 index 00000000000..e0bd60f49c3 --- /dev/null +++ b/sphinx/util/_pathlib.py @@ -0,0 +1,132 @@ +"""What follows is awful and will be gone in Sphinx 9. + +Instances of _StrPath should not be constructed except in Sphinx itself. +Consumers of Sphinx APIs should prefer using ``pathlib.Path`` objects +where possible. _StrPath objects can be treated as equivalent to ``Path``, +save that ``_StrPath.replace`` is overriden with ``str.replace``. + +To continue treating path-like objects as strings, use ``os.fspath``, +or explicit string coercion. + +In Sphinx 9, ``Path`` objects will be expected and returned in all instances +that ``_StrPath`` is currently used. +""" + +from __future__ import annotations + +import sys +import warnings +from pathlib import Path, PosixPath, PurePath, WindowsPath +from typing import Any + +from sphinx.deprecation import RemovedInSphinx90Warning + +_STR_METHODS = frozenset(str.__dict__) +_PATH_NAME = Path().__class__.__name__ + +_MSG = ( + 'Sphinx 8 will drop support for representing paths as strings. ' + 'Use "pathlib.Path" or "os.fspath" instead.' +) + +# https://docs.python.org/3/library/stdtypes.html#typesseq-common +# https://docs.python.org/3/library/stdtypes.html#string-methods + +if sys.platform == 'win32': + class _StrPath(WindowsPath): + def replace( # type: ignore[override] + self, old: str, new: str, count: int = -1, /, + ) -> str: + # replace exists in both Path and str; + # in Path it makes filesystem changes, so we use the safer str version + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return self.__str__().replace(old, new, count) # NoQA: PLC2801 + + def __getattr__(self, item: str) -> Any: + if item in _STR_METHODS: + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return getattr(self.__str__(), item) + msg = f'{_PATH_NAME!r} has no attribute {item!r}' + raise AttributeError(msg) + + def __add__(self, other: str) -> str: + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return self.__str__() + other + + def __bool__(self) -> bool: + if not self.__str__(): + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return False + return True + + def __contains__(self, item: str) -> bool: + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return item in self.__str__() + + def __eq__(self, other: object) -> bool: + if isinstance(other, PurePath): + return super().__eq__(other) + if isinstance(other, str): + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return self.__str__() == other + return NotImplemented + + def __hash__(self) -> int: + return super().__hash__() + + def __getitem__(self, item: int | slice) -> str: + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return self.__str__()[item] + + def __len__(self) -> int: + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return len(self.__str__()) +else: + class _StrPath(PosixPath): + def replace( # type: ignore[override] + self, old: str, new: str, count: int = -1, /, + ) -> str: + # replace exists in both Path and str; + # in Path it makes filesystem changes, so we use the safer str version + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return self.__str__().replace(old, new, count) # NoQA: PLC2801 + + def __getattr__(self, item: str) -> Any: + if item in _STR_METHODS: + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return getattr(self.__str__(), item) + msg = f'{_PATH_NAME!r} has no attribute {item!r}' + raise AttributeError(msg) + + def __add__(self, other: str) -> str: + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return self.__str__() + other + + def __bool__(self) -> bool: + if not self.__str__(): + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return False + return True + + def __contains__(self, item: str) -> bool: + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return item in self.__str__() + + def __eq__(self, other: object) -> bool: + if isinstance(other, PurePath): + return super().__eq__(other) + if isinstance(other, str): + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return self.__str__() == other + return NotImplemented + + def __hash__(self) -> int: + return super().__hash__() + + def __getitem__(self, item: int | slice) -> str: + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return self.__str__()[item] + + def __len__(self) -> int: + warnings.warn(_MSG, RemovedInSphinx90Warning, stacklevel=2) + return len(self.__str__()) diff --git a/tests/test_builders/test_build_html.py b/tests/test_builders/test_build_html.py index 28745ae9f13..45609e30554 100644 --- a/tests/test_builders/test_build_html.py +++ b/tests/test_builders/test_build_html.py @@ -10,6 +10,7 @@ import pytest from sphinx.builders.html import validate_html_extra_path, validate_html_static_path +from sphinx.deprecation import RemovedInSphinx90Warning from sphinx.errors import ConfigError from sphinx.util.console import strip_colors from sphinx.util.inventory import InventoryFile @@ -329,7 +330,8 @@ def test_validate_html_extra_path(app): app.outdir, # outdir app.outdir / '_static', # inside outdir ] - validate_html_extra_path(app, app.config) + with pytest.warns(RemovedInSphinx90Warning, match='Use "pathlib.Path" or "os.fspath" instead'): + validate_html_extra_path(app, app.config) assert app.config.html_extra_path == ['_static'] @@ -342,7 +344,8 @@ def test_validate_html_static_path(app): app.outdir, # outdir app.outdir / '_static', # inside outdir ] - validate_html_static_path(app, app.config) + with pytest.warns(RemovedInSphinx90Warning, match='Use "pathlib.Path" or "os.fspath" instead'): + validate_html_static_path(app, app.config) assert app.config.html_static_path == ['_static'] diff --git a/tests/test_builders/test_build_linkcheck.py b/tests/test_builders/test_build_linkcheck.py index 8577b46a45b..cd36981f74f 100644 --- a/tests/test_builders/test_build_linkcheck.py +++ b/tests/test_builders/test_build_linkcheck.py @@ -10,7 +10,6 @@ import wsgiref.handlers from base64 import b64encode from http.server import BaseHTTPRequestHandler -from pathlib import Path from queue import Queue from typing import TYPE_CHECKING from unittest import mock @@ -29,6 +28,7 @@ compile_linkcheck_allowed_redirects, ) from sphinx.util import requests +from sphinx.util._pathlib import _StrPath from sphinx.util.console import strip_colors from tests.utils import CERT_FILE, serve_application @@ -1061,7 +1061,7 @@ def test_connection_contention(get_adapter, app, capsys): wqueue: Queue[CheckRequest] = Queue() rqueue: Queue[CheckResult] = Queue() for _ in range(link_count): - wqueue.put(CheckRequest(0, Hyperlink(f"http://{address}", "test", Path("test.rst"), 1))) + wqueue.put(CheckRequest(0, Hyperlink(f"http://{address}", "test", _StrPath("test.rst"), 1))) begin = time.time() checked: list[CheckResult] = []