Skip to content

Commit

Permalink
Avoid consuming trailing whitespace during re-lexing (#11933)
Browse files Browse the repository at this point in the history
## Summary

This PR updates the re-lexing logic to avoid consuming the trailing
whitespace and move the lexer explicitly to the last newline character
encountered while moving backwards.

Consider the following code snippet as taken from the test case
highlighted with whitespace (`.`) and newline (`\n`) characters:
```py
# There are trailing whitespace before the newline character but those whitespaces are
# part of the comment token
f"""hello {x # comment....\n
#                     ^
y = 1\n
```

The parser is at `y` when it's trying to recover from an unclosed `{`,
so it calls into the re-lexing logic which tries to move the lexer back
to the end of the previous line. But, as it consumed all whitespaces it
moved the lexer to the location marked by `^` in the above code snippet.
But, those whitespaces are part of the comment token. This means that
the range for the two tokens were overlapping which introduced the
panic.

Note that this is only a bug when there's a comment with a trailing
whitespace otherwise it's fine to move the lexer to the whitespace
character. This is because the lexer would just skip the whitespace
otherwise. Nevertheless, this PR updates the logic to move it explicitly
to the newline character in all cases.

fixes: #11929 

## Test Plan

Add test cases and update the snapshot. Make sure that it doesn't panic
on the code snippet in the linked issue.
  • Loading branch information
dhruvmanila authored Jun 19, 2024
1 parent ff3bf58 commit cdc7c71
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,10 @@ def bar():

if call(f"hello
def bar():
pass
pass


# There are trailing whitespace before the newline character but those whitespaces are
# part of the comment token
f"""hello {x # comment
y = 1
15 changes: 7 additions & 8 deletions crates/ruff_python_parser/src/lexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1370,25 +1370,24 @@ impl<'src> Lexer<'src> {
// i.e., it recovered from an unclosed parenthesis (`(`, `[`, or `{`).
self.nesting -= 1;

let current_position = self.current_range().start();
let mut current_position = self.current_range().start();
let reverse_chars = self.source[..current_position.to_usize()].chars().rev();
let mut new_position = current_position;
let mut has_newline = false;
let mut newline_position = None;

for ch in reverse_chars {
if is_python_whitespace(ch) {
new_position -= ch.text_len();
current_position -= ch.text_len();
} else if matches!(ch, '\n' | '\r') {
has_newline |= true;
new_position -= ch.text_len();
current_position -= ch.text_len();
newline_position = Some(current_position);
} else {
break;
}
}

// The lexer should only be moved if there's a newline character which needs to be
// re-lexed.
if new_position != current_position && has_newline {
if let Some(newline_position) = newline_position {
// Earlier we reduced the nesting level unconditionally. Now that we know the lexer's
// position is going to be moved back, the lexer needs to be put back into a
// parenthesized context if the current token is a closing parenthesis.
Expand All @@ -1410,7 +1409,7 @@ impl<'src> Lexer<'src> {
}

self.cursor = Cursor::new(self.source);
self.cursor.skip_bytes(new_position.to_usize());
self.cursor.skip_bytes(newline_position.to_usize());
self.state = State::Other;
self.next_token();
true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ input_file: crates/ruff_python_parser/resources/invalid/re_lex_logical_token.py
```
Module(
ModModule {
range: 0..979,
range: 0..1129,
body: [
If(
StmtIf {
Expand Down Expand Up @@ -670,6 +670,53 @@ Module(
],
},
),
Expr(
StmtExpr {
range: 1097..1109,
value: FString(
ExprFString {
range: 1097..1109,
value: FStringValue {
inner: Single(
FString(
FString {
range: 1097..1109,
elements: [
Literal(
FStringLiteralElement {
range: 1101..1107,
value: "hello ",
},
),
Expression(
FStringExpressionElement {
range: 1107..1109,
expression: Name(
ExprName {
range: 1108..1109,
id: "x",
ctx: Load,
},
),
debug_text: None,
conversion: None,
format_spec: None,
},
),
],
flags: FStringFlags {
quote_style: Double,
prefix: Regular,
triple_quoted: true,
},
},
),
),
},
},
),
},
),
],
},
)
Expand Down Expand Up @@ -831,8 +878,45 @@ Module(


|
55 | if call(f"hello
56 | def bar():
57 | pass
| Syntax Error: Expected a statement
60 | # There are trailing whitespace before the newline character but those whitespaces are
61 | # part of the comment token
62 | f"""hello {x # comment
| Syntax Error: Expected a statement
63 | y = 1
|


|
60 | # There are trailing whitespace before the newline character but those whitespaces are
61 | # part of the comment token
62 | f"""hello {x # comment
| ___________________________^
63 | | y = 1
| |_____^ Syntax Error: f-string: unterminated triple-quoted string
|


|
61 | # part of the comment token
62 | f"""hello {x # comment
63 | y = 1
| ^ Syntax Error: f-string: expecting '}'
|


|
60 | # There are trailing whitespace before the newline character but those whitespaces are
61 | # part of the comment token
62 | f"""hello {x # comment
| ___________________________^
63 | | y = 1
| |_____^ Syntax Error: Expected FStringEnd, found Unknown
|


|
61 | # part of the comment token
62 | f"""hello {x # comment
63 | y = 1
| Syntax Error: Expected a statement
|

0 comments on commit cdc7c71

Please sign in to comment.