Skip to content

Omit constraints referencing dict if a conflicting TypedDict constraint exists #19225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sterliakov
Copy link
Collaborator

@sterliakov sterliakov commented Jun 4, 2025

Fixes #19201.

When inferring callable constraints, excludes constraints with dict target when other constraints have TypedDict type. The right way to do this would be to allow backtracking in meet and join (so that dict can be promoted to TypedDict if possible), but that seems to be way more difficult and needs significant refactoring first.

This should be safe as we never rely solely on solutions, such inference is followed by actually checking all arguments, so any incompatibilities will still be reported.

This comment has been minimized.

@sterliakov sterliakov marked this pull request as ready for review June 4, 2025 11:56
Copy link
Contributor

github-actions bot commented Jun 4, 2025

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

core (https://github.com/home-assistant/core)
+ homeassistant/core.py:2342: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/scripts/benchmark/__init__.py:144: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/scripts/benchmark/__init__.py:144: error: Argument 2 to "async_fire" of "EventBus" has incompatible type "dict[str, object]"; expected "EventStateChangedData | None"  [arg-type]
+ homeassistant/scripts/benchmark/__init__.py:144: note: Error code "arg-type" not covered by "type: ignore" comment
+ homeassistant/scripts/benchmark/__init__.py:177: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/scripts/benchmark/__init__.py:177: error: Argument 2 to "async_fire" of "EventBus" has incompatible type "dict[str, object]"; expected "EventStateChangedData | None"  [arg-type]
+ homeassistant/scripts/benchmark/__init__.py:177: note: Error code "arg-type" not covered by "type: ignore" comment
+ homeassistant/scripts/benchmark/__init__.py:215: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/scripts/benchmark/__init__.py:215: error: Argument 2 to "async_fire" of "EventBus" has incompatible type "dict[str, object]"; expected "EventStateChangedData | None"  [arg-type]
+ homeassistant/scripts/benchmark/__init__.py:215: note: Error code "arg-type" not covered by "type: ignore" comment

@sterliakov sterliakov requested a review from hauntsaninja June 4, 2025 22:02
@A5rocks
Copy link
Collaborator

A5rocks commented Jun 10, 2025

The right way to do this would be to allow backtracking in meet and join (so that dict can be promoted to TypedDict if possible),

Do you think there's any neat way to handle prioritization in such a general way that it solves this too? (i.e. marking constraints as higher quality than others)

I did some basic version of that in #18958 and I would be happy if there's a method that generalizes to solve both this issue and that issue.

Copy link
Collaborator

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

Never mind I thought more about my comment and any nice generalizations I can think of would rely on the dict argument being inferred as a TypedDict. I think this makes sense.

It looks like literals don't work in a similar fashion either, should we preemptively add support?

a_lit = A[Literal[1]]()
f(a_lit, 1)

@sterliakov
Copy link
Collaborator Author

Hm, I don't see any straightforward solution that could set constraints priorities reliably. But your Literal suggestion sounds interesting, I'll try to expand this PR tomorrow to cover this case

@sterliakov
Copy link
Collaborator Author

OK, Literal constraints are significantly different, nothing prevents their precise inference - we already keep track of last_known_value. That should be handled in join.py instead and does not belong to this PR. That would be something like

diff --git a/mypy/join.py b/mypy/join.py
index a012a633d..099df0268 100644
--- a/mypy/join.py
+++ b/mypy/join.py
@@ -625,6 +625,8 @@ class TypeJoinVisitor(TypeVisitor[ProperType]):
             if self.s.fallback.type.is_enum and t.fallback.type.is_enum:
                 return mypy.typeops.make_simplified_union([self.s, t])
             return join_types(self.s.fallback, t.fallback)
+        elif isinstance(self.s, Instance) and self.s.last_known_value == t:
+            return t
         else:
             return join_types(self.s, t.fallback)

This triggers another suggestion: should we record similar last_known_value for dict literals, at least simple ones? That isn't trivial either, but should be doable.

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.

Function with generic class and generic value arguments are not inferred when generic is a TypedDict
2 participants