Skip to content

[match-case] fix matching against typing.Callable and Protocol types. #19471

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 17 commits into
base: master
Choose a base branch
from

Conversation

randolf-scholz
Copy link
Contributor

@randolf-scholz randolf-scholz commented Jul 17, 2025

Added extra logic in checker.py:conditional_types function to deal with structural types such as typing.Callable or protocols.

new tests

  • testMatchClassPatternCallable: tests case Callable() as fn usage
  • testMatchClassPatternProtocol: tests case Proto() usage, where Proto is a Protocol
  • testMatchClassPatternCallbackProtocol: tests case Proto() usage, where Proto is a Callback-Protocol
  • testGenericAliasIsinstanceUnreachable: derived from a mypy-primer failure in mesonbuild. Tests that isinstance(x, Proto) can produce unreachable error.
  • testGenericAliasRedundantExprCompoundIfExpr: derived from a CI failure of python runtest.py self of an earlier version of this PR.

modified tests

  • testOverloadOnProtocol added annotations to overload implementation, which wasn't getting checked. Added missing return. Fixed return type in second branch.

This comment has been minimized.

@randolf-scholz randolf-scholz marked this pull request as draft July 19, 2025 22:16

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@randolf-scholz randolf-scholz marked this pull request as ready for review July 20, 2025 10:49
The second element is the type it would hold if it was not the proposed type, if any.
UninhabitedType means unreachable.
None means no new information can be inferred.
If default is set it is returned instead.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this line shouldn't be separated from the previous one? Now it's a bit ambiguous whether default is returned instead of None only or instead of both None and Uninhabited

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I do not think this part of the docstring is really accurate to begin with. Looking at the code on the main branch

https://github.com/python/mypy/blob/c6b40df63ce0fecab6bced800ce778d99587c9b8/mypy/checker.py#L7990-
L8045

How about something like:

Returns a 2-tuple:
    The first element is the proposed type, if the expression can be the proposed type.
        (or default, if default is set and the expression is a subtype of the proposed type).
    The second element is the type it would hold if it was not the proposed type, if any.
        (or default, if default is set and the expression is not a subtype of the proposed type).

    UninhabitedType means unreachable.
    None means no new information can be inferred.

@sterliakov
Copy link
Collaborator

Also, does this fix #19470 completely or partially? Please update the description to either remove "partially" or remove "fixes" magic word, so that half-resolved issue does not get autoclosed

This comment has been minimized.

Copy link
Contributor

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

mitmproxy (https://github.com/mitmproxy/mitmproxy)
+ mitmproxy/http.py:607: error: Unused "type: ignore" comment  [unused-ignore]

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.

match-case class checking against collection.abc.Callable raises error [misc]
2 participants