Skip to content

Commit

Permalink
Show parsed annotation syntax error message
Browse files Browse the repository at this point in the history
  • Loading branch information
@dhruvmanila
dhruvmanila committed Jan 16, 2025
1 parent cf4ab7c commit 10a25b6
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 87 deletions.
4 changes: 1 addition & 3 deletions crates/red_knot_python_semantic/src/types/string_annotation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,7 @@ pub(crate) fn parse_string_annotation(
} else if raw_contents(node_text)
.is_some_and(|raw_contents| raw_contents == string_literal.as_str())
{
let parsed =
ruff_python_parser::parse_string_annotation(source.as_str(), string_literal);
match parsed {
match ruff_python_parser::parse_string_annotation(source.as_str(), string_literal) {
Ok(parsed) => return Some(parsed),
Err(parse_error) => context.report_lint(
&INVALID_SYNTAX_IN_FORWARD_ANNOTATION,
Expand Down
120 changes: 63 additions & 57 deletions crates/ruff_linter/src/checkers/ast/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ use ruff_python_ast::{helpers, str, visitor, PySourceType};
use ruff_python_codegen::{Generator, Stylist};
use ruff_python_index::Indexer;
use ruff_python_parser::typing::{parse_type_annotation, AnnotationKind, ParsedAnnotation};
use ruff_python_parser::{Parsed, Tokens};
use ruff_python_parser::{ParseError, Parsed, Tokens};
use ruff_python_semantic::all::{DunderAllDefinition, DunderAllFlags};
use ruff_python_semantic::analyze::{imports, typing};
use ruff_python_semantic::{
Expand Down Expand Up @@ -234,7 +234,7 @@ impl<'a> Checker<'a> {
#[allow(clippy::too_many_arguments)]
pub(crate) fn new(
parsed: &'a Parsed<ModModule>,
parsed_annotations_arena: &'a typed_arena::Arena<ParsedAnnotation>,
parsed_annotations_arena: &'a typed_arena::Arena<Result<ParsedAnnotation, ParseError>>,
settings: &'a LinterSettings,
noqa_line_for: &'a NoqaMapping,
noqa: flags::Noqa,
Expand Down Expand Up @@ -425,7 +425,7 @@ impl<'a> Checker<'a> {
pub(crate) fn parse_type_annotation(
&self,
annotation: &ast::ExprStringLiteral,
) -> Option<&'a ParsedAnnotation> {
) -> Result<&'a ParsedAnnotation, &'a ParseError> {
self.parsed_annotations_cache
.lookup_or_parse(annotation, self.locator.contents())
}
Expand All @@ -441,7 +441,7 @@ impl<'a> Checker<'a> {
match_fn: impl FnOnce(&ast::Expr) -> bool,
) -> bool {
if let ast::Expr::StringLiteral(string_annotation) = expr {
let Some(parsed_annotation) = self.parse_type_annotation(string_annotation) else {
let Some(parsed_annotation) = self.parse_type_annotation(string_annotation).ok() else {
return false;
};
match_fn(parsed_annotation.expression())
Expand Down Expand Up @@ -2318,58 +2318,66 @@ impl<'a> Checker<'a> {
while !self.visit.string_type_definitions.is_empty() {
let type_definitions = std::mem::take(&mut self.visit.string_type_definitions);
for (string_expr, snapshot) in type_definitions {
let annotation_parse_result = self.parse_type_annotation(string_expr);
if let Some(parsed_annotation) = annotation_parse_result {
self.parsed_type_annotation = Some(parsed_annotation);
match self.parse_type_annotation(string_expr) {
Ok(parsed_annotation) => {
self.parsed_type_annotation = Some(parsed_annotation);

let annotation = string_expr.value.to_str();
let range = string_expr.range();
let annotation = string_expr.value.to_str();
let range = string_expr.range();

self.semantic.restore(snapshot);
self.semantic.restore(snapshot);

if self.semantic.in_annotation() && self.semantic.in_typing_only_annotation() {
if self.enabled(Rule::QuotedAnnotation) {
pyupgrade::rules::quoted_annotation(self, annotation, range);
if self.semantic.in_annotation()
&& self.semantic.in_typing_only_annotation()
{
if self.enabled(Rule::QuotedAnnotation) {
pyupgrade::rules::quoted_annotation(self, annotation, range);
}
}
}
if self.source_type.is_stub() {
if self.enabled(Rule::QuotedAnnotationInStub) {
flake8_pyi::rules::quoted_annotation_in_stub(self, annotation, range);
if self.source_type.is_stub() {
if self.enabled(Rule::QuotedAnnotationInStub) {
flake8_pyi::rules::quoted_annotation_in_stub(
self, annotation, range,
);
}
}
}

let type_definition_flag = match parsed_annotation.kind() {
AnnotationKind::Simple => SemanticModelFlags::SIMPLE_STRING_TYPE_DEFINITION,
AnnotationKind::Complex => {
SemanticModelFlags::COMPLEX_STRING_TYPE_DEFINITION
}
};

self.semantic.flags |=
SemanticModelFlags::TYPE_DEFINITION | type_definition_flag;
let parsed_expr = parsed_annotation.expression();
self.visit_expr(parsed_expr);
if self.semantic.in_type_alias_value() {
// stub files are covered by PYI020
if !self.source_type.is_stub() && self.enabled(Rule::QuotedTypeAlias) {
flake8_type_checking::rules::quoted_type_alias(
self,
parsed_expr,
string_expr,
);
let type_definition_flag = match parsed_annotation.kind() {
AnnotationKind::Simple => {
SemanticModelFlags::SIMPLE_STRING_TYPE_DEFINITION
}
AnnotationKind::Complex => {
SemanticModelFlags::COMPLEX_STRING_TYPE_DEFINITION
}
};

self.semantic.flags |=
SemanticModelFlags::TYPE_DEFINITION | type_definition_flag;
let parsed_expr = parsed_annotation.expression();
self.visit_expr(parsed_expr);
if self.semantic.in_type_alias_value() {
// stub files are covered by PYI020
if !self.source_type.is_stub() && self.enabled(Rule::QuotedTypeAlias) {
flake8_type_checking::rules::quoted_type_alias(
self,
parsed_expr,
string_expr,
);
}
}
self.parsed_type_annotation = None;
}
self.parsed_type_annotation = None;
} else {
self.semantic.restore(snapshot);

if self.enabled(Rule::ForwardAnnotationSyntaxError) {
self.push_type_diagnostic(Diagnostic::new(
pyflakes::rules::ForwardAnnotationSyntaxError {
body: string_expr.value.to_string(),
},
string_expr.range(),
));
Err(parse_error) => {
self.semantic.restore(snapshot);

if self.enabled(Rule::ForwardAnnotationSyntaxError) {
self.push_type_diagnostic(Diagnostic::new(
pyflakes::rules::ForwardAnnotationSyntaxError {
parse_error: parse_error.error.to_string(),
},
string_expr.range(),
));
}
}
}
}
Expand Down Expand Up @@ -2541,12 +2549,12 @@ impl<'a> Checker<'a> {
}

struct ParsedAnnotationsCache<'a> {
arena: &'a typed_arena::Arena<ParsedAnnotation>,
by_offset: RefCell<FxHashMap<TextSize, Option<&'a ParsedAnnotation>>>,
arena: &'a typed_arena::Arena<Result<ParsedAnnotation, ParseError>>,
by_offset: RefCell<FxHashMap<TextSize, Result<&'a ParsedAnnotation, &'a ParseError>>>,
}

impl<'a> ParsedAnnotationsCache<'a> {
fn new(arena: &'a typed_arena::Arena<ParsedAnnotation>) -> Self {
fn new(arena: &'a typed_arena::Arena<Result<ParsedAnnotation, ParseError>>) -> Self {
Self {
arena,
by_offset: RefCell::default(),
Expand All @@ -2557,17 +2565,15 @@ impl<'a> ParsedAnnotationsCache<'a> {
&self,
annotation: &ast::ExprStringLiteral,
source: &str,
) -> Option<&'a ParsedAnnotation> {
) -> Result<&'a ParsedAnnotation, &'a ParseError> {
*self
.by_offset
.borrow_mut()
.entry(annotation.start())
.or_insert_with(|| {
if let Ok(annotation) = parse_type_annotation(annotation, source) {
Some(self.arena.alloc(annotation))
} else {
None
}
self.arena
.alloc(parse_type_annotation(annotation, source))
.as_ref()
})
}

Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ fn check_dynamically_typed<F>(
{
if let Expr::StringLiteral(string_expr) = annotation {
// Quoted annotations
if let Some(parsed_annotation) = checker.parse_type_annotation(string_expr) {
if let Ok(parsed_annotation) = checker.parse_type_annotation(string_expr) {
if type_hint_resolves_to_any(
parsed_annotation.expression(),
checker,
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff_linter/src/rules/pyflakes/rules/forward_annotation_syntax_error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ use ruff_macros::{derive_message_formats, ViolationMetadata};
/// - [PEP 563 – Postponed Evaluation of Annotations](https://peps.python.org/pep-0563/)
#[derive(ViolationMetadata)]
pub(crate) struct ForwardAnnotationSyntaxError {
pub body: String,
pub parse_error: String,
}

impl Violation for ForwardAnnotationSyntaxError {
#[derive_message_formats]
fn message(&self) -> String {
let ForwardAnnotationSyntaxError { body } = self;
format!("Syntax error in forward annotation: `{body}`")
let ForwardAnnotationSyntaxError { parse_error } = self;
format!("Syntax error in forward annotation: {parse_error}")
}
}
23 changes: 6 additions & 17 deletions ...inter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F722_F722.py.snap
View file
Original file line number Diff line number Diff line change
@@ -1,26 +1,22 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
snapshot_kind: text
---
F722.py:9:12: F722 Syntax error in forward annotation: `///`
F722.py:9:12: F722 Syntax error in forward annotation: Expected an expression
|
9 | def g() -> "///":
| ^^^^^ F722
10 | pass
|

F722.py:13:4: F722 Syntax error in forward annotation: `List[int]☃`
F722.py:13:4: F722 Syntax error in forward annotation: Got unexpected token ☃
|
13 | X: """List[int]"""'☃' = []
| ^^^^^^^^^^^^^^^^^^ F722
14 |
15 | # Type annotations with triple quotes can contain newlines and indentation
|

F722.py:30:11: F722 Syntax error in forward annotation: `
int |
str)
`
F722.py:30:11: F722 Syntax error in forward annotation: Unexpected token at the end of an expression
|
28 | """
29 |
Expand All @@ -34,10 +30,7 @@ str)
35 | invalid2: """
|

F722.py:35:11: F722 Syntax error in forward annotation: `
int) |
str
`
F722.py:35:11: F722 Syntax error in forward annotation: Unexpected token at the end of an expression
|
33 | """
34 |
Expand All @@ -51,9 +44,7 @@ str
40 | ((int)
|

F722.py:39:11: F722 Syntax error in forward annotation: `
((int)
`
F722.py:39:11: F722 Syntax error in forward annotation: unexpected EOF while parsing
|
37 | str
38 | """
Expand All @@ -66,9 +57,7 @@ F722.py:39:11: F722 Syntax error in forward annotation: `
43 | (int
|

F722.py:42:11: F722 Syntax error in forward annotation: `
(int
`
F722.py:42:11: F722 Syntax error in forward annotation: unexpected EOF while parsing
|
40 | ((int)
41 | """
Expand Down
7 changes: 3 additions & 4 deletions ...ter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F722_F722_1.py.snap
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
snapshot_kind: text
---
F722_1.py:8:22: F722 Syntax error in forward annotation: `this isn't python`
F722_1.py:8:22: F722 Syntax error in forward annotation: Unexpected token at the end of an expression
|
6 | @no_type_check
7 | class C:
Expand All @@ -11,7 +10,7 @@ F722_1.py:8:22: F722 Syntax error in forward annotation: `this isn't python`
9 | x: "this also isn't python" = 1
|

F722_1.py:8:46: F722 Syntax error in forward annotation: `this isn't python either`
F722_1.py:8:46: F722 Syntax error in forward annotation: Unexpected token at the end of an expression
|
6 | @no_type_check
7 | class C:
Expand All @@ -20,7 +19,7 @@ F722_1.py:8:46: F722 Syntax error in forward annotation: `this isn't python eith
9 | x: "this also isn't python" = 1
|

F722_1.py:9:12: F722 Syntax error in forward annotation: `this also isn't python`
F722_1.py:9:12: F722 Syntax error in forward annotation: Unexpected token at the end of an expression
|
7 | class C:
8 | def f(self, arg: "this isn't python") -> "this isn't python either":
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ pub(crate) fn implicit_optional(checker: &mut Checker, parameters: &Parameters)

if let Expr::StringLiteral(string_expr) = annotation.as_ref() {
// Quoted annotation.
if let Some(parsed_annotation) = checker.parse_type_annotation(string_expr) {
if let Ok(parsed_annotation) = checker.parse_type_annotation(string_expr) {
let Some(expr) = type_hint_explicitly_allows_none(
parsed_annotation.expression(),
checker,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/ruff/typing.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl<'a> TypingTarget<'a> {
Expr::NoneLiteral(_) => Some(TypingTarget::None),
Expr::StringLiteral(string_expr) => checker
.parse_type_annotation(string_expr)
.as_ref()
.ok()
.map(|parsed_annotation| {
TypingTarget::ForwardReference(parsed_annotation.expression())
}),
Expand Down

0 comments on commit 10a25b6

Please sign in to comment.