diff --git a/src/poetry/core/packages/dependency.py b/src/poetry/core/packages/dependency.py index 363235dd8..5c3fd7442 100644 --- a/src/poetry/core/packages/dependency.py +++ b/src/poetry/core/packages/dependency.py @@ -2,6 +2,7 @@ import os import re +import warnings from contextlib import suppress from pathlib import Path @@ -58,7 +59,7 @@ def __init__( self._constraint: VersionConstraint self._pretty_constraint: str - self.set_constraint(constraint=constraint) + self.constraint = constraint # type: ignore[assignment] self._optional = optional @@ -99,7 +100,8 @@ def name(self) -> str: def constraint(self) -> VersionConstraint: return self._constraint - def set_constraint(self, constraint: str | VersionConstraint) -> None: + @constraint.setter + def constraint(self, constraint: str | VersionConstraint) -> None: from poetry.core.semver.version_constraint import VersionConstraint try: @@ -111,6 +113,15 @@ def set_constraint(self, constraint: str | VersionConstraint) -> None: self._constraint = parse_constraint("*") self._pretty_constraint = str(constraint) + def set_constraint(self, constraint: str | VersionConstraint) -> None: + warnings.warn( + "Calling method 'set_constraint' is deprecated and will be removed. " + "It has been replaced by the property 'constraint' for consistency.", + DeprecationWarning, + stacklevel=2, + ) + self.constraint = constraint # type: ignore[assignment] + @property def pretty_constraint(self) -> str: return self._pretty_constraint @@ -241,6 +252,7 @@ def transitive_python_constraint(self) -> VersionConstraint: @property def extras(self) -> frozenset[str]: + # extras activated in a dependency is the same as features return self._features @property @@ -608,17 +620,11 @@ def __eq__(self, other: Any) -> bool: if not isinstance(other, Dependency): return NotImplemented - return ( - self.is_same_package_as(other) - and self._constraint == other.constraint - and self.extras == other.extras - ) - - def __ne__(self, other: Any) -> bool: - return not self.__eq__(other) + return super().__eq__(other) and self._constraint == other.constraint def __hash__(self) -> int: - return super().__hash__() ^ hash(self._constraint) ^ hash(self.extras) + # don't include _constraint in hash because it is mutable! + return super().__hash__() def __str__(self) -> str: if self.is_root: diff --git a/src/poetry/core/packages/directory_dependency.py b/src/poetry/core/packages/directory_dependency.py index 05ad95ea6..8558c5d50 100644 --- a/src/poetry/core/packages/directory_dependency.py +++ b/src/poetry/core/packages/directory_dependency.py @@ -105,7 +105,7 @@ def with_constraint( extras=list(self.extras), ) - new.set_constraint(constraint) + new.constraint = constraint # type: ignore[assignment] new.is_root = self.is_root new.python_versions = self.python_versions new.marker = self.marker @@ -135,6 +135,3 @@ def __str__(self) -> str: path = self._path.as_posix() return f"{self._pretty_name} ({self._pretty_constraint} {path})" - - def __hash__(self) -> int: - return hash((self._name, self._full_path.as_posix())) diff --git a/src/poetry/core/packages/file_dependency.py b/src/poetry/core/packages/file_dependency.py index d8c01fdc0..f9afd1fc0 100644 --- a/src/poetry/core/packages/file_dependency.py +++ b/src/poetry/core/packages/file_dependency.py @@ -85,7 +85,7 @@ def with_constraint(self, constraint: str | VersionConstraint) -> FileDependency extras=list(self.extras), ) - new.set_constraint(constraint) + new.constraint = constraint # type: ignore[assignment] new.is_root = self.is_root new.python_versions = self.python_versions new.marker = self.marker @@ -114,6 +114,3 @@ def __str__(self) -> str: return self._pretty_name return f"{self._pretty_name} ({self._pretty_constraint} {self._path})" - - def __hash__(self) -> int: - return hash((self._name, self._full_path)) diff --git a/src/poetry/core/packages/package.py b/src/poetry/core/packages/package.py index 2d6ea5df3..f59ddb191 100644 --- a/src/poetry/core/packages/package.py +++ b/src/poetry/core/packages/package.py @@ -558,14 +558,14 @@ def clone(self: T) -> T: clone.__dict__ = copy.deepcopy(self.__dict__) return clone - def __hash__(self) -> int: - return super().__hash__() ^ hash(self._version) - def __eq__(self, other: object) -> bool: if not isinstance(other, Package): return NotImplemented - return self.is_same_package_as(other) and self._version == other.version + return super().__eq__(other) and self._version == other.version + + def __hash__(self) -> int: + return super().__hash__() ^ hash(self._version) def __str__(self) -> str: return f"{self.complete_name} ({self.full_pretty_version})" diff --git a/src/poetry/core/packages/specification.py b/src/poetry/core/packages/specification.py index f34a8773e..20fcfde89 100644 --- a/src/poetry/core/packages/specification.py +++ b/src/poetry/core/packages/specification.py @@ -151,19 +151,25 @@ def is_same_package_as(self, other: PackageSpecification) -> bool: return self.is_same_source_as(other) + def __eq__(self, other: object) -> bool: + if not isinstance(other, PackageSpecification): + return NotImplemented + return self.is_same_package_as(other) + def __hash__(self) -> int: - if not self._source_type: - return hash(self._name) - - return ( - hash(self._name) - ^ hash(self._source_type) - ^ hash(self._source_url) - ^ hash(self._source_reference) - ^ hash(self._source_resolved_reference) - ^ hash(self._source_subdirectory) - ^ hash(self._features) - ) + result = hash(self.complete_name) # complete_name includes features + + if self._source_type: + # Don't include _source_reference and _source_resolved_reference in hash + # because two specs can be equal even if these attributes are not equal. + # (They must still meet certain conditions. See is_same_source_as().) + result ^= ( + hash(self._source_type) + ^ hash(self._source_url) + ^ hash(self._source_subdirectory) + ) + + return result def __str__(self) -> str: raise NotImplementedError() diff --git a/src/poetry/core/packages/url_dependency.py b/src/poetry/core/packages/url_dependency.py index 5a14e89fd..dd18f85d8 100644 --- a/src/poetry/core/packages/url_dependency.py +++ b/src/poetry/core/packages/url_dependency.py @@ -65,7 +65,7 @@ def with_constraint(self, constraint: str | VersionConstraint) -> URLDependency: extras=list(self.extras), ) - new.set_constraint(constraint) + new.constraint = constraint # type: ignore[assignment] new.is_root = self.is_root new.python_versions = self.python_versions new.marker = self.marker @@ -78,6 +78,3 @@ def with_constraint(self, constraint: str | VersionConstraint) -> URLDependency: def __str__(self) -> str: return f"{self._pretty_name} ({self._pretty_constraint} url)" - - def __hash__(self) -> int: - return hash((self._name, self._url)) diff --git a/src/poetry/core/packages/vcs_dependency.py b/src/poetry/core/packages/vcs_dependency.py index 6da59f0d4..08b7a5772 100644 --- a/src/poetry/core/packages/vcs_dependency.py +++ b/src/poetry/core/packages/vcs_dependency.py @@ -148,7 +148,7 @@ def with_constraint(self, constraint: str | VersionConstraint) -> VCSDependency: extras=list(self.extras), ) - new.set_constraint(constraint) + new.constraint = constraint # type: ignore[assignment] new.is_root = self.is_root new.python_versions = self.python_versions new.marker = self.marker @@ -169,6 +169,3 @@ def __str__(self) -> str: reference += f" rev {self._rev}" return f"{self._pretty_name} ({self._constraint} {reference})" - - def __hash__(self) -> int: - return hash((self._name, self._vcs, self._branch, self._tag, self._rev)) diff --git a/tests/packages/test_dependency.py b/tests/packages/test_dependency.py index 63a8900f6..5089632e9 100644 --- a/tests/packages/test_dependency.py +++ b/tests/packages/test_dependency.py @@ -288,7 +288,7 @@ def test_dependency_string_representation( def test_set_constraint_sets_pretty_constraint() -> None: dependency = Dependency("A", "^1.0") assert dependency.pretty_constraint == "^1.0" - dependency.set_constraint("^2.0") + dependency.constraint = "^2.0" # type: ignore[assignment] assert dependency.pretty_constraint == "^2.0" @@ -359,3 +359,23 @@ def test_marker_properly_unsets_python_constraint() -> None: def test_create_from_pep_508_url_with_activated_extras() -> None: dependency = Dependency.create_from_pep_508("name [fred,bar] @ http://foo.com") assert dependency.extras == {"fred", "bar"} + + +@pytest.mark.parametrize( + "attr_name, value", + [ + ("constraint", "2.0"), + ("python_versions", "<3.8"), + ("transitive_python_versions", "<3.8"), + ("marker", "sys_platform == 'linux'"), + ("transitive_marker", "sys_platform == 'linux'"), + ], +) +def test_mutable_attributes_not_in_hash(attr_name: str, value: str) -> None: + dependency = Dependency("foo", "^1.2.3") + ref_hash = hash(dependency) + + ref_value = getattr(dependency, attr_name) + setattr(dependency, attr_name, value) + assert value != ref_value + assert hash(dependency) == ref_hash diff --git a/tests/packages/test_specification.py b/tests/packages/test_specification.py index 21d5c165e..79ec3052d 100644 --- a/tests/packages/test_specification.py +++ b/tests/packages/test_specification.py @@ -77,3 +77,41 @@ def test_specification_provides( expected: bool, ) -> None: assert spec1.provides(spec2) == expected + + +@pytest.mark.parametrize( + "spec1, spec2", + [ + ( + # nothing except for name and features matters if no source + PackageSpecification("a", None, "url1", "ref1", "resref1", "sub1"), + PackageSpecification("a", None, "url2", "ref2", "resref2", "sub2"), + ), + ( + # ref does not matter if resolved ref is equal + PackageSpecification("a", "type", "url", "ref1", "resref1"), + PackageSpecification("a", "type", "url", "ref2", "resref1"), + ), + ( + # resolved ref does not matter if no ref + PackageSpecification("a", "type", "url", None, "resref1"), + PackageSpecification("a", "type", "url", None, "resref2"), + ), + ( + # resolved ref unset when ref starts with other + PackageSpecification("a", "type", "url", "ref/a", "resref1"), + PackageSpecification("a", "type", "url", "ref", None), + ), + ( + # resolved ref unset when ref starts with other + PackageSpecification("a", "type", "url", "ref/a", None), + PackageSpecification("a", "type", "url", "ref", "resref2"), + ), + ], +) +def test_equal_specifications_have_same_hash( + spec1: PackageSpecification, spec2: PackageSpecification +) -> None: + assert spec1 == spec2 + assert spec2 == spec1 + assert hash(spec1) == hash(spec2)