packages: make hash() consistent and incl features

Co-authored-by: Arun Babu Neelicattu <arun.neelicattu@gmail.com>

src/poetry/core/packages/dependency.py

@@ -241,6 +241,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 +609,14 @@ 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
-        )
+        return super().__eq__(other) and self._constraint == other.constraint
 
     def __ne__(self, other: Any) -> bool:
         return not self.__eq__(other)
 
     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

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

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

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

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