Skip to content

fix: Narrowing with "tags" on unions (of TypedDicts or normal classes) doesn't work with the match statement #18791

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

Merged
merged 2 commits into from
Mar 14, 2025

Conversation

gopoto
Copy link
Contributor

@gopoto gopoto commented Mar 12, 2025

Summary

Addressed issue #16286, which reported that match statements failed to narrow unions when matching on discriminant values (e.g. tag fields in TypedDicts or classes). This led to mypy complaining about missing keys/attributes inside case blocks even though plain if ... == branches worked.

What changed?

  • Binder fix in mypy/checker.py:
    • In visit_match_stmt, after calling conditional_types_to_typemaps for the subject, the resulting type map is passed through propagate_up_typemap_info (just like in the if/isinstance code paths). This means if the subject is an attribute or index expression (d.tag / d["tag"]), we try to refine the parent d based on the pattern.
    • Corrected indentation and ensured we only push pattern_map when it's defined, to avoid UnboundLocalError in unreachable branches.
  • New tests in test-data/unit/check-python310.test:
    • testMatchNarrowingUnionTypedDictViaIndex checks that match d["tag"] refines a TypedDict union and allows access to the appropriate key in each branch.
    • testMatchNarrowingUnionClassViaAttribute checks the same for a class union using match d.tag.
  • Changelog: Added a short bullet under “Next Release” describing the bug fix.

Why?

Previously, the binder only narrowed the type of the expression being matched (e.g. d["tag"]), not the parent expression. Without propagating that information upwards, mypy didn't realize it could rule out unrelated TypedDict/class variants in each case, leading to erroneous typeddict-item and union-attr errors. The fix mirrors the existing behaviour for if and similar checks.

How was it tested?

  • Ran pytest -k testMatchNarrowingUnionTypedDictViaIndex and pytest -k testMatchNarrowingUnionClassViaAttribute to confirm the new tests passed.
  • Ran pytest -k check-python310 to ensure the entire pattern-matching suite still passes after the change.
  • Ran the mypy self-check (python runtests.py self).
  • Ran pre-commit run --all-files to satisfy formatting and lint hooks.

Checklist

  • Added regression tests for the reported bug.
  • Verified new tests fail on the unpatched code and pass with the fix.
  • Ran mypy’s self-check and relevant pytest suites.
  • Updated CHANGELOG.md.
  • Ran pre-commit hooks.

Fixes #16286.


This PR was generated by an AI system in collaboration with maintainers: @hauntsaninja

…) doesn't work with the match statement

Signed-off-by: Gene Parmesan Thomas <[email protected]>

This comment has been minimized.

@gopoto gopoto marked this pull request as ready for review March 13, 2025 23:57
Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

This looks good, just had two minor comments

CHANGELOG.md Outdated
@@ -45,6 +45,14 @@ class A:

Contributed by Marc Mueller (PR [18641](https://github.com/python/mypy/pull/18641))

### Other Notable Fixes and Improvements
Copy link
Collaborator

Choose a reason for hiding this comment

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

we typically will do changelog entries later, can you remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the changelog entry for now; will add it later alongside other notes.

mypy/checker.py Outdated
@@ -5527,6 +5527,14 @@ def visit_match_stmt(self, s: MatchStmt) -> None:
pattern_map, else_map = conditional_types_to_typemaps(
named_subject, pattern_type.type, pattern_type.rest_type
)
# Also refine the parent expression of the subject.
Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks for docs, but i think this is fairly clear from the propagate_up_typemap_info docstring, so could you remove the comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dropped the extra comment here since the helper’s docstring is enough

Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@hauntsaninja hauntsaninja merged commit 6e21887 into python:master Mar 14, 2025
18 checks passed
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.

Narrowing with "tags" on unions (of TypedDicts or normal classes) doesn't work with the match statement
2 participants