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

Add warning for PEP 604 when not in annotation #3937

Closed
wookie184 opened this issue Apr 11, 2023 · 1 comment · Fixed by #4170
Closed

Add warning for PEP 604 when not in annotation #3937

wookie184 opened this issue Apr 11, 2023 · 1 comment · Fixed by #4170
Labels
rule Implementing or modifying a lint rule

Comments

@wookie184
Copy link
Contributor

wookie184 commented Apr 11, 2023

Ruff doesn't warn on this:

import typing

y = typing.Union[int, str]
z = typing.Optional[str]

This seems to be as a result of #3215, although I don't really understand the reasoning there, the same change is made if a type is used in an annotation, and as it can still be accessed at runtime it could still cause errors.

For example,

from typing import Union

new_types = (int, float)
def thing(arg: Union[new_types]):
    ...

becomes

new_types = (int, float)
def thing(arg: new_types):
    ...

(really the issue there seems to be invalid use of Union in the first place, afaik the only variables you can use there are type aliases, which that tuple is not)

This is also the case in the other issue linked, #3215, where as far as I can tell the issue happens whether the union is defined in a variable or directly in the annotation.

Anyway, I don't really mind too much about autofixes, but I think it would be nice to be able to have a warning for it regardless.

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Apr 12, 2023
@charliermarsh
Copy link
Member

I think it makes sense to warn on them at least. I'm hesitant to fix since it can introduce subtle bugs, such as that identified in #3215, whereby replacing Union[new_types] with new_types leads to a runtime error (since we can't know that new_types is a tuple there, and not a single type).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants