diff --git a/news/f14947e7-deea-4e17-bdc2-dd8dab2a1fa5.trivial.rst b/news/f14947e7-deea-4e17-bdc2-dd8dab2a1fa5.trivial.rst new file mode 100644 index 00000000000..001cec34342 --- /dev/null +++ b/news/f14947e7-deea-4e17-bdc2-dd8dab2a1fa5.trivial.rst @@ -0,0 +1,5 @@ +Convert numerous internal classes to dataclasses for readability and stricter +enforcement of immutability across the codebase. A conservative approach was +taken in selecting which classes to convert. Classes which did not convert +cleanly into a dataclass or were "too complex" (e.g. maintains interconnected +state) were left alone. diff --git a/src/pip/_internal/index/collector.py b/src/pip/_internal/index/collector.py index a22e3b91fcb..5f8fdee3d46 100644 --- a/src/pip/_internal/index/collector.py +++ b/src/pip/_internal/index/collector.py @@ -11,6 +11,7 @@ import os import urllib.parse import urllib.request +from dataclasses import dataclass from html.parser import HTMLParser from optparse import Values from typing import ( @@ -248,29 +249,22 @@ def parse_links(page: "IndexContent") -> Iterable[Link]: yield link +@dataclass(frozen=True) class IndexContent: - """Represents one response (or page), along with its URL""" + """Represents one response (or page), along with its URL. - def __init__( - self, - content: bytes, - content_type: str, - encoding: Optional[str], - url: str, - cache_link_parsing: bool = True, - ) -> None: - """ - :param encoding: the encoding to decode the given content. - :param url: the URL from which the HTML was downloaded. - :param cache_link_parsing: whether links parsed from this page's url - should be cached. PyPI index urls should - have this set to False, for example. - """ - self.content = content - self.content_type = content_type - self.encoding = encoding - self.url = url - self.cache_link_parsing = cache_link_parsing + :param encoding: the encoding to decode the given content. + :param url: the URL from which the HTML was downloaded. + :param cache_link_parsing: whether links parsed from this page's url + should be cached. PyPI index urls should + have this set to False, for example. + """ + + content: bytes + content_type: str + encoding: Optional[str] + url: str + cache_link_parsing: bool = True def __str__(self) -> str: return redact_auth_from_url(self.url) diff --git a/src/pip/_internal/index/package_finder.py b/src/pip/_internal/index/package_finder.py index b5b9679f246..98ee6315567 100644 --- a/src/pip/_internal/index/package_finder.py +++ b/src/pip/_internal/index/package_finder.py @@ -5,6 +5,7 @@ import itertools import logging import re +from dataclasses import dataclass from typing import TYPE_CHECKING, FrozenSet, Iterable, List, Optional, Set, Tuple, Union from pip._vendor.packaging import specifiers @@ -322,22 +323,15 @@ def filter_unallowed_hashes( return filtered +@dataclass class CandidatePreferences: """ Encapsulates some of the preferences for filtering and sorting InstallationCandidate objects. """ - def __init__( - self, - prefer_binary: bool = False, - allow_all_prereleases: bool = False, - ) -> None: - """ - :param allow_all_prereleases: Whether to allow all pre-releases. - """ - self.allow_all_prereleases = allow_all_prereleases - self.prefer_binary = prefer_binary + prefer_binary: bool = False + allow_all_prereleases: bool = False class BestCandidateResult: diff --git a/src/pip/_internal/locations/_sysconfig.py b/src/pip/_internal/locations/_sysconfig.py index 97aef1f1ac2..ca860ea562c 100644 --- a/src/pip/_internal/locations/_sysconfig.py +++ b/src/pip/_internal/locations/_sysconfig.py @@ -192,9 +192,10 @@ def get_scheme( data=paths["data"], ) if root is not None: + converted_keys = {} for key in SCHEME_KEYS: - value = change_root(root, getattr(scheme, key)) - setattr(scheme, key, value) + converted_keys[key] = change_root(root, getattr(scheme, key)) + scheme = Scheme(**converted_keys) return scheme diff --git a/src/pip/_internal/models/candidate.py b/src/pip/_internal/models/candidate.py index b12ffeeac4a..f27f283154a 100644 --- a/src/pip/_internal/models/candidate.py +++ b/src/pip/_internal/models/candidate.py @@ -1,28 +1,25 @@ +from dataclasses import dataclass + +from pip._vendor.packaging.version import Version from pip._vendor.packaging.version import parse as parse_version from pip._internal.models.link import Link -from pip._internal.utils.models import KeyBasedCompareMixin -class InstallationCandidate(KeyBasedCompareMixin): +@dataclass(frozen=True) +class InstallationCandidate: """Represents a potential "candidate" for installation.""" __slots__ = ["name", "version", "link"] + name: str + version: Version + link: Link + def __init__(self, name: str, version: str, link: Link) -> None: - self.name = name - self.version = parse_version(version) - self.link = link - - super().__init__( - key=(self.name, self.version, self.link), - defining_class=InstallationCandidate, - ) - - def __repr__(self) -> str: - return ( - f"" - ) + object.__setattr__(self, "name", name) + object.__setattr__(self, "version", parse_version(version)) + object.__setattr__(self, "link", link) def __str__(self) -> str: return f"{self.name!r} candidate (version {self.version} at {self.link})" diff --git a/src/pip/_internal/models/direct_url.py b/src/pip/_internal/models/direct_url.py index 1063952489a..fc5ec8d4aa9 100644 --- a/src/pip/_internal/models/direct_url.py +++ b/src/pip/_internal/models/direct_url.py @@ -3,7 +3,8 @@ import json import re import urllib.parse -from typing import Any, Dict, Iterable, Optional, Type, TypeVar, Union +from dataclasses import dataclass +from typing import Any, ClassVar, Dict, Iterable, Optional, Type, TypeVar, Union __all__ = [ "DirectUrl", @@ -65,18 +66,13 @@ def _filter_none(**kwargs: Any) -> Dict[str, Any]: return {k: v for k, v in kwargs.items() if v is not None} +@dataclass class VcsInfo: - name = "vcs_info" + name: ClassVar = "vcs_info" - def __init__( - self, - vcs: str, - commit_id: str, - requested_revision: Optional[str] = None, - ) -> None: - self.vcs = vcs - self.requested_revision = requested_revision - self.commit_id = commit_id + vcs: str + commit_id: str + requested_revision: Optional[str] = None @classmethod def _from_dict(cls, d: Optional[Dict[str, Any]]) -> Optional["VcsInfo"]: @@ -140,14 +136,11 @@ def _to_dict(self) -> Dict[str, Any]: return _filter_none(hash=self.hash, hashes=self.hashes) +@dataclass class DirInfo: - name = "dir_info" + name: ClassVar = "dir_info" - def __init__( - self, - editable: bool = False, - ) -> None: - self.editable = editable + editable: bool = False @classmethod def _from_dict(cls, d: Optional[Dict[str, Any]]) -> Optional["DirInfo"]: @@ -162,16 +155,11 @@ def _to_dict(self) -> Dict[str, Any]: InfoType = Union[ArchiveInfo, DirInfo, VcsInfo] +@dataclass class DirectUrl: - def __init__( - self, - url: str, - info: InfoType, - subdirectory: Optional[str] = None, - ) -> None: - self.url = url - self.info = info - self.subdirectory = subdirectory + url: str + info: InfoType + subdirectory: Optional[str] = None def _remove_auth_from_netloc(self, netloc: str) -> str: if "@" not in netloc: diff --git a/src/pip/_internal/models/link.py b/src/pip/_internal/models/link.py index 73041b864c3..2f41f2f6a09 100644 --- a/src/pip/_internal/models/link.py +++ b/src/pip/_internal/models/link.py @@ -27,7 +27,6 @@ split_auth_from_netloc, splitext, ) -from pip._internal.utils.models import KeyBasedCompareMixin from pip._internal.utils.urls import path_to_url, url_to_path if TYPE_CHECKING: @@ -179,7 +178,8 @@ def _ensure_quoted_url(url: str) -> str: return urllib.parse.urlunparse(result._replace(path=path)) -class Link(KeyBasedCompareMixin): +@functools.total_ordering +class Link: """Represents a parsed link from a Package Index's simple URL""" __slots__ = [ @@ -254,8 +254,6 @@ def __init__( self.yanked_reason = yanked_reason self.metadata_file_data = metadata_file_data - super().__init__(key=url, defining_class=Link) - self.cache_link_parsing = cache_link_parsing self.egg_fragment = self._egg_fragment() @@ -375,6 +373,19 @@ def __str__(self) -> str: def __repr__(self) -> str: return f"" + def __hash__(self) -> int: + return hash(self.url) + + def __eq__(self, other: Any) -> bool: + if not isinstance(other, Link): + return NotImplemented + return self.url == other.url + + def __lt__(self, other: Any) -> bool: + if not isinstance(other, Link): + return NotImplemented + return self.url < other.url + @property def url(self) -> str: return self._url diff --git a/src/pip/_internal/models/scheme.py b/src/pip/_internal/models/scheme.py index 9d804a67371..06a9a550e34 100644 --- a/src/pip/_internal/models/scheme.py +++ b/src/pip/_internal/models/scheme.py @@ -5,9 +5,12 @@ https://docs.python.org/3/install/index.html#alternate-installation. """ +from dataclasses import dataclass + SCHEME_KEYS = ["platlib", "purelib", "headers", "scripts", "data"] +@dataclass(frozen=True) class Scheme: """A Scheme holds paths which are used as the base directories for artifacts associated with a Python package. @@ -15,16 +18,8 @@ class Scheme: __slots__ = SCHEME_KEYS - def __init__( - self, - platlib: str, - purelib: str, - headers: str, - scripts: str, - data: str, - ) -> None: - self.platlib = platlib - self.purelib = purelib - self.headers = headers - self.scripts = scripts - self.data = data + platlib: str + purelib: str + headers: str + scripts: str + data: str diff --git a/src/pip/_internal/models/search_scope.py b/src/pip/_internal/models/search_scope.py index a96ddcb4096..ee7bc86229a 100644 --- a/src/pip/_internal/models/search_scope.py +++ b/src/pip/_internal/models/search_scope.py @@ -3,6 +3,7 @@ import os import posixpath import urllib.parse +from dataclasses import dataclass from typing import List from pip._vendor.packaging.utils import canonicalize_name @@ -14,6 +15,7 @@ logger = logging.getLogger(__name__) +@dataclass(frozen=True) class SearchScope: """ Encapsulates the locations that pip is configured to search. @@ -21,6 +23,10 @@ class SearchScope: __slots__ = ["find_links", "index_urls", "no_index"] + find_links: List[str] + index_urls: List[str] + no_index: bool + @classmethod def create( cls, @@ -63,16 +69,6 @@ def create( no_index=no_index, ) - def __init__( - self, - find_links: List[str], - index_urls: List[str], - no_index: bool, - ) -> None: - self.find_links = find_links - self.index_urls = index_urls - self.no_index = no_index - def get_formatted_locations(self) -> str: lines = [] redacted_index_urls = [] diff --git a/src/pip/_internal/models/selection_prefs.py b/src/pip/_internal/models/selection_prefs.py index 977bc4caa75..e9b50aa5175 100644 --- a/src/pip/_internal/models/selection_prefs.py +++ b/src/pip/_internal/models/selection_prefs.py @@ -3,6 +3,8 @@ from pip._internal.models.format_control import FormatControl +# TODO: This needs Python 3.10's improved slots support for dataclasses +# to be converted into a dataclass. class SelectionPreferences: """ Encapsulates the candidate selection preferences for downloading diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 956717d1e52..e6aa3447200 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -7,6 +7,7 @@ import mimetypes import os import shutil +from dataclasses import dataclass from pathlib import Path from typing import Dict, Iterable, List, Optional @@ -80,13 +81,14 @@ def unpack_vcs_link(link: Link, location: str, verbosity: int) -> None: vcs_backend.unpack(location, url=hide_url(link.url), verbosity=verbosity) +@dataclass class File: - def __init__(self, path: str, content_type: Optional[str]) -> None: - self.path = path - if content_type is None: - self.content_type = mimetypes.guess_type(path)[0] - else: - self.content_type = content_type + path: str + content_type: Optional[str] = None + + def __post_init__(self) -> None: + if self.content_type is None: + self.content_type = mimetypes.guess_type(self.path)[0] def get_http_url( diff --git a/src/pip/_internal/req/__init__.py b/src/pip/_internal/req/__init__.py index 16de903a44c..422d851d729 100644 --- a/src/pip/_internal/req/__init__.py +++ b/src/pip/_internal/req/__init__.py @@ -1,5 +1,6 @@ import collections import logging +from dataclasses import dataclass from typing import Generator, List, Optional, Sequence, Tuple from pip._internal.utils.logging import indent_log @@ -18,12 +19,9 @@ logger = logging.getLogger(__name__) +@dataclass(frozen=True) class InstallationResult: - def __init__(self, name: str) -> None: - self.name = name - - def __repr__(self) -> str: - return f"InstallationResult(name={self.name!r})" + name: str def _validate_requirements( diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index 74296345620..36f517e599d 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -12,6 +12,7 @@ import logging import os import re +from dataclasses import dataclass from typing import Collection, Dict, List, Optional, Set, Tuple, Union from pip._vendor.packaging.markers import Marker @@ -191,18 +192,12 @@ def deduce_helpful_msg(req: str) -> str: return msg +@dataclass(frozen=True) class RequirementParts: - def __init__( - self, - requirement: Optional[Requirement], - link: Optional[Link], - markers: Optional[Marker], - extras: Set[str], - ): - self.requirement = requirement - self.link = link - self.markers = markers - self.extras = extras + requirement: Optional[Requirement] + link: Optional[Link] + markers: Optional[Marker] + extras: Set[str] def parse_req_from_editable(editable_req: str) -> RequirementParts: diff --git a/src/pip/_internal/resolution/resolvelib/base.py b/src/pip/_internal/resolution/resolvelib/base.py index 9c0ef5ca7b9..4269c7fbecc 100644 --- a/src/pip/_internal/resolution/resolvelib/base.py +++ b/src/pip/_internal/resolution/resolvelib/base.py @@ -1,3 +1,4 @@ +from dataclasses import dataclass from typing import FrozenSet, Iterable, Optional, Tuple, Union from pip._vendor.packaging.specifiers import SpecifierSet @@ -19,13 +20,11 @@ def format_name(project: NormalizedName, extras: FrozenSet[NormalizedName]) -> s return f"{project}[{extras_expr}]" +@dataclass(frozen=True) class Constraint: - def __init__( - self, specifier: SpecifierSet, hashes: Hashes, links: FrozenSet[Link] - ) -> None: - self.specifier = specifier - self.hashes = hashes - self.links = links + specifier: SpecifierSet + hashes: Hashes + links: FrozenSet[Link] @classmethod def empty(cls) -> "Constraint": diff --git a/src/pip/_internal/utils/misc.py b/src/pip/_internal/utils/misc.py index 1ad3f6162a2..a5d17565812 100644 --- a/src/pip/_internal/utils/misc.py +++ b/src/pip/_internal/utils/misc.py @@ -11,6 +11,7 @@ import sys import sysconfig import urllib.parse +from dataclasses import dataclass from functools import partial from io import StringIO from itertools import filterfalse, tee, zip_longest @@ -580,10 +581,10 @@ def redact_auth_from_requirement(req: Requirement) -> str: return str(req).replace(req.url, redact_auth_from_url(req.url)) +@dataclass(frozen=True) class HiddenText: - def __init__(self, secret: str, redacted: str) -> None: - self.secret = secret - self.redacted = redacted + secret: str + redacted: str def __repr__(self) -> str: return f"" diff --git a/src/pip/_internal/utils/models.py b/src/pip/_internal/utils/models.py deleted file mode 100644 index b6bb21a8b26..00000000000 --- a/src/pip/_internal/utils/models.py +++ /dev/null @@ -1,39 +0,0 @@ -"""Utilities for defining models -""" - -import operator -from typing import Any, Callable, Type - - -class KeyBasedCompareMixin: - """Provides comparison capabilities that is based on a key""" - - __slots__ = ["_compare_key", "_defining_class"] - - def __init__(self, key: Any, defining_class: Type["KeyBasedCompareMixin"]) -> None: - self._compare_key = key - self._defining_class = defining_class - - def __hash__(self) -> int: - return hash(self._compare_key) - - def __lt__(self, other: Any) -> bool: - return self._compare(other, operator.__lt__) - - def __le__(self, other: Any) -> bool: - return self._compare(other, operator.__le__) - - def __gt__(self, other: Any) -> bool: - return self._compare(other, operator.__gt__) - - def __ge__(self, other: Any) -> bool: - return self._compare(other, operator.__ge__) - - def __eq__(self, other: Any) -> bool: - return self._compare(other, operator.__eq__) - - def _compare(self, other: Any, method: Callable[[Any, Any], bool]) -> bool: - if not isinstance(other, self._defining_class): - return NotImplemented - - return method(self._compare_key, other._compare_key) diff --git a/src/pip/_internal/vcs/git.py b/src/pip/_internal/vcs/git.py index 8c242cf8956..0425debb3ae 100644 --- a/src/pip/_internal/vcs/git.py +++ b/src/pip/_internal/vcs/git.py @@ -4,6 +4,7 @@ import re import urllib.parse import urllib.request +from dataclasses import replace from typing import List, Optional, Tuple from pip._internal.exceptions import BadCommand, InstallationError @@ -217,7 +218,7 @@ def resolve_revision( if sha is not None: rev_options = rev_options.make_new(sha) - rev_options.branch_name = rev if is_branch else None + rev_options = replace(rev_options, branch_name=(rev if is_branch else None)) return rev_options diff --git a/src/pip/_internal/vcs/versioncontrol.py b/src/pip/_internal/vcs/versioncontrol.py index 7eef8ec898e..ce9cf112d31 100644 --- a/src/pip/_internal/vcs/versioncontrol.py +++ b/src/pip/_internal/vcs/versioncontrol.py @@ -5,6 +5,7 @@ import shutil import sys import urllib.parse +from dataclasses import dataclass, field from typing import ( Any, Dict, @@ -114,33 +115,22 @@ def __init__(self, url: str): self.url = url +@dataclass(frozen=True) class RevOptions: """ Encapsulates a VCS-specific revision to install, along with any VCS install options. - Instances of this class should be treated as if immutable. + Args: + vc_class: a VersionControl subclass. + rev: the name of the revision to install. + extra_args: a list of extra options. """ - def __init__( - self, - vc_class: Type["VersionControl"], - rev: Optional[str] = None, - extra_args: Optional[CommandArgs] = None, - ) -> None: - """ - Args: - vc_class: a VersionControl subclass. - rev: the name of the revision to install. - extra_args: a list of extra options. - """ - if extra_args is None: - extra_args = [] - - self.extra_args = extra_args - self.rev = rev - self.vc_class = vc_class - self.branch_name: Optional[str] = None + vc_class: Type["VersionControl"] + rev: Optional[str] = None + extra_args: CommandArgs = field(default_factory=list) + branch_name: Optional[str] = None def __repr__(self) -> str: return f"" @@ -354,7 +344,7 @@ def make_rev_options( rev: the name of a revision to install. extra_args: a list of extra options. """ - return RevOptions(cls, rev, extra_args=extra_args) + return RevOptions(cls, rev, extra_args=extra_args or []) @classmethod def _is_local_repository(cls, repo: str) -> bool: diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index c5545e37d01..2550cae412d 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -49,11 +49,3 @@ def test_sets_correct_variables(self) -> None: assert obj.name == "A" assert obj.version == parse_version("1.0.0") assert obj.link.url == "https://somewhere.com/path/A-1.0.0.tar.gz" - - # NOTE: This isn't checking the ordering logic; only the data provided to - # it is correct. - def test_sets_the_right_key(self) -> None: - obj = candidate.InstallationCandidate( - "A", "1.0.0", Link("https://somewhere.com/path/A-1.0.0.tar.gz") - ) - assert obj._compare_key == (obj.name, obj.version, obj.link)