Skip to content
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

Fix narrowing on match with function subject #16503

Merged
merged 5 commits into from
Feb 17, 2024

Conversation

edpaget
Copy link
Contributor

@edpaget edpaget commented Nov 16, 2023

Fixes #12998

mypy can't narrow match statements with functions subjects because the callexpr node is not a literal node. This adds a 'dummy' literal node that the match statement visitor can use to do the type narrowing.

The python grammar describes the the match subject as a named expression so this uses that nameexpr node as it's literal.

This comment has been minimized.

Fixes python#12998

mypy can't narrow match statements with functions subjects because the
callexpr node is not a literal node. This adds a 'dummy' literal node
that the match statement visitor can use to do the type narrowing.

The python grammar describes the the match subject as a named expression
so this uses that nameexpr node as it's literal.
@edpaget edpaget force-pushed the edpaget/fix-match-type-narrowing branch from 2d04f05 to 5806bc7 Compare November 16, 2023 00:29

This comment has been minimized.

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.

Thank you! I renamed the dummy expr so it won't clash with valid identifiers, but it'd still be a problem with nested match statement :-/

This comment has been minimized.

@edpaget
Copy link
Contributor Author

edpaget commented Jan 3, 2024

Thank you @hauntsaninja! Could we generate random identifiers (maybe uuidv4s?) to avoid the clashes.

Copy link
Contributor

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

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.

Okay, I fixed a bug where because the constructed NameExpr didn't have an associated node, we ended up extending narrowing to all nested match subjects (due to the code in literal_hash)

I'm wary of adding anything nondeterministic to mypy. I added the function name into the ID, we can see how it goes in practice.

@hauntsaninja hauntsaninja merged commit 17271e5 into python:master Feb 17, 2024
18 checks passed
@edpaget edpaget deleted the edpaget/fix-match-type-narrowing branch February 18, 2024 22:18
@edpaget
Copy link
Contributor Author

edpaget commented Feb 18, 2024

Thank you again @hauntsaninja!

hamdanal pushed a commit to hamdanal/mypy that referenced this pull request Feb 20, 2024
Fixes python#12998

mypy can't narrow match statements with functions subjects because the
callexpr node is not a literal node. This adds a 'dummy' literal node
that the match statement visitor can use to do the type narrowing.

The python grammar describes the the match subject as a named expression
so this uses that nameexpr node as it's literal.

---------

Co-authored-by: hauntsaninja <hauntsaninja@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mypy's narrowing for match statements sometimes fails when the subject of the match is a function call
2 participants