From 68102fde3db90b3c44a20b3626563e2a3bac9317 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Tue, 4 Jul 2023 16:48:43 +0200 Subject: [PATCH] refactor: consider order in hash methods and make _markers attribute a tuple instead of a list because it must not be changed --- .../constraints/generic/multi_constraint.py | 6 +-- .../constraints/generic/union_constraint.py | 6 +-- src/poetry/core/version/markers.py | 46 ++++++++----------- tests/version/test_markers.py | 8 ++-- 4 files changed, 26 insertions(+), 40 deletions(-) diff --git a/src/poetry/core/constraints/generic/multi_constraint.py b/src/poetry/core/constraints/generic/multi_constraint.py index b03e1868e..0c9a96b63 100644 --- a/src/poetry/core/constraints/generic/multi_constraint.py +++ b/src/poetry/core/constraints/generic/multi_constraint.py @@ -128,11 +128,7 @@ def __eq__(self, other: object) -> bool: return self._constraints == other._constraints def __hash__(self) -> int: - h = hash("multi") - for constraint in self._constraints: - h ^= hash(constraint) - - return h + return hash(("multi", *self._constraints)) def __str__(self) -> str: constraints = [str(constraint) for constraint in self._constraints] diff --git a/src/poetry/core/constraints/generic/union_constraint.py b/src/poetry/core/constraints/generic/union_constraint.py index 951f5748d..cd19a6a68 100644 --- a/src/poetry/core/constraints/generic/union_constraint.py +++ b/src/poetry/core/constraints/generic/union_constraint.py @@ -178,11 +178,7 @@ def __eq__(self, other: object) -> bool: return self._constraints == other._constraints def __hash__(self) -> int: - h = hash("union") - for constraint in self._constraints: - h ^= hash(constraint) - - return h + return hash(("union", *self._constraints)) def __str__(self) -> str: constraints = [str(constraint) for constraint in self._constraints] diff --git a/src/poetry/core/version/markers.py b/src/poetry/core/version/markers.py index 0dcc5219a..a592805aa 100644 --- a/src/poetry/core/version/markers.py +++ b/src/poetry/core/version/markers.py @@ -151,7 +151,7 @@ def __repr__(self) -> str: return "" def __hash__(self) -> int: - return hash(("", "")) + return hash("any") def __eq__(self, other: object) -> bool: if not isinstance(other, BaseMarker): @@ -192,7 +192,7 @@ def __repr__(self) -> str: return "" def __hash__(self) -> int: - return hash(("", "")) + return hash("empty") def __eq__(self, other: object) -> bool: if not isinstance(other, BaseMarker): @@ -231,6 +231,10 @@ def name(self) -> str: def constraint(self) -> SingleMarkerConstraint: return self._constraint + @property + def _key(self) -> tuple[object, ...]: + return self._name, self._constraint + def validate(self, environment: dict[str, Any] | None) -> bool: if environment is None: return True @@ -283,10 +287,10 @@ def __eq__(self, other: object) -> bool: if not isinstance(other, SingleMarkerLike): return NotImplemented - return self._name == other.name and self._constraint == other.constraint + return self._key == other._key def __hash__(self) -> int: - return hash((self._name, self._constraint)) + return hash(self._key) class SingleMarker(SingleMarkerLike[Union[BaseConstraint, VersionConstraint]]): @@ -367,6 +371,10 @@ def operator(self) -> str: def value(self) -> str: return self._value + @property + def _key(self) -> tuple[object, ...]: + return self._name, self._operator, self._value + def invert(self) -> BaseMarker: if self._operator in ("===", "=="): operator = "!=" @@ -416,10 +424,10 @@ def __eq__(self, other: object) -> bool: if not isinstance(other, SingleMarker): return NotImplemented - return str(self) == str(other) + return self._key == other._key def __hash__(self) -> int: - return hash(str(self)) + return hash(self._key) def __str__(self) -> str: return f'{self._name} {self._operator} "{self._value}"' @@ -502,10 +510,10 @@ def _flatten_markers( class MultiMarker(BaseMarker): def __init__(self, *markers: BaseMarker) -> None: - self._markers = _flatten_markers(markers, MultiMarker) + self._markers = tuple(_flatten_markers(markers, MultiMarker)) @property - def markers(self) -> list[BaseMarker]: + def markers(self) -> tuple[BaseMarker, ...]: return self._markers @property @@ -656,11 +664,7 @@ def __eq__(self, other: object) -> bool: return self._markers == other.markers def __hash__(self) -> int: - h = hash("multi") - for m in self._markers: - h ^= hash(m) - - return h + return hash(("multi", *self._markers)) def __str__(self) -> str: elements = [] @@ -675,10 +679,10 @@ def __str__(self) -> str: class MarkerUnion(BaseMarker): def __init__(self, *markers: BaseMarker) -> None: - self._markers = _flatten_markers(markers, MarkerUnion) + self._markers = tuple(_flatten_markers(markers, MarkerUnion)) @property - def markers(self) -> list[BaseMarker]: + def markers(self) -> tuple[BaseMarker, ...]: return self._markers @property @@ -743,12 +747,6 @@ def of(cls, *markers: BaseMarker) -> BaseMarker: return MarkerUnion(*new_markers) - def append(self, marker: BaseMarker) -> None: - if marker in self._markers: - return - - self._markers.append(marker) - def intersect(self, other: BaseMarker) -> BaseMarker: return intersection(self, other) @@ -836,11 +834,7 @@ def __eq__(self, other: object) -> bool: return self._markers == other.markers def __hash__(self) -> int: - h = hash("union") - for m in self._markers: - h ^= hash(m) - - return h + return hash(("union", *self._markers)) def __str__(self) -> str: return " or ".join(str(m) for m in self._markers) diff --git a/tests/version/test_markers.py b/tests/version/test_markers.py index af1e773e7..ecd494407 100644 --- a/tests/version/test_markers.py +++ b/tests/version/test_markers.py @@ -376,10 +376,10 @@ def test_multi_marker() -> None: m = parse_marker('sys_platform == "darwin" and implementation_name == "cpython"') assert isinstance(m, MultiMarker) - assert m.markers == [ + assert m.markers == ( parse_marker('sys_platform == "darwin"'), parse_marker('implementation_name == "cpython"'), - ] + ) def test_multi_marker_is_empty_is_contradictory() -> None: @@ -652,10 +652,10 @@ def test_marker_union() -> None: m = parse_marker('sys_platform == "darwin" or implementation_name == "cpython"') assert isinstance(m, MarkerUnion) - assert m.markers == [ + assert m.markers == ( parse_marker('sys_platform == "darwin"'), parse_marker('implementation_name == "cpython"'), - ] + ) def test_marker_union_deduplicate() -> None: