Skip to content

Commit 6b46d0d

Browse files
youknowoneyouknowone
committed
Refine replacement field, comma and string decode error reporting
Add `ParseErrorType::ExpectedIdentifier` for a missing identifier. Treat only missing expressions and identifiers as making an expression incomplete, require a complete prefix of the second expression for the missing comma error, keep string decode errors read before an unknown token, and report them ahead of an unclosed bracket. Assisted-by: Claude Code:claude-opus-5-5
1 parent 060b62d commit 6b46d0d

6 files changed

Lines changed: 188 additions & 15 deletions

File tree

‎crates/ruff_python_parser/resources/inline/err/missing_comma_before_invalid_expression.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[a ~]
2+
[a b +]

‎crates/ruff_python_parser/src/error.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,8 @@ pub enum ParseErrorType {
288288
ExpectedImaginaryNumber,
289289
/// Expected an expression at the current parser location.
290290
ExpectedExpression,
291+
/// Expected an identifier at the current parser location.
292+
ExpectedIdentifier,
291293
/// The parser expected a specific token that was not found.
292294
ExpectedToken {
293295
expected: TokenKind,
@@ -579,7 +581,9 @@ impl std::fmt::Display for ParseErrorType {
579581
ParseErrorType::ExpectedImaginaryNumber => {
580582
f.write_str("expected an imaginary number in complex literal pattern")
581583
}
582-
ParseErrorType::ExpectedExpression => f.write_str("invalid syntax"),
584+
ParseErrorType::ExpectedExpression | ParseErrorType::ExpectedIdentifier => {
585+
f.write_str("invalid syntax")
586+
}
583587
ParseErrorType::UnexpectedIndentation => f.write_str("unexpected indent"),
584588
ParseErrorType::InvalidAssignmentTarget {
585589
kind,

‎crates/ruff_python_parser/src/parser/expression.rs‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -439,10 +439,18 @@ impl<'src> Parser<'src> {
439439
return;
440440
}
441441

442+
// test_err missing_comma_before_invalid_expression
443+
// [a ~]
444+
// [a b +]
442445
let checkpoint = self.checkpoint();
446+
let errors_before = self.errors.len();
443447
let second = self.parse_conditional_expression_or_higher();
444-
let range = TextRange::new(first.start(), second.end());
448+
let second_end = self.complete_prefix_end(&second.expr, errors_before);
445449
self.rewind(checkpoint);
450+
let Some(second_end) = second_end else {
451+
return;
452+
};
453+
let range = TextRange::new(first.start(), second_end);
446454
self.add_error(
447455
ParseErrorType::OtherError("invalid syntax. Perhaps you forgot a comma?".to_string()),
448456
range,
@@ -798,7 +806,7 @@ impl<'src> Parser<'src> {
798806

799807
if self.current_token_kind().is_keyword() {
800808
// Non-soft keyword
801-
self.add_error(ParseErrorType::OtherError("invalid syntax".into()), range);
809+
self.add_error(ParseErrorType::ExpectedIdentifier, range);
802810

803811
let text = self.src_text(range);
804812
let id = self.intern_name(text);
@@ -814,10 +822,7 @@ impl<'src> Parser<'src> {
814822
}
815823

816824
fn parse_missing_identifier(&mut self) -> ast::Identifier {
817-
self.add_error(
818-
ParseErrorType::OtherError("invalid syntax".into()),
819-
self.current_token_range(),
820-
);
825+
self.add_error(ParseErrorType::ExpectedIdentifier, self.current_token_range());
821826

822827
ast::Identifier {
823828
id: Name::empty(),
@@ -2119,7 +2124,12 @@ impl<'src> Parser<'src> {
21192124
return;
21202125
}
21212126
let (error, range) = match self.errors.get(errors_before) {
2122-
Some(first) if matches!(first.error, ParseErrorType::ExpectedExpression) => {
2127+
Some(first)
2128+
if matches!(
2129+
first.error,
2130+
ParseErrorType::ExpectedExpression | ParseErrorType::ExpectedIdentifier
2131+
) =>
2132+
{
21232133
match self.complete_prefix_end(expr, errors_before) {
21242134
None => (
21252135
InterpolatedStringErrorType::ExpectedExpressionAfterLbrace,
@@ -2148,13 +2158,16 @@ impl<'src> Parser<'src> {
21482158
}
21492159

21502160
/// Returns the end of the longest prefix of `expr` that is a complete expression, where an
2151-
/// expression is incomplete if one of the errors after the first `errors_before` errors is
2152-
/// inside it, or `None` if no prefix is complete.
2161+
/// expression is incomplete if a missing expression or identifier after the first
2162+
/// `errors_before` errors is inside it, or `None` if no prefix is complete.
21532163
fn complete_prefix_end(&self, expr: &Expr, errors_before: usize) -> Option<TextSize> {
21542164
let is_complete = |expr: &Expr| {
2155-
!self.errors[errors_before..]
2156-
.iter()
2157-
.any(|error| expr.range().contains_inclusive(error.location.start()))
2165+
!self.errors[errors_before..].iter().any(|error| {
2166+
matches!(
2167+
error.error,
2168+
ParseErrorType::ExpectedExpression | ParseErrorType::ExpectedIdentifier
2169+
) && expr.range().contains_inclusive(error.location.start())
2170+
})
21582171
};
21592172
if is_complete(expr) {
21602173
return Some(expr.end());

‎crates/ruff_python_parser/src/parser/mod.rs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -885,8 +885,9 @@ impl<'src> Parser<'src> {
885885
}
886886
}
887887

888-
// The lexer already reported an error for the current token, which comes first.
889-
if self.at(TokenKind::Unknown) {
888+
// The lexer already reported an error for the current token, which comes first. A
889+
// string literal that fails to decode was read before it.
890+
if self.at(TokenKind::Unknown) && !matches!(error, ParseErrorType::Lexical(_)) {
890891
return;
891892
}
892893

@@ -2043,8 +2044,17 @@ fn prioritize_tokenizer_error(
20432044
}
20442045
_ => first_start,
20452046
};
2047+
// An escape sequence error is found when its string is parsed, before the end of the
2048+
// source shows an unclosed bracket.
2049+
let escape_error = matches!(
2050+
first.error,
2051+
ParseErrorType::Lexical(
2052+
LexicalErrorType::UnicodeEscapeError { .. } | LexicalErrorType::BytesEscapeError { .. }
2053+
)
2054+
);
20462055
let wins = detected <= first_start
20472056
|| (!in_interpolated_string
2057+
&& !escape_error
20482058
&& match tokenizer_error.error() {
20492059
LexicalErrorType::UnclosedBracket { .. } => {
20502060
let bracket_start = tokenizer_error.location().start();

‎crates/ruff_python_parser/tests/snapshots/invalid_syntax@f_string_expected_separator_after_expression.py.snap‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,15 @@ Module(
557557
|
558558

559559

560+
|
561+
2 | f"{1 +}"
562+
3 | f"{a, b and}"
563+
4 | f"{x.}"
564+
| ^ Syntax Error: f-string: expecting '=', or '!', or ':', or '}'
565+
5 | A tokenizer error up to where the expression fails comes first.
566+
|
567+
568+
560569
|
561570
2 | f"{1 +}"
562571
3 | f"{a, b and}"

‎crates/ruff_python_parser/tests/snapshots/invalid_syntax@missing_comma_before_invalid_expression.py.snap‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
---
2+
source: crates/ruff_python_parser/tests/fixtures.rs
3+
input_file: crates/ruff_python_parser/resources/inline/err/missing_comma_before_invalid_expression.py
4+
---
5+
## AST
6+
7+
```
8+
Module(
9+
ModModule {
10+
node_index: NodeIndex(None),
11+
range: 0..14,
12+
body: [
13+
Expr(
14+
StmtExpr {
15+
node_index: NodeIndex(None),
16+
range: 0..5,
17+
value: List(
18+
ExprList {
19+
node_index: NodeIndex(None),
20+
range: 0..5,
21+
elts: [
22+
Name(
23+
ExprName {
24+
node_index: NodeIndex(None),
25+
range: 1..2,
26+
id: Name("a"),
27+
ctx: Load,
28+
},
29+
),
30+
UnaryOp(
31+
ExprUnaryOp {
32+
node_index: NodeIndex(None),
33+
range: 3..4,
34+
op: Invert,
35+
operand: Name(
36+
ExprName {
37+
node_index: NodeIndex(None),
38+
range: 4..4,
39+
id: Name(""),
40+
ctx: Invalid,
41+
},
42+
),
43+
},
44+
),
45+
],
46+
ctx: Load,
47+
runtime_elts: None,
48+
},
49+
),
50+
},
51+
),
52+
Expr(
53+
StmtExpr {
54+
node_index: NodeIndex(None),
55+
range: 6..13,
56+
value: List(
57+
ExprList {
58+
node_index: NodeIndex(None),
59+
range: 6..13,
60+
elts: [
61+
Name(
62+
ExprName {
63+
node_index: NodeIndex(None),
64+
range: 7..8,
65+
id: Name("a"),
66+
ctx: Load,
67+
},
68+
),
69+
BinOp(
70+
ExprBinOp {
71+
node_index: NodeIndex(None),
72+
range: 9..12,
73+
left: Name(
74+
ExprName {
75+
node_index: NodeIndex(None),
76+
range: 9..10,
77+
id: Name("b"),
78+
ctx: Load,
79+
},
80+
),
81+
op: Add,
82+
right: Name(
83+
ExprName {
84+
node_index: NodeIndex(None),
85+
range: 12..12,
86+
id: Name(""),
87+
ctx: Invalid,
88+
},
89+
),
90+
},
91+
),
92+
],
93+
ctx: Load,
94+
runtime_elts: None,
95+
},
96+
),
97+
},
98+
),
99+
],
100+
runtime_body: None,
101+
},
102+
)
103+
```
104+
## Errors
105+
106+
|
107+
1 | [a ~]
108+
| ^ Syntax Error: invalid syntax
109+
2 | [a b +]
110+
|
111+
112+
113+
|
114+
1 | [a ~]
115+
| ^ Syntax Error: invalid syntax
116+
2 | [a b +]
117+
|
118+
119+
120+
|
121+
1 | [a ~]
122+
2 | [a b +]
123+
| ^^^ Syntax Error: invalid syntax. Perhaps you forgot a comma?
124+
125+
126+
|
127+
1 | [a ~]
128+
2 | [a b +]
129+
| ^ Syntax Error: invalid syntax
130+
131+
132+
|
133+
1 | [a ~]
134+
2 | [a b +]
135+
| ^ Syntax Error: invalid syntax

0 commit comments

Comments
 (0)