Implement RUF010 to detect explicit type conversions within f-strings (#4387)

crates/ruff/resources/test/fixtures/ruff/RUF010.py

@@ -0,0 +1,23 @@
+bla = b"bla"
+
+
+def foo(one_arg):
+    pass
+
+
+f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
+
+f"{foo(bla)}"  # OK
+
+f"{str(bla, 'ascii')}, {str(bla, encoding='cp1255')}"  # OK
+
+f"{bla!s} {[]!r} {'bar'!a}"  # OK
+
+"Not an f-string {str(bla)}, {repr(bla)}, {ascii(bla)}"  # OK
+
+
+def ascii(arg):
+    pass
+
+
+f"{ascii(bla)}"  # OK

crates/ruff/src/checkers/ast/mod.rs

@@ -3643,6 +3643,17 @@ where
                     flake8_simplify::rules::expr_and_false(self, expr);
                 }
             }
+            ExprKind::FormattedValue(ast::ExprFormattedValue {
+                value, conversion, ..
+            }) => {
+                if self
+                    .settings
+                    .rules
+                    .enabled(Rule::ExplicitFStringTypeConversion)
+                {
+                    ruff::rules::explicit_f_string_type_conversion(self, expr, value, *conversion);
+                }
+            }
             _ => {}
         };
 

crates/ruff/src/codes.rs

@@ -727,6 +727,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
         (Ruff, "007") => Rule::PairwiseOverZipped,
         (Ruff, "008") => Rule::MutableDataclassDefault,
         (Ruff, "009") => Rule::FunctionCallInDataclassDefaultArgument,
+        (Ruff, "010") => Rule::ExplicitFStringTypeConversion,
         (Ruff, "100") => Rule::UnusedNOQA,
 
         // flake8-django

crates/ruff/src/registry.rs

@@ -663,6 +663,7 @@ ruff_macros::register_rules!(
     rules::ruff::rules::PairwiseOverZipped,
     rules::ruff::rules::MutableDataclassDefault,
     rules::ruff::rules::FunctionCallInDataclassDefaultArgument,
+    rules::ruff::rules::ExplicitFStringTypeConversion,
     // flake8-django
     rules::flake8_django::rules::DjangoNullableModelStringField,
     rules::flake8_django::rules::DjangoLocalsInRenderFunction,

crates/ruff/src/rules/ruff/mod.rs

@@ -17,6 +17,7 @@ mod tests {
     use crate::test::test_path;
     use crate::{assert_messages, settings};
 
+    #[test_case(Rule::ExplicitFStringTypeConversion, Path::new("RUF010.py"); "RUF010")]
     #[test_case(Rule::CollectionLiteralConcatenation, Path::new("RUF005.py"); "RUF005")]
     #[test_case(Rule::AsyncioDanglingTask, Path::new("RUF006.py"); "RUF006")]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {

crates/ruff/src/rules/ruff/rules/explicit_f_string_type_conversion.rs

@@ -0,0 +1,113 @@
+use ruff_text_size::TextSize;
+use rustpython_parser::ast::{self, Expr, ExprKind};
+
+use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::helpers::unparse_expr;
+
+use crate::checkers::ast::Checker;
+use crate::registry::AsRule;
+
+/// ## What it does
+/// Checks for usages of `str()`, `repr()`, and `ascii()` as explicit type
+/// conversions within f-strings.
+///
+/// ## Why is this bad?
+/// f-strings support dedicated conversion flags for these types, which are
+/// more succinct and idiomatic.
+///
+/// ## Example
+/// ```python
+/// a = "some string"
+/// f"{repr(a)}"
+/// ```
+///
+/// Use instead:
+/// ```python
+/// a = "some string"
+/// f"{a!r}"
+/// ```
+#[violation]
+pub struct ExplicitFStringTypeConversion;
+
+impl AlwaysAutofixableViolation for ExplicitFStringTypeConversion {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Use conversion in f-string")
+    }
+
+    fn autofix_title(&self) -> String {
+        "Replace f-string function call with conversion".to_string()
+    }
+}
+
+/// RUF010
+pub(crate) fn explicit_f_string_type_conversion(
+    checker: &mut Checker,
+    expr: &Expr,
+    formatted_value: &Expr,
+    conversion: ast::Int,
+) {
+    // Skip if there's already a conversion flag.
+    if conversion != ast::ConversionFlag::None as u32 {
+        return;
+    }
+
+    let ExprKind::Call(ast::ExprCall {
+        func,
+        args,
+        keywords,
+    }) = &formatted_value.node else {
+        return;
+    };
+
+    // Can't be a conversion otherwise.
+    if args.len() != 1 || !keywords.is_empty() {
+        return;
+    }
+
+    let ExprKind::Name(ast::ExprName { id, .. }) = &func.node else {
+        return;
+    };
+
+    if !matches!(id.as_str(), "str" | "repr" | "ascii") {
+        return;
+    };
+
+    if !checker.ctx.is_builtin(id) {
+        return;
+    }
+
+    let mut diagnostic = Diagnostic::new(ExplicitFStringTypeConversion, formatted_value.range());
+
+    if checker.patch(diagnostic.kind.rule()) {
+        // Replace the call node with its argument and a conversion flag.
+        let mut conv_expr = expr.clone();
+        let ExprKind::FormattedValue(ast::ExprFormattedValue {
+            ref mut conversion,
+            ref mut value,
+            ..
+        }) = conv_expr.node else {
+            return;
+        };
+
+        *conversion = match id.as_str() {
+            "ascii" => ast::Int::new(ast::ConversionFlag::Ascii as u32),
+            "str" => ast::Int::new(ast::ConversionFlag::Str as u32),
+            "repr" => ast::Int::new(ast::ConversionFlag::Repr as u32),
+            &_ => unreachable!(),
+        };
+
+        value.node = args[0].node.clone();
+
+        diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
+            unparse_expr(&conv_expr, checker.stylist),
+            formatted_value
+                .range()
+                .sub_start(TextSize::from(1))
+                .add_end(TextSize::from(1)),
+        )));
+    }
+
+    checker.diagnostics.push(diagnostic);
+}

crates/ruff/src/rules/ruff/rules/mod.rs

@@ -2,6 +2,7 @@ mod ambiguous_unicode_character;
 mod asyncio_dangling_task;
 mod collection_literal_concatenation;
 mod confusables;
+mod explicit_f_string_type_conversion;
 mod mutable_defaults_in_dataclass_fields;
 mod pairwise_over_zipped;
 mod unused_noqa;
@@ -21,6 +22,10 @@ pub(crate) use mutable_defaults_in_dataclass_fields::{
 pub(crate) use pairwise_over_zipped::{pairwise_over_zipped, PairwiseOverZipped};
 pub(crate) use unused_noqa::{UnusedCodes, UnusedNOQA};
 
+pub(crate) use explicit_f_string_type_conversion::{
+    explicit_f_string_type_conversion, ExplicitFStringTypeConversion,
+};
+
 #[derive(Clone, Copy)]
 pub(crate) enum Context {
     String,

crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF010_RUF010.py.snap

@@ -0,0 +1,61 @@
+---
+source: crates/ruff/src/rules/ruff/mod.rs
+---
+RUF010.py:8:4: RUF010 [*] Use conversion in f-string
+   |
+ 8 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
+   |    ^^^^^^^^ RUF010
+ 9 | 
+10 | f"{foo(bla)}"  # OK
+   |
+   = help: Replace f-string function call with conversion
+
+ℹ Fix
+5 5 |     pass
+6 6 | 
+7 7 | 
+8   |-f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
+  8 |+f"{bla!s}, {repr(bla)}, {ascii(bla)}"  # RUF010
+9 9 | 
+10 10 | f"{foo(bla)}"  # OK
+11 11 | 
+
+RUF010.py:8:16: RUF010 [*] Use conversion in f-string
+   |
+ 8 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
+   |                ^^^^^^^^^ RUF010
+ 9 | 
+10 | f"{foo(bla)}"  # OK
+   |
+   = help: Replace f-string function call with conversion
+
+ℹ Fix
+5 5 |     pass
+6 6 | 
+7 7 | 
+8   |-f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
+  8 |+f"{str(bla)}, {bla!r}, {ascii(bla)}"  # RUF010
+9 9 | 
+10 10 | f"{foo(bla)}"  # OK
+11 11 | 
+
+RUF010.py:8:29: RUF010 [*] Use conversion in f-string
+   |
+ 8 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
+   |                             ^^^^^^^^^^ RUF010
+ 9 | 
+10 | f"{foo(bla)}"  # OK
+   |
+   = help: Replace f-string function call with conversion
+
+ℹ Fix
+5 5 |     pass
+6 6 | 
+7 7 | 
+8   |-f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
+  8 |+f"{str(bla)}, {repr(bla)}, {bla!a}"  # RUF010
+9 9 | 
+10 10 | f"{foo(bla)}"  # OK
+11 11 | 
+
+

ruff.schema.json

@@ -2169,6 +2169,8 @@
         "RUF007",
         "RUF008",
         "RUF009",
+        "RUF01",
+        "RUF010",
         "RUF1",
         "RUF10",
         "RUF100",