From a935467229f023763322d1ed92ed0015048b844e Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Fri, 16 Feb 2024 12:04:39 -0800 Subject: [PATCH] Fix panic on RUF027 (#9990) ## Summary Fixes #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. --- .../resources/test/fixtures/ruff/RUF027_0.py | 4 ++++ .../resources/test/fixtures/ruff/RUF027_2.py | 2 ++ crates/ruff_linter/src/rules/ruff/mod.rs | 1 + .../ruff/rules/missing_fstring_syntax.rs | 8 ++++--- ...ules__ruff__tests__RUF027_RUF027_0.py.snap | 21 +++++++++++++++++++ ...ules__ruff__tests__RUF027_RUF027_2.py.snap | 4 ++++ 6 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF027_2.py create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_2.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_0.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_0.py index 4d9ecd2c49f16..4780348c121bd 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_0.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_0.py @@ -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}" diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_2.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_2.py new file mode 100644 index 0000000000000..4c90f8bc8c166 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_2.py @@ -0,0 +1,2 @@ +# 测试eval函数,eval()函数用来执行一个字符串表达式，并返t表达式的值。另外，可以讲字符串转换成列表或元组或字典 +a = "{1: 'a', 2: 'b'}" \ No newline at end of file diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 7c68c805e1499..7239323bcbddb 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -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( diff --git a/crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs b/crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs index 7cfc2e7bf01b3..3e11577e8b972 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs @@ -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; }; @@ -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; } diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_0.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_0.py.snap index 2a3447006e433..ca858cdcda640 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_0.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_0.py.snap @@ -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 @@ -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}" diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_2.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_2.py.snap new file mode 100644 index 0000000000000..7f58cfd7246a3 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_2.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +