packages: make hash() consistent and incl features

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:

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()))

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))

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})"

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()

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))

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))

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

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)