Skip to content

Commit

Permalink
Fix panic on RUF027 (astral-sh#9990)
Browse files Browse the repository at this point in the history
## Summary

Fixes astral-sh#9895 

The cause for this panic came from an offset error in the code. When
analyzing a hypothetical f-string, we attempt to re-parse it as an
f-string, and use the AST data to determine, among other things, whether
the format specifiers are correct. To determine the 'correctness' of a
format specifier, we actually have to re-parse the format specifier, and
this is where the issue lies. To get the source text for the specifier,
we were taking a slice from the original file source text... even though
the AST data for the specifier belongs to the standalone parsed f-string
expression, meaning that the ranges are going to be way off. In a file
with Unicode, this can cause panics if the slice is inside a char
boundary.

To fix this, we now slice from the temporary source we created earlier
to parse the literal as an f-string.

## Test Plan

The RUF027 snapshot test was amended to include a string with format
specifiers which we _should_ be calling out. This is to ensure we do
slice format specifiers from the source text correctly.
  • Loading branch information
snowsignal authored and nkxxll committed Mar 4, 2024
1 parent 4955d13 commit a935467
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 3 deletions.
4 changes: 4 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF027_0.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,7 @@ def method_calls():
first = "Wendy"
last = "Appleseed"
value.method("{first} {last}") # RUF027

def format_specifiers():
a = 4
b = "{a:b} {a:^5}"
2 changes: 2 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF027_2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# 测试eval函数,eval()函数用来执行一个字符串表达式,并返t表达式的值。另外,可以讲字符串转换成列表或元组或字典
a = "{1: 'a', 2: 'b'}"
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ mod tests {
#[test_case(Rule::DefaultFactoryKwarg, Path::new("RUF026.py"))]
#[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_0.py"))]
#[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_1.py"))]
#[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_2.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,10 @@ fn should_be_fstring(
return false;
}

let Ok(ast::Expr::FString(ast::ExprFString { value, .. })) =
parse_expression(&format!("f{}", locator.slice(literal.range())))
let fstring_expr = format!("f{}", locator.slice(literal));

// Note: Range offsets for `value` are based on `fstring_expr`
let Ok(ast::Expr::FString(ast::ExprFString { value, .. })) = parse_expression(&fstring_expr)
else {
return false;
};
Expand Down Expand Up @@ -159,7 +161,7 @@ fn should_be_fstring(
has_name = true;
}
if let Some(spec) = &element.format_spec {
let spec = locator.slice(spec.range());
let spec = &fstring_expr[spec.range()];
if FormatSpec::parse(spec).is_err() {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,8 @@ RUF027_0.py:70:18: RUF027 [*] Possible f-string without an `f` prefix
69 | last = "Appleseed"
70 | value.method("{first} {last}") # RUF027
| ^^^^^^^^^^^^^^^^ RUF027
71 |
72 | def format_specifiers():
|
= help: Add `f` prefix

Expand All @@ -294,5 +296,24 @@ RUF027_0.py:70:18: RUF027 [*] Possible f-string without an `f` prefix
69 69 | last = "Appleseed"
70 |- value.method("{first} {last}") # RUF027
70 |+ value.method(f"{first} {last}") # RUF027
71 71 |
72 72 | def format_specifiers():
73 73 | a = 4

RUF027_0.py:74:9: RUF027 [*] Possible f-string without an `f` prefix
|
72 | def format_specifiers():
73 | a = 4
74 | b = "{a:b} {a:^5}"
| ^^^^^^^^^^^^^^ RUF027
|
= help: Add `f` prefix

Unsafe fix
71 71 |
72 72 | def format_specifiers():
73 73 | a = 4
74 |- b = "{a:b} {a:^5}"
74 |+ b = f"{a:b} {a:^5}"


Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---

0 comments on commit a935467

Please sign in to comment.