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

Detect multiple Literal members in a union #112

Merged
merged 9 commits into from
Jan 20, 2022

Conversation

AlexWaygood
Copy link
Collaborator

@AlexWaygood AlexWaygood commented Jan 20, 2022

Fix #110

Copy link
Collaborator

@Akuli Akuli 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! Just one nit.

pyi.py Show resolved Hide resolved
@AlexWaygood
Copy link
Collaborator Author

Typeshed PR here: python/typeshed#6984

pyi.py Outdated Show resolved Hide resolved
@Akuli
Copy link
Collaborator

Akuli commented Jan 20, 2022

Python 3.7 and 3.8 fail the CI because they don't have ast.Constant. Actually they have it, but Python just doesn't use it in the AST for whatever reason.

I added print(ast.dump(literal)) just under for literal in literals_in_union:, and here's how the AST looks:

akuli@akuli-desktop:~/flake8-pyi$ cat a.pyi
from typing import Union
from typing_extensions import Literal

x: Union[str, Literal['foo'], Literal['bar']]
akuli@akuli-desktop:~/flake8-pyi$ env37/bin/flake8 a.pyi
Index(value=Str(s='foo'))
Index(value=Str(s='bar'))
a.pyi:4:10: Y030 Multiple Literal members in a union. Combine them into one, e.g. "Literal[]".

akuli@akuli-desktop:~/flake8-pyi$ env310/bin/flake8 a.pyi
Constant(value='foo')
Constant(value='bar')
a.pyi:4:10: Y030 Multiple Literal members in a union. Combine them into one, e.g. "Literal['foo', 'bar']".

We should be prepared to handle Literals containing "literal ints, byte and unicode strings, bools, Enum values and None", according to PEP 586.

@AlexWaygood
Copy link
Collaborator Author

AlexWaygood commented Jan 20, 2022

We should be prepared to handle Literals containing "literal ints, byte and unicode strings, bools, Enum values and None", according to PEP 586.

Ugh bytes and enums. Great.

@Akuli
Copy link
Collaborator

Akuli commented Jan 20, 2022

Thinking about it a bit more, I think the logic can be quite simple: if it's a Tuple, append the elts, and if it's anything else, append the object as is, regardless of whether it's ast.Constant or ast.Str.

@AlexWaygood
Copy link
Collaborator Author

Thinking about it a bit more, I think the logic can be quite simple: if it's a Tuple, append the elts, and if it's anything else, append the object as is, regardless of whether it's ast.Constant or ast.Str.

Yeah, mypy can reliably complain if a user puts in an incompatible value, we just need to handle the formatting

pyi.py Show resolved Hide resolved
@Akuli
Copy link
Collaborator

Akuli commented Jan 20, 2022

I added one more test case, because I wasn't sure whether it would work in different Python versions. It seems to be fine though.

CHANGELOG.rst Outdated Show resolved Hide resolved
@AlexWaygood
Copy link
Collaborator Author

I added one more test case, because I wasn't sure whether it would work in different Python versions. It seems to be fine though.

Thanks! I was having my supper :)

@AlexWaygood
Copy link
Collaborator Author

AlexWaygood commented Jan 20, 2022

Shall I hold off on merging until #113 is in?

@Akuli
Copy link
Collaborator

Akuli commented Jan 20, 2022

I think I'll merge this first and fix the conflicts in my PR then.

@AlexWaygood AlexWaygood merged commit d6a3c96 into PyCQA:master Jan 20, 2022
@AlexWaygood AlexWaygood deleted the literal-unions branch January 20, 2022 19:48
Akuli added a commit that referenced this pull request Jan 20, 2022
Detect multiple `Literal` values in a union

Co-authored-by: Akuli <[email protected]>
Akuli added a commit that referenced this pull request Jan 20, 2022
@JelleZijlstra
Copy link
Collaborator

Thanks for the quick implementation here @AlexWaygood!

@AlexWaygood
Copy link
Collaborator Author

Thanks for the quick implementation here @AlexWaygood!

Pleasure 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Literal[X] | Literal[Y] -> Literal[X, Y]
3 participants