Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_StrPath is dead; long live _StrPath #12690

Merged
merged 4 commits into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
--------------
Expand Down
3 changes: 2 additions & 1 deletion doc/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'),
Expand Down
10 changes: 5 additions & 5 deletions sphinx/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)') %
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions sphinx/builders/linkcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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')

Expand Down Expand Up @@ -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


Expand Down
3 changes: 2 additions & 1 deletion sphinx/environment/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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.
Expand Down
9 changes: 5 additions & 4 deletions sphinx/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand All @@ -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)
132 changes: 132 additions & 0 deletions sphinx/util/_pathlib.py
Original file line number Diff line number Diff line change
@@ -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. '
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this say Sphinx 9?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, done in 46ce447.

A

'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__())
7 changes: 5 additions & 2 deletions tests/test_builders/test_build_html.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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']


Expand All @@ -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']


Expand Down
4 changes: 2 additions & 2 deletions tests/test_builders/test_build_linkcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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] = []
Expand Down