Consider unterminated f-strings in FStringRanges (#8154)

## Summary

This PR removes the `debug_assertion` in the `Indexer` to allow
unterminated f-strings. This is mainly a fix in the development build
which now matches the release build.

The fix is simple: remove the `debug_assertion` which means that the
there could be `FStringStart` and possibly `FStringMiddle` tokens
without a corresponding f-string range in the `Indexer`. This means that
the code requesting for the f-string index need to account for the
`None` case, making the code safer.

This also updates the code which queries the `FStringRanges` to account
for the `None` case. This will happen when the `FStringStart` /
`FStringMiddle` tokens are present but the `FStringEnd` token isn't
which means that the `Indexer` won't contain the range for that
f-string.

## Test Plan

`cargo test`

Taking the following code as an example:

```python
f"{123}
```

This only emits a `FStringStart` token, but no `FStringMiddle` or
`FStringEnd` tokens.

And,

```python
f"\.png${
```

This emits a `FStringStart` and `FStringMiddle` token, but no
`FStringEnd` token.

fixes: #8065
This commit is contained in:
Dhruv Manilawala 2023-10-27 16:41:44 +05:30 committed by GitHub
parent cd8e1bad64
commit 097e703071
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 32 additions and 29 deletions

View file

@ -110,18 +110,27 @@ pub(crate) fn implicit(
{ {
let (a_range, b_range) = match (a_tok, b_tok) { let (a_range, b_range) = match (a_tok, b_tok) {
(Tok::String { .. }, Tok::String { .. }) => (*a_range, *b_range), (Tok::String { .. }, Tok::String { .. }) => (*a_range, *b_range),
(Tok::String { .. }, Tok::FStringStart) => ( (Tok::String { .. }, Tok::FStringStart) => {
*a_range, match indexer.fstring_ranges().innermost(b_range.start()) {
indexer.fstring_ranges().innermost(b_range.start()).unwrap(), Some(b_range) => (*a_range, b_range),
), None => continue,
(Tok::FStringEnd, Tok::String { .. }) => ( }
indexer.fstring_ranges().innermost(a_range.start()).unwrap(), }
*b_range, (Tok::FStringEnd, Tok::String { .. }) => {
), match indexer.fstring_ranges().innermost(a_range.start()) {
(Tok::FStringEnd, Tok::FStringStart) => ( Some(a_range) => (a_range, *b_range),
indexer.fstring_ranges().innermost(a_range.start()).unwrap(), None => continue,
indexer.fstring_ranges().innermost(b_range.start()).unwrap(), }
), }
(Tok::FStringEnd, Tok::FStringStart) => {
match (
indexer.fstring_ranges().innermost(a_range.start()),
indexer.fstring_ranges().innermost(b_range.start()),
) {
(Some(a_range), Some(b_range)) => (a_range, b_range),
_ => continue,
}
}
_ => continue, _ => continue,
}; };

View file

@ -78,18 +78,21 @@ pub(crate) fn invalid_escape_sequence(
token: &Tok, token: &Tok,
token_range: TextRange, token_range: TextRange,
) { ) {
let token_source_code = match token { let (token_source_code, string_start_location) = match token {
Tok::FStringMiddle { value, is_raw } => { Tok::FStringMiddle { value, is_raw } => {
if *is_raw { if *is_raw {
return; return;
} }
value.as_str() let Some(range) = indexer.fstring_ranges().innermost(token_range.start()) else {
return;
};
(value.as_str(), range.start())
} }
Tok::String { kind, .. } => { Tok::String { kind, .. } => {
if kind.is_raw() { if kind.is_raw() {
return; return;
} }
locator.slice(token_range) (locator.slice(token_range), token_range.start())
} }
_ => return, _ => return,
}; };
@ -206,17 +209,6 @@ pub(crate) fn invalid_escape_sequence(
invalid_escape_sequence.push(diagnostic); invalid_escape_sequence.push(diagnostic);
} }
} else { } else {
let tok_start = if token.is_f_string_middle() {
// SAFETY: If this is a `FStringMiddle` token, then the indexer
// must have the f-string range.
indexer
.fstring_ranges()
.innermost(token_range.start())
.unwrap()
.start()
} else {
token_range.start()
};
// Turn into raw string. // Turn into raw string.
for invalid_escape_char in &invalid_escape_chars { for invalid_escape_char in &invalid_escape_chars {
let diagnostic = Diagnostic::new( let diagnostic = Diagnostic::new(
@ -231,8 +223,8 @@ pub(crate) fn invalid_escape_sequence(
// `assert`, etc.) and the string. For example, `return"foo"` is valid, but // `assert`, etc.) and the string. For example, `return"foo"` is valid, but
// `returnr"foo"` is not. // `returnr"foo"` is not.
Fix::safe_edit(Edit::insertion( Fix::safe_edit(Edit::insertion(
pad_start("r".to_string(), tok_start, locator), pad_start("r".to_string(), string_start_location, locator),
tok_start, string_start_location,
)), )),
); );
invalid_escape_sequence.push(diagnostic); invalid_escape_sequence.push(diagnostic);

View file

@ -5,8 +5,11 @@ use ruff_text_size::{TextRange, TextSize};
/// Stores the ranges of all f-strings in a file sorted by [`TextRange::start`]. /// Stores the ranges of all f-strings in a file sorted by [`TextRange::start`].
/// There can be multiple overlapping ranges for nested f-strings. /// There can be multiple overlapping ranges for nested f-strings.
///
/// Note that the ranges for all unterminated f-strings are not stored.
#[derive(Debug)] #[derive(Debug)]
pub struct FStringRanges { pub struct FStringRanges {
// Mapping from the f-string start location to its range.
raw: BTreeMap<TextSize, TextRange>, raw: BTreeMap<TextSize, TextRange>,
} }
@ -89,7 +92,6 @@ impl FStringRangesBuilder {
} }
pub(crate) fn finish(self) -> FStringRanges { pub(crate) fn finish(self) -> FStringRanges {
debug_assert!(self.start_locations.is_empty());
FStringRanges { raw: self.raw } FStringRanges { raw: self.raw }
} }
} }