-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
support pep593 annotations #333
Conversation
src/check_ast.rs
Outdated
if match_name_or_attr(value, "Literal") { | ||
self.in_literal = true; | ||
} | ||
if match_name_or_attr(value, "Annotated") { | ||
self.in_pep593_annotated = true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a side note: it's totally valid to do from typing import Literal as Foo
, I think this is just checking the name of the variable right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's right. These checks are a bit limited in that they don't follow reassignment for these identifiers.
Thanks for this! Excited to review. |
Great PR! Thanks for putting it together! I made a couple minor tweaks but nothing major. |
ExprKind::Attribute { attr, .. } => { | ||
if typing::is_annotated_subscript(attr) { | ||
Some(SubscriptKind::AnnotatedSubscript) | ||
} else if typing::is_pep593_annotated_subscript(attr) { | ||
Some(SubscriptKind::PEP593AnnotatedSubscript) | ||
} else { | ||
None | ||
} | ||
} | ||
ExprKind::Name { id, .. } => { | ||
if typing::is_annotated_subscript(id) { | ||
Some(SubscriptKind::AnnotatedSubscript) | ||
} else if typing::is_pep593_annotated_subscript(id) { | ||
Some(SubscriptKind::PEP593AnnotatedSubscript) | ||
} else { | ||
None | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can definitely be refactored to avoid duplication, I got lazy 😴
) ## Summary I think this code has existed since the start of `typing.Annotated` support (astral-sh#333), and was then overlooked over a series of refactors. Closes astral-sh#10279.
Currently PEP593 annotations throw an error because they are being processed as regular type annotations where instead the first argument should be processed as a regular annotations and the rest should be processed as arbitrary code (and can have string constants, which is what causes the current error).