Skip to content

Commit 79e52c7

Browse files
dhruvmaniladhruvmanila
authored
[pyflakes] Show syntax error message for F722 (#15523)
## Summary Ref: #15387 (comment) This PR updates `F722` to show syntax error message instead of the string content. I think it's more useful to show the syntax error message than the string content. In the future, when the diagnostics renderer is more capable, we could even highlight the exact location of the syntax error along with the annotation string. This is also in line with how we show the diagnostic in red knot. ## Test Plan Update existing test snapshots.
1 parent cf4ab7c commit 79e52c7

File tree

8 files changed

+79
-87
lines changed

8 files changed

+79
-87
lines changed

‎crates/red_knot_python_semantic/src/types/string_annotation.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,7 @@ pub(crate) fn parse_string_annotation(
153153
} else if raw_contents(node_text)
154154
.is_some_and(|raw_contents| raw_contents == string_literal.as_str())
155155
{
156-
let parsed =
157-
ruff_python_parser::parse_string_annotation(source.as_str(), string_literal);
158-
match parsed {
156+
match ruff_python_parser::parse_string_annotation(source.as_str(), string_literal) {
159157
Ok(parsed) => return Some(parsed),
160158
Err(parse_error) => context.report_lint(
161159
&INVALID_SYNTAX_IN_FORWARD_ANNOTATION,

‎crates/ruff_linter/src/checkers/ast/mod.rs

Lines changed: 63 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ use ruff_python_ast::{helpers, str, visitor, PySourceType};
4949
use ruff_python_codegen::{Generator, Stylist};
5050
use ruff_python_index::Indexer;
5151
use ruff_python_parser::typing::{parse_type_annotation, AnnotationKind, ParsedAnnotation};
52-
use ruff_python_parser::{Parsed, Tokens};
52+
use ruff_python_parser::{ParseError, Parsed, Tokens};
5353
use ruff_python_semantic::all::{DunderAllDefinition, DunderAllFlags};
5454
use ruff_python_semantic::analyze::{imports, typing};
5555
use ruff_python_semantic::{
@@ -234,7 +234,7 @@ impl<'a> Checker<'a> {
234234
#[allow(clippy::too_many_arguments)]
235235
pub(crate) fn new(
236236
parsed: &'a Parsed<ModModule>,
237-
parsed_annotations_arena: &'a typed_arena::Arena<ParsedAnnotation>,
237+
parsed_annotations_arena: &'a typed_arena::Arena<Result<ParsedAnnotation, ParseError>>,
238238
settings: &'a LinterSettings,
239239
noqa_line_for: &'a NoqaMapping,
240240
noqa: flags::Noqa,
@@ -425,7 +425,7 @@ impl<'a> Checker<'a> {
425425
pub(crate) fn parse_type_annotation(
426426
&self,
427427
annotation: &ast::ExprStringLiteral,
428-
) -> Option<&'a ParsedAnnotation> {
428+
) -> Result<&'a ParsedAnnotation, &'a ParseError> {
429429
self.parsed_annotations_cache
430430
.lookup_or_parse(annotation, self.locator.contents())
431431
}
@@ -441,7 +441,7 @@ impl<'a> Checker<'a> {
441441
match_fn: impl FnOnce(&ast::Expr) -> bool,
442442
) -> bool {
443443
if let ast::Expr::StringLiteral(string_annotation) = expr {
444-
let Some(parsed_annotation) = self.parse_type_annotation(string_annotation) else {
444+
let Some(parsed_annotation) = self.parse_type_annotation(string_annotation).ok() else {
445445
return false;
446446
};
447447
match_fn(parsed_annotation.expression())
@@ -2318,58 +2318,66 @@ impl<'a> Checker<'a> {
23182318
while !self.visit.string_type_definitions.is_empty() {
23192319
let type_definitions = std::mem::take(&mut self.visit.string_type_definitions);
23202320
for (string_expr, snapshot) in type_definitions {
2321-
let annotation_parse_result = self.parse_type_annotation(string_expr);
2322-
if let Some(parsed_annotation) = annotation_parse_result {
2323-
self.parsed_type_annotation = Some(parsed_annotation);
2321+
match self.parse_type_annotation(string_expr) {
2322+
Ok(parsed_annotation) => {
2323+
self.parsed_type_annotation = Some(parsed_annotation);
23242324

2325-
let annotation = string_expr.value.to_str();
2326-
let range = string_expr.range();
2325+
let annotation = string_expr.value.to_str();
2326+
let range = string_expr.range();
23272327

2328-
self.semantic.restore(snapshot);
2328+
self.semantic.restore(snapshot);
23292329

2330-
if self.semantic.in_annotation() && self.semantic.in_typing_only_annotation() {
2331-
if self.enabled(Rule::QuotedAnnotation) {
2332-
pyupgrade::rules::quoted_annotation(self, annotation, range);
2330+
if self.semantic.in_annotation()
2331+
&& self.semantic.in_typing_only_annotation()
2332+
{
2333+
if self.enabled(Rule::QuotedAnnotation) {
2334+
pyupgrade::rules::quoted_annotation(self, annotation, range);
2335+
}
23332336
}
2334-
}
2335-
if self.source_type.is_stub() {
2336-
if self.enabled(Rule::QuotedAnnotationInStub) {
2337-
flake8_pyi::rules::quoted_annotation_in_stub(self, annotation, range);
2337+
if self.source_type.is_stub() {
2338+
if self.enabled(Rule::QuotedAnnotationInStub) {
2339+
flake8_pyi::rules::quoted_annotation_in_stub(
2340+
self, annotation, range,
2341+
);
2342+
}
23382343
}
2339-
}
23402344

2341-
let type_definition_flag = match parsed_annotation.kind() {
2342-
AnnotationKind::Simple => SemanticModelFlags::SIMPLE_STRING_TYPE_DEFINITION,
2343-
AnnotationKind::Complex => {
2344-
SemanticModelFlags::COMPLEX_STRING_TYPE_DEFINITION
2345-
}
2346-
};
2347-
2348-
self.semantic.flags |=
2349-
SemanticModelFlags::TYPE_DEFINITION | type_definition_flag;
2350-
let parsed_expr = parsed_annotation.expression();
2351-
self.visit_expr(parsed_expr);
2352-
if self.semantic.in_type_alias_value() {
2353-
// stub files are covered by PYI020
2354-
if !self.source_type.is_stub() && self.enabled(Rule::QuotedTypeAlias) {
2355-
flake8_type_checking::rules::quoted_type_alias(
2356-
self,
2357-
parsed_expr,
2358-
string_expr,
2359-
);
2345+
let type_definition_flag = match parsed_annotation.kind() {
2346+
AnnotationKind::Simple => {
2347+
SemanticModelFlags::SIMPLE_STRING_TYPE_DEFINITION
2348+
}
2349+
AnnotationKind::Complex => {
2350+
SemanticModelFlags::COMPLEX_STRING_TYPE_DEFINITION
2351+
}
2352+
};
2353+
2354+
self.semantic.flags |=
2355+
SemanticModelFlags::TYPE_DEFINITION | type_definition_flag;
2356+
let parsed_expr = parsed_annotation.expression();
2357+
self.visit_expr(parsed_expr);
2358+
if self.semantic.in_type_alias_value() {
2359+
// stub files are covered by PYI020
2360+
if !self.source_type.is_stub() && self.enabled(Rule::QuotedTypeAlias) {
2361+
flake8_type_checking::rules::quoted_type_alias(
2362+
self,
2363+
parsed_expr,
2364+
string_expr,
2365+
);
2366+
}
23602367
}
2368+
self.parsed_type_annotation = None;
23612369
}
2362-
self.parsed_type_annotation = None;
2363-
} else {
2364-
self.semantic.restore(snapshot);
2365-
2366-
if self.enabled(Rule::ForwardAnnotationSyntaxError) {
2367-
self.push_type_diagnostic(Diagnostic::new(
2368-
pyflakes::rules::ForwardAnnotationSyntaxError {
2369-
body: string_expr.value.to_string(),
2370-
},
2371-
string_expr.range(),
2372-
));
2370+
Err(parse_error) => {
2371+
self.semantic.restore(snapshot);
2372+
2373+
if self.enabled(Rule::ForwardAnnotationSyntaxError) {
2374+
self.push_type_diagnostic(Diagnostic::new(
2375+
pyflakes::rules::ForwardAnnotationSyntaxError {
2376+
parse_error: parse_error.error.to_string(),
2377+
},
2378+
string_expr.range(),
2379+
));
2380+
}
23732381
}
23742382
}
23752383
}
@@ -2541,12 +2549,12 @@ impl<'a> Checker<'a> {
25412549
}
25422550

25432551
struct ParsedAnnotationsCache<'a> {
2544-
arena: &'a typed_arena::Arena<ParsedAnnotation>,
2545-
by_offset: RefCell<FxHashMap<TextSize, Option<&'a ParsedAnnotation>>>,
2552+
arena: &'a typed_arena::Arena<Result<ParsedAnnotation, ParseError>>,
2553+
by_offset: RefCell<FxHashMap<TextSize, Result<&'a ParsedAnnotation, &'a ParseError>>>,
25462554
}
25472555

25482556
impl<'a> ParsedAnnotationsCache<'a> {
2549-
fn new(arena: &'a typed_arena::Arena<ParsedAnnotation>) -> Self {
2557+
fn new(arena: &'a typed_arena::Arena<Result<ParsedAnnotation, ParseError>>) -> Self {
25502558
Self {
25512559
arena,
25522560
by_offset: RefCell::default(),
@@ -2557,17 +2565,15 @@ impl<'a> ParsedAnnotationsCache<'a> {
25572565
&self,
25582566
annotation: &ast::ExprStringLiteral,
25592567
source: &str,
2560-
) -> Option<&'a ParsedAnnotation> {
2568+
) -> Result<&'a ParsedAnnotation, &'a ParseError> {
25612569
*self
25622570
.by_offset
25632571
.borrow_mut()
25642572
.entry(annotation.start())
25652573
.or_insert_with(|| {
2566-
if let Ok(annotation) = parse_type_annotation(annotation, source) {
2567-
Some(self.arena.alloc(annotation))
2568-
} else {
2569-
None
2570-
}
2574+
self.arena
2575+
.alloc(parse_type_annotation(annotation, source))
2576+
.as_ref()
25712577
})
25722578
}
25732579

‎crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ fn check_dynamically_typed<F>(
519519
{
520520
if let Expr::StringLiteral(string_expr) = annotation {
521521
// Quoted annotations
522-
if let Some(parsed_annotation) = checker.parse_type_annotation(string_expr) {
522+
if let Ok(parsed_annotation) = checker.parse_type_annotation(string_expr) {
523523
if type_hint_resolves_to_any(
524524
parsed_annotation.expression(),
525525
checker,

‎crates/ruff_linter/src/rules/pyflakes/rules/forward_annotation_syntax_error.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,13 @@ use ruff_macros::{derive_message_formats, ViolationMetadata};
2424
/// - [PEP 563 – Postponed Evaluation of Annotations](https://peps.python.org/pep-0563/)
2525
#[derive(ViolationMetadata)]
2626
pub(crate) struct ForwardAnnotationSyntaxError {
27-
pub body: String,
27+
pub parse_error: String,
2828
}
2929

3030
impl Violation for ForwardAnnotationSyntaxError {
3131
#[derive_message_formats]
3232
fn message(&self) -> String {
33-
let ForwardAnnotationSyntaxError { body } = self;
34-
format!("Syntax error in forward annotation: `{body}`")
33+
let ForwardAnnotationSyntaxError { parse_error } = self;
34+
format!("Syntax error in forward annotation: {parse_error}")
3535
}
3636
}

‎crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F722_F722.py.snap

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,22 @@
11
---
22
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
3-
snapshot_kind: text
43
---
5-
F722.py:9:12: F722 Syntax error in forward annotation: `///`
4+
F722.py:9:12: F722 Syntax error in forward annotation: Expected an expression
65
|
76
9 | def g() -> "///":
87
| ^^^^^ F722
98
10 | pass
109
|
1110

12-
F722.py:13:4: F722 Syntax error in forward annotation: `List[int]☃`
11+
F722.py:13:4: F722 Syntax error in forward annotation: Got unexpected token ☃
1312
|
1413
13 | X: """List[int]"""'☃' = []
1514
| ^^^^^^^^^^^^^^^^^^ F722
1615
14 |
1716
15 | # Type annotations with triple quotes can contain newlines and indentation
1817
|
1918

20-
F722.py:30:11: F722 Syntax error in forward annotation: `
21-
int |
22-
str)
23-
`
19+
F722.py:30:11: F722 Syntax error in forward annotation: Unexpected token at the end of an expression
2420
|
2521
28 | """
2622
29 |
@@ -34,10 +30,7 @@ str)
3430
35 | invalid2: """
3531
|
3632

37-
F722.py:35:11: F722 Syntax error in forward annotation: `
38-
int) |
39-
str
40-
`
33+
F722.py:35:11: F722 Syntax error in forward annotation: Unexpected token at the end of an expression
4134
|
4235
33 | """
4336
34 |
@@ -51,9 +44,7 @@ str
5144
40 | ((int)
5245
|
5346

54-
F722.py:39:11: F722 Syntax error in forward annotation: `
55-
((int)
56-
`
47+
F722.py:39:11: F722 Syntax error in forward annotation: unexpected EOF while parsing
5748
|
5849
37 | str
5950
38 | """
@@ -66,9 +57,7 @@ F722.py:39:11: F722 Syntax error in forward annotation: `
6657
43 | (int
6758
|
6859

69-
F722.py:42:11: F722 Syntax error in forward annotation: `
70-
(int
71-
`
60+
F722.py:42:11: F722 Syntax error in forward annotation: unexpected EOF while parsing
7261
|
7362
40 | ((int)
7463
41 | """

‎crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F722_F722_1.py.snap

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
---
22
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
3-
snapshot_kind: text
43
---
5-
F722_1.py:8:22: F722 Syntax error in forward annotation: `this isn't python`
4+
F722_1.py:8:22: F722 Syntax error in forward annotation: Unexpected token at the end of an expression
65
|
76
6 | @no_type_check
87
7 | class C:
@@ -11,7 +10,7 @@ F722_1.py:8:22: F722 Syntax error in forward annotation: `this isn't python`
1110
9 | x: "this also isn't python" = 1
1211
|
1312

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

23-
F722_1.py:9:12: F722 Syntax error in forward annotation: `this also isn't python`
22+
F722_1.py:9:12: F722 Syntax error in forward annotation: Unexpected token at the end of an expression
2423
|
2524
7 | class C:
2625
8 | def f(self, arg: "this isn't python") -> "this isn't python either":

‎crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ pub(crate) fn implicit_optional(checker: &mut Checker, parameters: &Parameters)
179179

180180
if let Expr::StringLiteral(string_expr) = annotation.as_ref() {
181181
// Quoted annotation.
182-
if let Some(parsed_annotation) = checker.parse_type_annotation(string_expr) {
182+
if let Ok(parsed_annotation) = checker.parse_type_annotation(string_expr) {
183183
let Some(expr) = type_hint_explicitly_allows_none(
184184
parsed_annotation.expression(),
185185
checker,

‎crates/ruff_linter/src/rules/ruff/typing.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ impl<'a> TypingTarget<'a> {
109109
Expr::NoneLiteral(_) => Some(TypingTarget::None),
110110
Expr::StringLiteral(string_expr) => checker
111111
.parse_type_annotation(string_expr)
112-
.as_ref()
112+
.ok()
113113
.map(|parsed_annotation| {
114114
TypingTarget::ForwardReference(parsed_annotation.expression())
115115
}),

0 commit comments

Comments
 (0)