Skip to content

Stdlib: add many missing __hash__ and __eq__ methods - #10464

Merged
srittau merged 4 commits into
python:mainfrom
AlexWaygood:missing-hash
Jul 17, 2023
Merged

srittau merged 4 commits into
python:mainfrom
AlexWaygood:missing-hash

Conversation

@AlexWaygood

Copy link
Copy Markdown
Member

Pyright has recently been trying to add some sophisticated understanding of how hashability works at runtime. Namely, if a class overrides __eq__ at runtime but doesn't override __hash__, the runtime views instances of the class as unhashable: if you override __eq__, you also have to explicitly override __hash__ as well if you want instances to be hashable. However, pyright is currently unable to implement its new feature, because of the fact that many classes in the stdlib override __hash__ at runtime, but don't currently include __hash__ in typeshed's stubs: microsoft/pyright#5513. Let's fix that.

I also added many missing __eq__ methods. As well as their relevance to hashability, these are also used as a heuristic for mypy's overlapping-equality check. Like with __hash__, therefore, it's important to include these in the stub even if the signature on a subclass is the same as on a superclass.

I found all these with the following stubtest patch:

Details
diff --git a/mypy/stubtest.py b/mypy/stubtest.py
index f06faa962..5921bd49d 100644
--- a/mypy/stubtest.py
+++ b/mypy/stubtest.py
@@ -512,6 +512,8 @@ def verify_typeinfo(
     if is_runtime_typeddict:
         to_check.discard("__new__")

+    errored_entries: set[str] = set()
+
     for entry in sorted(to_check):
         mangled_entry = entry
         if entry.startswith("__") and not entry.endswith("__"):
@@ -536,7 +538,43 @@ def verify_typeinfo(
             and isinstance(runtime_attr, types.WrapperDescriptorType)
             and is_dunder(mangled_entry, exclude_special=True)
         ):
-            yield from verify(stub_to_verify, runtime_attr, object_path + [entry])
+            for error in verify(stub_to_verify, runtime_attr, object_path + [entry]):
+                errored_entries.add(entry)
+                yield error
+
+    if "__hash__" not in errored_entries:
+        if (
+            runtime.__dict__.get("__hash__") is not None
+            and "__hash__" not in stub.names
+        ):
+            yield Error(
+                object_path + ["__hash__"],
+                "overrides __hash__ at runtime but not in the stub",
+                stub_object=MISSING,
+                runtime_object=runtime.__hash__
+            )
+        if "__hash__" not in runtime.__dict__ and "__hash__" in stub.names:
+            yield Error(
+                object_path + ["__hash__"],
+                "overrides __hash__ in the stub but not at runtime",
+                stub_object=stub.names["__hash__"].node,
+                runtime_object=MISSING
+            )
+    if "__eq__" not in errored_entries:
+        if "__eq__" in runtime.__dict__ and "__eq__" not in stub.names:
+            yield Error(
+                object_path + ["__eq__"],
+                "overrides __eq__ at runtime but not in the stub",
+                stub_object=MISSING,
+                runtime_object=runtime.__eq__
+            )
+        if "__eq__" not in runtime.__dict__ and "__eq__" in stub.names:
+            yield Error(
+                object_path + ["__eq__"],
+                "overrides __eq__ in the stub but not at runtime",
+                stub_object=stub.names["__eq__"].node,
+                runtime_object=MISSING
+            )

Cc. @TheTripleV

@github-actions

Copy link
Copy Markdown
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

streamlit (https://github.com/streamlit/streamlit)
- lib/tests/streamlit/elements/date_input_test.py: note: In member "test_range_session_state" of class "DateInputTest":
- lib/tests/streamlit/elements/date_input_test.py:174:16: error: Non-overlapping equality check (left operand type: "Union[date, Tuple[], Tuple[date], Tuple[date, date]]", right operand type: "List[datetime]")  [comparison-overlap]

@srittau srittau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants