From 11987f74652b76f0259c8a130badc21788457b5b Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Sat, 4 Jan 2025 04:00:39 +0000 Subject: [PATCH 1/2] [`ruff`] Recognize more expressions (`RUF027`) --- .../resources/test/fixtures/ruff/RUF027_3.py | 20 +++ crates/ruff_linter/src/rules/ruff/mod.rs | 1 + .../ruff/rules/missing_fstring_syntax.rs | 13 +- ...ules__ruff__tests__RUF027_RUF027_3.py.snap | 144 ++++++++++++++++++ 4 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF027_3.py create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_3.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_3.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_3.py new file mode 100644 index 0000000000000..0e50403360f43 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_3.py @@ -0,0 +1,20 @@ +foo = 0 + + +## Errors + +print("{foo()}") +print("{foo(non_existent)}") +print("{foo.baz}") +print("{foo['bar']}") + +print("{foo().qux}") +print("{foo[lorem].ipsum()}") +print("{foo.dolor[sit]().amet}") + + +## No errors + +print("{foo if consectetur else adipiscing}") +print("{[foo]}") +print("{ {foo} }") diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index e78c31f065c9b..9c1604c3c26a2 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -56,6 +56,7 @@ mod tests { #[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"))] + #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_3.py"))] #[test_case(Rule::InvalidFormatterSuppressionComment, Path::new("RUF028.py"))] #[test_case(Rule::UnusedAsync, Path::new("RUF029.py"))] #[test_case(Rule::AssertWithPrintMessage, Path::new("RUF030.py"))] 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 ca19c06346d23..c8a95aadc3ba6 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 @@ -204,7 +204,7 @@ fn should_be_fstring( for f_string in value.f_strings() { let mut has_name = false; for element in f_string.elements.expressions() { - if let ast::Expr::Name(ast::ExprName { id, .. }) = element.expression.as_ref() { + if let Some(id) = left_most_name(element.expression.as_ref()) { if arg_names.contains(id) { return false; } @@ -238,6 +238,17 @@ fn should_be_fstring( true } +#[inline] +fn left_most_name(expr: &ast::Expr) -> Option<&ast::name::Name> { + match expr { + ast::Expr::Name(ast::ExprName { id, .. }) => Some(id), + ast::Expr::Attribute(ast::ExprAttribute { value, .. }) => left_most_name(value), + ast::Expr::Call(ast::ExprCall { func, .. }) => left_most_name(func), + ast::Expr::Subscript(ast::ExprSubscript { value, .. }) => left_most_name(value), + _ => None, + } +} + // fast check to disqualify any string literal without brackets #[inline] fn has_brackets(possible_fstring: &str) -> bool { diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_3.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_3.py.snap new file mode 100644 index 0000000000000..7454bc530a20e --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_3.py.snap @@ -0,0 +1,144 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +snapshot_kind: text +--- +RUF027_3.py:6:7: RUF027 [*] Possible f-string without an `f` prefix + | +4 | ## Errors +5 | +6 | print("{foo()}") + | ^^^^^^^^^ RUF027 +7 | print("{foo(non_existent)}") +8 | print("{foo.baz}") + | + = help: Add `f` prefix + +ℹ Unsafe fix +3 3 | +4 4 | ## Errors +5 5 | +6 |-print("{foo()}") + 6 |+print(f"{foo()}") +7 7 | print("{foo(non_existent)}") +8 8 | print("{foo.baz}") +9 9 | print("{foo['bar']}") + +RUF027_3.py:7:7: RUF027 [*] Possible f-string without an `f` prefix + | +6 | print("{foo()}") +7 | print("{foo(non_existent)}") + | ^^^^^^^^^^^^^^^^^^^^^ RUF027 +8 | print("{foo.baz}") +9 | print("{foo['bar']}") + | + = help: Add `f` prefix + +ℹ Unsafe fix +4 4 | ## Errors +5 5 | +6 6 | print("{foo()}") +7 |-print("{foo(non_existent)}") + 7 |+print(f"{foo(non_existent)}") +8 8 | print("{foo.baz}") +9 9 | print("{foo['bar']}") +10 10 | + +RUF027_3.py:8:7: RUF027 [*] Possible f-string without an `f` prefix + | +6 | print("{foo()}") +7 | print("{foo(non_existent)}") +8 | print("{foo.baz}") + | ^^^^^^^^^^^ RUF027 +9 | print("{foo['bar']}") + | + = help: Add `f` prefix + +ℹ Unsafe fix +5 5 | +6 6 | print("{foo()}") +7 7 | print("{foo(non_existent)}") +8 |-print("{foo.baz}") + 8 |+print(f"{foo.baz}") +9 9 | print("{foo['bar']}") +10 10 | +11 11 | print("{foo().qux}") + +RUF027_3.py:9:7: RUF027 [*] Possible f-string without an `f` prefix + | + 7 | print("{foo(non_existent)}") + 8 | print("{foo.baz}") + 9 | print("{foo['bar']}") + | ^^^^^^^^^^^^^^ RUF027 +10 | +11 | print("{foo().qux}") + | + = help: Add `f` prefix + +ℹ Unsafe fix +6 6 | print("{foo()}") +7 7 | print("{foo(non_existent)}") +8 8 | print("{foo.baz}") +9 |-print("{foo['bar']}") + 9 |+print(f"{foo['bar']}") +10 10 | +11 11 | print("{foo().qux}") +12 12 | print("{foo[lorem].ipsum()}") + +RUF027_3.py:11:7: RUF027 [*] Possible f-string without an `f` prefix + | + 9 | print("{foo['bar']}") +10 | +11 | print("{foo().qux}") + | ^^^^^^^^^^^^^ RUF027 +12 | print("{foo[lorem].ipsum()}") +13 | print("{foo.dolor[sit]().amet}") + | + = help: Add `f` prefix + +ℹ Unsafe fix +8 8 | print("{foo.baz}") +9 9 | print("{foo['bar']}") +10 10 | +11 |-print("{foo().qux}") + 11 |+print(f"{foo().qux}") +12 12 | print("{foo[lorem].ipsum()}") +13 13 | print("{foo.dolor[sit]().amet}") +14 14 | + +RUF027_3.py:12:7: RUF027 [*] Possible f-string without an `f` prefix + | +11 | print("{foo().qux}") +12 | print("{foo[lorem].ipsum()}") + | ^^^^^^^^^^^^^^^^^^^^^^ RUF027 +13 | print("{foo.dolor[sit]().amet}") + | + = help: Add `f` prefix + +ℹ Unsafe fix +9 9 | print("{foo['bar']}") +10 10 | +11 11 | print("{foo().qux}") +12 |-print("{foo[lorem].ipsum()}") + 12 |+print(f"{foo[lorem].ipsum()}") +13 13 | print("{foo.dolor[sit]().amet}") +14 14 | +15 15 | + +RUF027_3.py:13:7: RUF027 [*] Possible f-string without an `f` prefix + | +11 | print("{foo().qux}") +12 | print("{foo[lorem].ipsum()}") +13 | print("{foo.dolor[sit]().amet}") + | ^^^^^^^^^^^^^^^^^^^^^^^^^ RUF027 + | + = help: Add `f` prefix + +ℹ Unsafe fix +10 10 | +11 11 | print("{foo().qux}") +12 12 | print("{foo[lorem].ipsum()}") +13 |-print("{foo.dolor[sit]().amet}") + 13 |+print(f"{foo.dolor[sit]().amet}") +14 14 | +15 15 | +16 16 | ## No errors From 5d2d6bbeb06daa0973a84746bbc5be5a48278da0 Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Sat, 4 Jan 2025 10:21:45 +0000 Subject: [PATCH 2/2] Avoid false negatives --- .../resources/test/fixtures/ruff/RUF027_3.py | 7 +++ .../ruff/rules/missing_fstring_syntax.rs | 26 ++++++----- ...ules__ruff__tests__RUF027_RUF027_3.py.snap | 46 +++++++++++++++++-- 3 files changed, 65 insertions(+), 14 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_3.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_3.py index 0e50403360f43..3816b1abf7095 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_3.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_3.py @@ -12,9 +12,16 @@ print("{foo[lorem].ipsum()}") print("{foo.dolor[sit]().amet}") +print("{id(foo)}") +print("{__path__}") + ## No errors print("{foo if consectetur else adipiscing}") print("{[foo]}") print("{ {foo} }") + +print("{id}") +print("{id.foo}") +print("{id[foo]}") 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 c8a95aadc3ba6..6586f4b571119 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 @@ -4,6 +4,7 @@ use rustc_hash::FxHashSet; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast as ast; +use ruff_python_ast::helpers::is_dunder; use ruff_python_literal::format::FormatSpec; use ruff_python_parser::parse_expression; use ruff_python_semantic::analyze::logging::is_logger_candidate; @@ -204,20 +205,23 @@ fn should_be_fstring( for f_string in value.f_strings() { let mut has_name = false; for element in f_string.elements.expressions() { - if let Some(id) = left_most_name(element.expression.as_ref()) { + let expr = element.expression.as_ref(); + if let Some(id) = left_most_name(expr) { if arg_names.contains(id) { return false; } - if semantic - // the parsed expression nodes have incorrect ranges - // so we need to use the range of the literal for the - // lookup in order to get reasonable results. - .simulate_runtime_load_at_location_in_scope( - id, - literal.range(), - semantic.scope_id, - ) - .map_or(true, |id| semantic.binding(id).kind.is_builtin()) + if !matches!(expr, ast::Expr::Call(_)) + && !is_dunder(id) + && semantic + // the parsed expression nodes have incorrect ranges + // so we need to use the range of the literal for the + // lookup in order to get reasonable results. + .simulate_runtime_load_at_location_in_scope( + id, + literal.range(), + semantic.scope_id, + ) + .map_or(true, |id| semantic.binding(id).kind.is_builtin()) { return false; } diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_3.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_3.py.snap index 7454bc530a20e..75d04159c01dc 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_3.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027_3.py.snap @@ -122,7 +122,7 @@ RUF027_3.py:12:7: RUF027 [*] Possible f-string without an `f` prefix 12 |+print(f"{foo[lorem].ipsum()}") 13 13 | print("{foo.dolor[sit]().amet}") 14 14 | -15 15 | +15 15 | print("{id(foo)}") RUF027_3.py:13:7: RUF027 [*] Possible f-string without an `f` prefix | @@ -130,6 +130,8 @@ RUF027_3.py:13:7: RUF027 [*] Possible f-string without an `f` prefix 12 | print("{foo[lorem].ipsum()}") 13 | print("{foo.dolor[sit]().amet}") | ^^^^^^^^^^^^^^^^^^^^^^^^^ RUF027 +14 | +15 | print("{id(foo)}") | = help: Add `f` prefix @@ -140,5 +142,43 @@ RUF027_3.py:13:7: RUF027 [*] Possible f-string without an `f` prefix 13 |-print("{foo.dolor[sit]().amet}") 13 |+print(f"{foo.dolor[sit]().amet}") 14 14 | -15 15 | -16 16 | ## No errors +15 15 | print("{id(foo)}") +16 16 | print("{__path__}") + +RUF027_3.py:15:7: RUF027 [*] Possible f-string without an `f` prefix + | +13 | print("{foo.dolor[sit]().amet}") +14 | +15 | print("{id(foo)}") + | ^^^^^^^^^^^ RUF027 +16 | print("{__path__}") + | + = help: Add `f` prefix + +ℹ Unsafe fix +12 12 | print("{foo[lorem].ipsum()}") +13 13 | print("{foo.dolor[sit]().amet}") +14 14 | +15 |-print("{id(foo)}") + 15 |+print(f"{id(foo)}") +16 16 | print("{__path__}") +17 17 | +18 18 | + +RUF027_3.py:16:7: RUF027 [*] Possible f-string without an `f` prefix + | +15 | print("{id(foo)}") +16 | print("{__path__}") + | ^^^^^^^^^^^^ RUF027 + | + = help: Add `f` prefix + +ℹ Unsafe fix +13 13 | print("{foo.dolor[sit]().amet}") +14 14 | +15 15 | print("{id(foo)}") +16 |-print("{__path__}") + 16 |+print(f"{__path__}") +17 17 | +18 18 | +19 19 | ## No errors