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 PartialTypes and the enum plugin #14021

Merged
merged 1 commit into from
Nov 7, 2022

Conversation

Michael0x2a
Copy link
Collaborator

Fixes #12109.

The original issue reported that the bug had to do with the use of the --follow-imports=skip flag. However, it turned out this was a red herring after closer inspection: I was able to trigger a more minimal repro both with and without this flag:

from enum import Enum

class Foo(Enum):
    a = []  # E: Need type annotation for "a" (hint: "a: List[<type>] = ...")
    b = None

    def check(self) -> None:
        reveal_type(Foo.a.value)  # N: Revealed type is "<partial list[?]>"
        reveal_type(Foo.b.value)  # N: Revealed type is "<partial None>"

The first two reveal_types demonstrate the crux of the bug: the enum plugin does not correctly handle and convert partial types into regular types when inferring the type of the .value field.

This can then cause any number of downstream problems. For example, suppose we modify def check(...) so it runs reveal_type(self.value). Doing this will trigger a crash in mypy because it makes the enum plugin eventually try running is_equivalent(...) on the two partial types. But is_equivalent does not support partial types, so we crash.

I opted to solve this problem by:

  1. Making the enum plugin explicitly call the fixup_partial_types function on all field types. This prevents the code from crashing.

  2. Modifies mypy so that Final vars are never marked as being PartialTypes. Without this, reveal_type(Foo.b.value) would report a type of Union[Any, None] instead of just None. (Note that all enum fields are implicitly final).

(Explain how this PR changes mypy.)

Fixes python#12109.

The original issue reported that the bug had to do with the use of the
`--follow-imports=skip` flag. However, it turned out this was a red
herring after closer inspection: I was able to trigger a more minimal
repro both with and without this flag:

```python
from enum import Enum

class Foo(Enum):
    a = []  # E: Need type annotation for "a" (hint: "a: List[<type>] = ...")
    b = None

    def check(self) -> None:
        reveal_type(Foo.a.value)  # N: Revealed type is "<partial list[?]>"
        reveal_type(Foo.b.value)  # N: Revealed type is "<partial None>"
```

The first two `reveal_types` demonstrate the crux of the bug: the enum
plugin does not correctly handle and convert partial types into regular
types when inferring the type of the `.value` field.

This can then cause any number of downstream problems. For example,
suppose we modify `def check(...)` so it runs `reveal_type(self.value)`.
Doing this will trigger a crash in mypy because it makes the enum plugin
eventually try running `is_equivalent(...)` on the two partial types.
But `is_equivalent` does not support partial types, so we crash.

I opted to solve this problem by:

1. Making the enum plugin explicitly call the `fixup_partial_types`
   function on all field types. This prevents the code from crashing.

2. Modifies mypy so that Final vars are never marked as being
   PartialTypes. Without this, `reveal_type(Foo.b.value)` would
   report a type of `Union[Any, None]` instead of just `None`.
   (Note that all enum fields are implicitly final).
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2022

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.

One more crash fixed, nice! Looks good.

@JukkaL JukkaL merged commit d2a3e66 into python:master Nov 7, 2022
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.

INTERNAL ERROR RuntimeError visiting partial-type of imported variable with follow_imports=skip
2 participants