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

Quoted annotations should be parsed as if parenthesized #9467

Open
charliermarsh opened this issue Jan 11, 2024 · 3 comments · May be fixed by #10742
Open

Quoted annotations should be parsed as if parenthesized #9467

charliermarsh opened this issue Jan 11, 2024 · 3 comments · May be fixed by #10742
Labels
bug Something isn't working parser Related to the parser

Comments

@charliermarsh
Copy link
Member

It was recently agreed that quoted annotations should be parsed as if they were implicitly wrapped in parentheses. So, e.g., they can contain otherwise-invalid indentation, newlines within binary operators, etc.

See: microsoft/pyright#6940 (comment)

@charliermarsh charliermarsh added the parser Related to the parser label Jan 11, 2024
@charliermarsh charliermarsh added the bug Something isn't working label Jan 29, 2024
@Glyphack
Copy link
Contributor

Glyphack commented Feb 1, 2024

Should this be implemented in the parser layer or here where we emit the warning?

@dhruvmanila
Copy link
Member

I think it should be implemented in the parse_type_annotation function which is used by the AST checker to report an error if found:

/// Parse a type annotation from a string.
pub fn parse_type_annotation(
value: &str,
range: TextRange,
source: &str,
) -> Result<(Expr, AnnotationKind)> {
let expression = &source[range];
if str::raw_contents(expression).is_some_and(|body| body == value) {
// The annotation is considered "simple" if and only if the raw representation (e.g.,
// `List[int]` within "List[int]") exactly matches the parsed representation. This
// isn't the case, e.g., for implicit concatenations, or for annotations that contain
// escaped quotes.
let leading_quote = str::leading_quote(expression).unwrap();
let expr = parse_expression_starts_at(value, range.start() + leading_quote.text_len())?;
Ok((expr, AnnotationKind::Simple))
} else {
// Otherwise, consider this a "complex" annotation.
let mut expr = parse_expression(value)?;
relocate_expr(&mut expr, range);
Ok((expr, AnnotationKind::Complex))
}
}

I would also suggest to look at other references of parse_type_annotation function to make sure they aren't affected in any way with this change. They shouldn't be but just to make sure :)

@Glyphack
Copy link
Contributor

Nice, I can work on this.

@Glyphack Glyphack linked a pull request Apr 2, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working parser Related to the parser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants