Skip to content

Support for singleton types in unions with Enum #7693

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
Oct 12, 2019

Conversation

ilai-deutel
Copy link
Contributor

This PR adds supports for singleton types in unions using Enums as described in PEP 484 (without using Final).

As suggested by @Michael0x2a in the corresponding issue, adding another case to the is_singleton_type and coerce_to_literal functions allows mypy to recognize an Enum with 1 value as a singleton type.

Fixes #7279

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.

Thanks for the PR! Looks great.

(I'll give @Michael0x2a a chance to have a look before merging since he's our resident expert on literal types.)

mypy/checker.py Outdated
if typ.last_known_value:
return typ.last_known_value
elif typ.type.is_enum and len(typ.type.names) == 1:
key, = typ.type.names
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe the names dict can also contain reference to methods, which shouldn't be counted as an enum values.

I'd try refactoring out the "get all enum values" check on lines ~4794 to ~4798 of this file into a separate helper function and call it here instead of using typ.type.names directly, and maybe also add a test case making sure mypy doesn't do anything weird if we have an single-value enum that also defines a method.

mypy/checker.py Outdated
@@ -4761,7 +4762,8 @@ def is_singleton_type(typ: Type) -> bool:
typ = get_proper_type(typ)
# TODO: Also make this return True if the type is a bool LiteralType.
# Also make this return True if the type corresponds to ... (ellipsis) or NotImplemented?
return isinstance(typ, NoneType) or (isinstance(typ, LiteralType) and typ.is_enum_literal())
return (isinstance(typ, NoneType) or (isinstance(typ, LiteralType) and typ.is_enum_literal())
or (isinstance(typ, Instance) and typ.type.is_enum and len(typ.type.names) == 1))
Copy link
Collaborator

@Michael0x2a Michael0x2a Oct 11, 2019

Choose a reason for hiding this comment

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

See other comment about typ.type.names -- the same fix should be done here.

Copy link
Collaborator

@Michael0x2a Michael0x2a left a comment

Choose a reason for hiding this comment

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

Looks good now, thanks!

@Michael0x2a Michael0x2a merged commit 34fe00b into python:master Oct 12, 2019
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.

Singleton as enum doesn't work as expected
3 participants