From d457d0e87889aefe2093cd79ab4d1ee35d3101e7 Mon Sep 17 00:00:00 2001 From: Avasam Date: Tue, 20 Aug 2024 12:57:29 -0400 Subject: [PATCH 1/3] Type sequence checks in setuptools/dist.py --- newsfragments/4578.bugfix.rst | 1 + newsfragments/4578.feature.rst | 1 + setuptools/dist.py | 48 ++++++++++++++++++++++++---------- setuptools/tests/test_dist.py | 8 +++--- 4 files changed, 40 insertions(+), 18 deletions(-) create mode 100644 newsfragments/4578.bugfix.rst create mode 100644 newsfragments/4578.feature.rst diff --git a/newsfragments/4578.bugfix.rst b/newsfragments/4578.bugfix.rst new file mode 100644 index 0000000000..e9bde46269 --- /dev/null +++ b/newsfragments/4578.bugfix.rst @@ -0,0 +1 @@ +Fix a `TypeError` when a ``Distribution``'s old included attribute was a `tuple` -- by :user:`Avasam` diff --git a/newsfragments/4578.feature.rst b/newsfragments/4578.feature.rst new file mode 100644 index 0000000000..48f57edce3 --- /dev/null +++ b/newsfragments/4578.feature.rst @@ -0,0 +1 @@ +Made errors when parsing ``Distribution`` data more explicit about the expected type (``tuple[str, ...] | list[str]``) -- by :user:`Avasam` diff --git a/setuptools/dist.py b/setuptools/dist.py index 7c516fefb8..bb9a2a9951 100644 --- a/setuptools/dist.py +++ b/setuptools/dist.py @@ -8,7 +8,7 @@ import sys from glob import iglob from pathlib import Path -from typing import TYPE_CHECKING, MutableMapping +from typing import TYPE_CHECKING, List, MutableMapping, NoReturn, Tuple, Union, overload from more_itertools import partition, unique_everseen from packaging.markers import InvalidMarker, Marker @@ -21,6 +21,7 @@ command as _, # noqa: F401 # imported for side-effects ) from ._importlib import metadata +from ._reqs import _StrOrIter from .config import pyprojecttoml, setupcfg from .discovery import ConfigDiscovery from .monkey import get_unpatched @@ -36,9 +37,22 @@ from distutils.fancy_getopt import translate_longopt from distutils.util import strtobool +if TYPE_CHECKING: + from typing_extensions import TypeAlias + __all__ = ['Distribution'] sequence = tuple, list +""" +Supported iterable types that are known to be: +- ordered (which `set` isn't) +- not match a str (which `Sequence[str]` does) +- not imply a nested type (like `dict`) +for use with `isinstance`. +""" +_Sequence: TypeAlias = Union[Tuple[str, ...], List[str]] +# This is how stringifying _Sequence would look in Python 3.10 +_requence_type_repr = "tuple[str, ...] | list[str]" def check_importable(dist, attr, value): @@ -51,7 +65,7 @@ def check_importable(dist, attr, value): ) from e -def assert_string_list(dist, attr, value): +def assert_string_list(dist, attr: str, value: _Sequence) -> None: """Verify that value is a string list""" try: # verify that value is a list or tuple to exclude unordered @@ -61,7 +75,7 @@ def assert_string_list(dist, attr, value): assert ''.join(value) != value except (TypeError, ValueError, AttributeError, AssertionError) as e: raise DistutilsSetupError( - "%r must be a list of strings (got %r)" % (attr, value) + f"{attr!r} must be of type <{_requence_type_repr}> (got {value!r})" ) from e @@ -138,7 +152,11 @@ def invalid_unless_false(dist, attr, value): raise DistutilsSetupError(f"{attr} is invalid.") -def check_requirements(dist, attr, value): +@overload +def check_requirements(dist, attr: str, value: set | dict) -> NoReturn: ... +@overload +def check_requirements(dist, attr: str, value: _StrOrIter) -> None: ... +def check_requirements(dist, attr: str, value: _StrOrIter) -> None: """Verify that install_requires is a valid requirements list""" try: list(_reqs.parse(value)) @@ -146,10 +164,10 @@ def check_requirements(dist, attr, value): raise TypeError("Unordered types are not allowed") except (TypeError, ValueError) as error: tmpl = ( - "{attr!r} must be a string or list of strings " - "containing valid project/version requirement specifiers; {error}" + f"{attr!r} must be a string or iterable of strings " + f"containing valid project/version requirement specifiers; {error}" ) - raise DistutilsSetupError(tmpl.format(attr=attr, error=error)) from error + raise DistutilsSetupError(tmpl) from error def check_specifier(dist, attr, value): @@ -767,11 +785,11 @@ def has_contents_for(self, package): return False - def _exclude_misc(self, name, value): + def _exclude_misc(self, name: str, value: _Sequence) -> None: """Handle 'exclude()' for list/tuple attrs without a special handler""" if not isinstance(value, sequence): raise DistutilsSetupError( - "%s: setting must be a list or tuple (%r)" % (name, value) + f"{name}: setting must be of type <{_requence_type_repr}> (got {value!r})" ) try: old = getattr(self, name) @@ -784,11 +802,13 @@ def _exclude_misc(self, name, value): elif old: setattr(self, name, [item for item in old if item not in value]) - def _include_misc(self, name, value): + def _include_misc(self, name: str, value: _Sequence) -> None: """Handle 'include()' for list/tuple attrs without a special handler""" if not isinstance(value, sequence): - raise DistutilsSetupError("%s: setting must be a list (%r)" % (name, value)) + raise DistutilsSetupError( + f"{name}: setting must be of type <{_requence_type_repr}> (got {value!r})" + ) try: old = getattr(self, name) except AttributeError as e: @@ -801,7 +821,7 @@ def _include_misc(self, name, value): ) else: new = [item for item in value if item not in old] - setattr(self, name, old + new) + setattr(self, name, list(old) + new) def exclude(self, **attrs): """Remove items from distribution that are named in keyword arguments @@ -826,10 +846,10 @@ def exclude(self, **attrs): else: self._exclude_misc(k, v) - def _exclude_packages(self, packages): + def _exclude_packages(self, packages: _Sequence) -> None: if not isinstance(packages, sequence): raise DistutilsSetupError( - "packages: setting must be a list or tuple (%r)" % (packages,) + f"packages: setting must be of type <{_requence_type_repr}> (got {packages!r})" ) list(map(self.exclude_package, packages)) diff --git a/setuptools/tests/test_dist.py b/setuptools/tests/test_dist.py index 597785b519..fde0de99ac 100644 --- a/setuptools/tests/test_dist.py +++ b/setuptools/tests/test_dist.py @@ -118,8 +118,8 @@ def test_provides_extras_deterministic_order(): 'hello': '*.msg', }, ( - "\"values of 'package_data' dict\" " - "must be a list of strings (got '*.msg')" + "\"values of 'package_data' dict\" must be of type " + " (got '*.msg')" ), ), # Invalid value type (generators are single use) @@ -128,8 +128,8 @@ def test_provides_extras_deterministic_order(): 'hello': (x for x in "generator"), }, ( - "\"values of 'package_data' dict\" must be a list of strings " - "(got " + " (got Date: Tue, 27 Aug 2024 11:32:59 +0100 Subject: [PATCH 2/3] Use variable msg instead of tmpl in setuptools/dist --- setuptools/dist.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/setuptools/dist.py b/setuptools/dist.py index bb9a2a9951..b5d78aa37d 100644 --- a/setuptools/dist.py +++ b/setuptools/dist.py @@ -140,8 +140,7 @@ def _check_marker(marker): def assert_bool(dist, attr, value): """Verify that value is True, False, 0, or 1""" if bool(value) != value: - tmpl = "{attr!r} must be a boolean value (got {value!r})" - raise DistutilsSetupError(tmpl.format(attr=attr, value=value)) + raise DistutilsSetupError(f"{attr!r} must be a boolean value (got {value!r})") def invalid_unless_false(dist, attr, value): @@ -163,11 +162,11 @@ def check_requirements(dist, attr: str, value: _StrOrIter) -> None: if isinstance(value, (dict, set)): raise TypeError("Unordered types are not allowed") except (TypeError, ValueError) as error: - tmpl = ( + msg = ( f"{attr!r} must be a string or iterable of strings " f"containing valid project/version requirement specifiers; {error}" ) - raise DistutilsSetupError(tmpl) from error + raise DistutilsSetupError(msg) from error def check_specifier(dist, attr, value): @@ -175,8 +174,8 @@ def check_specifier(dist, attr, value): try: SpecifierSet(value) except (InvalidSpecifier, AttributeError) as error: - tmpl = "{attr!r} must be a string containing valid version specifiers; {error}" - raise DistutilsSetupError(tmpl.format(attr=attr, error=error)) from error + msg = f"{attr!r} must be a string containing valid version specifiers; {error}" + raise DistutilsSetupError(msg) from error def check_entry_points(dist, attr, value): From 000a413e2af9c271166cebe6909ad664907887f1 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Tue, 27 Aug 2024 11:49:33 +0100 Subject: [PATCH 3/3] Deprecate public access to setuptools.dist.sequence --- setuptools/dist.py | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/setuptools/dist.py b/setuptools/dist.py index b5d78aa37d..f22e3eea54 100644 --- a/setuptools/dist.py +++ b/setuptools/dist.py @@ -8,7 +8,16 @@ import sys from glob import iglob from pathlib import Path -from typing import TYPE_CHECKING, List, MutableMapping, NoReturn, Tuple, Union, overload +from typing import ( + TYPE_CHECKING, + Any, + List, + MutableMapping, + NoReturn, + Tuple, + Union, + overload, +) from more_itertools import partition, unique_everseen from packaging.markers import InvalidMarker, Marker @@ -42,8 +51,10 @@ __all__ = ['Distribution'] -sequence = tuple, list +_sequence = tuple, list """ +:meta private: + Supported iterable types that are known to be: - ordered (which `set` isn't) - not match a str (which `Sequence[str]` does) @@ -55,6 +66,17 @@ _requence_type_repr = "tuple[str, ...] | list[str]" +def __getattr__(name: str) -> Any: # pragma: no cover + if name == "sequence": + SetuptoolsDeprecationWarning.emit( + "`setuptools.dist.sequence` is an internal implementation detail.", + "Please define your own `sequence = tuple, list` instead.", + due_date=(2025, 8, 28), # Originally added on 2024-08-27 + ) + return _sequence + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + + def check_importable(dist, attr, value): try: ep = metadata.EntryPoint(value=value, name=None, group=None) @@ -70,7 +92,7 @@ def assert_string_list(dist, attr: str, value: _Sequence) -> None: try: # verify that value is a list or tuple to exclude unordered # or single-use iterables - assert isinstance(value, sequence) + assert isinstance(value, _sequence) # verify that elements of value are strings assert ''.join(value) != value except (TypeError, ValueError, AttributeError, AssertionError) as e: @@ -786,7 +808,7 @@ def has_contents_for(self, package): def _exclude_misc(self, name: str, value: _Sequence) -> None: """Handle 'exclude()' for list/tuple attrs without a special handler""" - if not isinstance(value, sequence): + if not isinstance(value, _sequence): raise DistutilsSetupError( f"{name}: setting must be of type <{_requence_type_repr}> (got {value!r})" ) @@ -794,7 +816,7 @@ def _exclude_misc(self, name: str, value: _Sequence) -> None: old = getattr(self, name) except AttributeError as e: raise DistutilsSetupError("%s: No such distribution setting" % name) from e - if old is not None and not isinstance(old, sequence): + if old is not None and not isinstance(old, _sequence): raise DistutilsSetupError( name + ": this setting cannot be changed via include/exclude" ) @@ -804,7 +826,7 @@ def _exclude_misc(self, name: str, value: _Sequence) -> None: def _include_misc(self, name: str, value: _Sequence) -> None: """Handle 'include()' for list/tuple attrs without a special handler""" - if not isinstance(value, sequence): + if not isinstance(value, _sequence): raise DistutilsSetupError( f"{name}: setting must be of type <{_requence_type_repr}> (got {value!r})" ) @@ -814,7 +836,7 @@ def _include_misc(self, name: str, value: _Sequence) -> None: raise DistutilsSetupError("%s: No such distribution setting" % name) from e if old is None: setattr(self, name, value) - elif not isinstance(old, sequence): + elif not isinstance(old, _sequence): raise DistutilsSetupError( name + ": this setting cannot be changed via include/exclude" ) @@ -846,7 +868,7 @@ def exclude(self, **attrs): self._exclude_misc(k, v) def _exclude_packages(self, packages: _Sequence) -> None: - if not isinstance(packages, sequence): + if not isinstance(packages, _sequence): raise DistutilsSetupError( f"packages: setting must be of type <{_requence_type_repr}> (got {packages!r})" )