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 crash with malformed TypedDicts and disllow-any-expr #13963

Merged

Conversation

Michael0x2a
Copy link
Collaborator

Fixes #13066

During the semanal phase, mypy opts to ignore and skip processing any malformed or illegal statements inside of a TypedDict class definition, such as method definitions.

Skipping semanal analysis on these statements can cause any number of odd downstream problems: the type-checking phase assumes that all semanal-only semantic constructs (e.g. FakeInfo) have been purged by this point, and so can crash at any point once this precondition has been violated.

This diff opts to solve this problem by filtering down the list of statements so we keep only the ones we know are legal within a TypedDict definition.

The other possible solution to this problem is to modify mypy so we skip checking TypedDict class bodies entirely during type checking and fine-grained deps analysis. Doing this would also let address #10007 and supersede my other diff #13732.

I decided against doing this for now because:

  1. I wasn't sure if this was actually safe, especially in the fine-grained deps phase and for mypyc.
  2. I think no matter what, the semanal phase should not leak semanal-only types: relaxing this postcondition would make it harder to reason about mypy. So, we'd probably want to make this change regardless of what we do in the later phases.

Fixes python#13066

During the semanal phase, mypy opts to ignore and skip processing
any malformed or illegal statements inside of a TypedDict class
definition, such as method definitions.

Skipping semanal analysis on these statements can cause any number
of odd downstream problems: the type-checking phase assumes that all
semanal-only semantic constructs (e.g. FakeInfo) have been purged
by this point, and so can crash at any point once this precondition has
been violated.

This diff opts to solve this problem by filtering down the list of
statements so we keep only the ones we know are legal within a TypedDict
definition.

The other possible solution to this problem is to modify mypy so we
skip checking TypedDict class bodies entirely during type checking and
fine-grained deps analysis. Doing this would also let address python#10007
and supersede my other diff python#13732.

I decided against doing this for now because:

1. I wasn't sure if this was actually safe, especially in the
   fine-grained deps phase and for mypyc.
2. I think no matter what, the semanal phase should not leak
   semanal-only types: relaxing this postcondition would make
   it harder to reason about mypy. So, we'd probably want to make this
   change regardless of what we do in the later phases.
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Great, I agree that this makes it much easier to reason about what will happen in later phases of type checking.

@JukkaL JukkaL merged commit 7569d88 into python:master Oct 31, 2022
ilevkivskyi added a commit that referenced this pull request Nov 18, 2022
Fixes #14098 

Having invalid statements in a NamedTuple is almost like a syntax error,
we can remove them after giving an error (without further analysis).
This PR does almost exactly the same as
#13963 did for TypedDicts.

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants