Fix RUF010 auto-fix with parenthesis (#4524)

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

@@ -10,6 +10,8 @@ def foo(one_arg):
 
 f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
 
+f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+
 f"{foo(bla)}"  # OK
 
 f"{str(bla, 'ascii')}, {str(bla, encoding='cp1255')}"  # OK

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

@@ -3360,6 +3360,13 @@ where
                 if self.settings.rules.enabled(Rule::HardcodedSQLExpression) {
                     flake8_bandit::rules::hardcoded_sql_expression(self, expr);
                 }
+                if self
+                    .settings
+                    .rules
+                    .enabled(Rule::ExplicitFStringTypeConversion)
+                {
+                    ruff::rules::explicit_f_string_type_conversion(self, expr, values);
+                }
             }
             Expr::BinOp(ast::ExprBinOp {
                 left,
@@ -3859,17 +3866,6 @@ where
                     flake8_simplify::rules::expr_and_false(self, expr);
                 }
             }
-            Expr::FormattedValue(ast::ExprFormattedValue {
-                value, conversion, ..
-            }) => {
-                if self
-                    .settings
-                    .rules
-                    .enabled(Rule::ExplicitFStringTypeConversion)
-                {
-                    ruff::rules::explicit_f_string_type_conversion(self, value, *conversion);
-                }
-            }
             _ => {}
         };
 

crates/ruff/src/cst/matchers.rs

@@ -1,7 +1,8 @@
 use anyhow::{bail, Result};
 use libcst_native::{
-    Attribute, Call, Comparison, Dict, Expr, Expression, Import, ImportAlias, ImportFrom,
-    ImportNames, Module, SimpleString, SmallStatement, Statement,
+    Attribute, Call, Comparison, Dict, Expr, Expression, FormattedString, FormattedStringContent,
+    FormattedStringExpression, Import, ImportAlias, ImportFrom, ImportNames, Module, Name,
+    SimpleString, SmallStatement, Statement,
 };
 
 pub(crate) fn match_module(module_text: &str) -> Result<Module> {
@@ -111,3 +112,33 @@ pub(crate) fn match_simple_string<'a, 'b>(
         bail!("Expected Expression::SimpleString")
     }
 }
+
+pub(crate) fn match_formatted_string<'a, 'b>(
+    expression: &'a mut Expression<'b>,
+) -> Result<&'a mut FormattedString<'b>> {
+    if let Expression::FormattedString(formatted_string) = expression {
+        Ok(formatted_string)
+    } else {
+        bail!("Expected Expression::FormattedString")
+    }
+}
+
+pub(crate) fn match_formatted_string_expression<'a, 'b>(
+    formatted_string_content: &'a mut FormattedStringContent<'b>,
+) -> Result<&'a mut FormattedStringExpression<'b>> {
+    if let FormattedStringContent::Expression(formatted_string_expression) =
+        formatted_string_content
+    {
+        Ok(formatted_string_expression)
+    } else {
+        bail!("Expected FormattedStringContent::Expression")
+    }
+}
+
+pub(crate) fn match_name<'a, 'b>(expression: &'a mut Expression<'b>) -> Result<&'a mut Name<'b>> {
+    if let Expression::Name(name) = expression {
+        Ok(name)
+    } else {
+        bail!("Expected Expression::Name")
+    }
+}

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

@@ -1,9 +1,16 @@
-use rustpython_parser::ast::{self, ConversionFlag, Expr, Ranged};
+use anyhow::{bail, Result};
+use libcst_native::{Codegen, CodegenState};
+use rustpython_parser::ast::{self, Expr, Ranged};
 
 use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::source_code::{Locator, Stylist};
 
 use crate::checkers::ast::Checker;
+use crate::cst::matchers::{
+    match_call, match_expression, match_formatted_string, match_formatted_string_expression,
+    match_name,
+};
 use crate::registry::AsRule;
 
 /// ## What it does
@@ -39,59 +46,101 @@ impl AlwaysAutofixableViolation for ExplicitFStringTypeConversion {
     }
 }
 
+fn fix_explicit_f_string_type_conversion(
+    expr: &Expr,
+    index: usize,
+    locator: &Locator,
+    stylist: &Stylist,
+) -> Result<Fix> {
+    // Replace the call node with its argument and a conversion flag.
+    let range = expr.range();
+    let content = locator.slice(range);
+    let mut expression = match_expression(content)?;
+    let formatted_string = match_formatted_string(&mut expression)?;
+
+    // Replace the formatted call expression at `index` with a conversion flag.
+    let mut formatted_string_expression =
+        match_formatted_string_expression(&mut formatted_string.parts[index])?;
+    let call = match_call(&mut formatted_string_expression.expression)?;
+    let name = match_name(&mut call.func)?;
+    match name.value {
+        "str" => {
+            formatted_string_expression.conversion = Some("s");
+        }
+        "repr" => {
+            formatted_string_expression.conversion = Some("r");
+        }
+        "ascii" => {
+            formatted_string_expression.conversion = Some("a");
+        }
+        _ => bail!("Unexpected function call: `{:?}`", name.value),
+    }
+    formatted_string_expression.expression = call.args[0].value.clone();
+
+    let mut state = CodegenState {
+        default_newline: &stylist.line_ending(),
+        default_indent: stylist.indentation(),
+        ..CodegenState::default()
+    };
+    expression.codegen(&mut state);
+
+    Ok(Fix::automatic(Edit::range_replacement(
+        state.to_string(),
+        range,
+    )))
+}
+
 /// RUF010
 pub(crate) fn explicit_f_string_type_conversion(
     checker: &mut Checker,
-    formatted_value: &Expr,
-    conversion: ConversionFlag,
+    expr: &Expr,
+    values: &[Expr],
 ) {
-    // Skip if there's already a conversion flag.
-    if !conversion.is_none() {
-        return;
-    }
+    for (index, formatted_value) in values.iter().enumerate() {
+        let Expr::FormattedValue(ast::ExprFormattedValue {
+            conversion,
+            value,
+            ..
+        }) = &formatted_value else {
+            continue;
+        };
+        // Skip if there's already a conversion flag.
+        if !conversion.is_none() {
+            return;
+        }
 
-    let Expr::Call(ast::ExprCall {
-        func,
-        args,
-        keywords,
-        range: _,
-    }) = formatted_value else {
-        return;
-    };
+        let Expr::Call(ast::ExprCall {
+            func,
+            args,
+            keywords,
+            ..
+        }) = value.as_ref() else {
+            return;
+        };
 
-    // Can't be a conversion otherwise.
-    if args.len() != 1 || !keywords.is_empty() {
-        return;
-    }
+        // Can't be a conversion otherwise.
+        if args.len() != 1 || !keywords.is_empty() {
+            return;
+        }
 
-    let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else {
-        return;
-    };
+        let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else {
+            return;
+        };
 
-    let conversion = match id.as_str() {
-        "ascii" => 'a',
-        "str" => 's',
-        "repr" => 'r',
-        _ => return,
-    };
+        if !matches!(id.as_str(), "str" | "repr" | "ascii") {
+            return;
+        };
 
-    if !checker.ctx.is_builtin(id) {
-        return;
-    }
+        if !checker.ctx.is_builtin(id) {
+            return;
+        }
 
-    let formatted_value_range = formatted_value.range();
-    let mut diagnostic = Diagnostic::new(ExplicitFStringTypeConversion, formatted_value_range);
-
-    if checker.patch(diagnostic.kind.rule()) {
-        let arg_range = args[0].range();
-        let remove_call = Edit::deletion(formatted_value_range.start(), arg_range.start());
-        let add_conversion = Edit::replacement(
-            format!("!{conversion}"),
-            arg_range.end(),
-            formatted_value_range.end(),
-        );
-        diagnostic.set_fix(Fix::automatic_edits(remove_call, [add_conversion]));
+        let mut diagnostic = Diagnostic::new(ExplicitFStringTypeConversion, value.range());
+        if checker.patch(diagnostic.kind.rule()) {
+            diagnostic.try_set_fix(|| {
+                fix_explicit_f_string_type_conversion(expr, index, checker.locator, checker.stylist)
+            });
+        }
+        checker.diagnostics.push(diagnostic);
     }
-
-    checker.diagnostics.push(diagnostic);
 }

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

@@ -65,7 +65,7 @@ RUF010.py:11:4: RUF010 [*] Use conversion in f-string
 13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
    |    ^^^^^^^^^^^ RUF010
 14 | 
-15 | f"{foo(bla)}"  # OK
+15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
    |
    = help: Replace f-string function call with conversion
 
@@ -76,7 +76,7 @@ RUF010.py:11:4: RUF010 [*] Use conversion in f-string
 11    |-f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
    11 |+f"{d['a']!s}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
 12 12 | 
-13 13 | f"{foo(bla)}"  # OK
+13 13 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
 14 14 | 
 
 RUF010.py:11:19: RUF010 [*] Use conversion in f-string
@@ -86,7 +86,7 @@ RUF010.py:11:19: RUF010 [*] Use conversion in f-string
 13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
    |                   ^^^^^^^^^^^^ RUF010
 14 | 
-15 | f"{foo(bla)}"  # OK
+15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
    |
    = help: Replace f-string function call with conversion
 
@@ -97,7 +97,7 @@ RUF010.py:11:19: RUF010 [*] Use conversion in f-string
 11    |-f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
    11 |+f"{str(d['a'])}, {d['b']!r}, {ascii(d['c'])}"  # RUF010
 12 12 | 
-13 13 | f"{foo(bla)}"  # OK
+13 13 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
 14 14 | 
 
 RUF010.py:11:35: RUF010 [*] Use conversion in f-string
@@ -107,7 +107,7 @@ RUF010.py:11:35: RUF010 [*] Use conversion in f-string
 13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
    |                                   ^^^^^^^^^^^^^ RUF010
 14 | 
-15 | f"{foo(bla)}"  # OK
+15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
    |
    = help: Replace f-string function call with conversion
 
@@ -118,7 +118,70 @@ RUF010.py:11:35: RUF010 [*] Use conversion in f-string
 11    |-f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
    11 |+f"{str(d['a'])}, {repr(d['b'])}, {d['c']!a}"  # RUF010
 12 12 | 
-13 13 | f"{foo(bla)}"  # OK
+13 13 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
 14 14 | 
 
+RUF010.py:13:5: RUF010 [*] Use conversion in f-string
+   |
+13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
+14 | 
+15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+   |     ^^^^^^^^ RUF010
+16 | 
+17 | f"{foo(bla)}"  # OK
+   |
+   = help: Replace f-string function call with conversion
+
+ℹ Fix
+10 10 | 
+11 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
+12 12 | 
+13    |-f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+   13 |+f"{bla!s}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+14 14 | 
+15 15 | f"{foo(bla)}"  # OK
+16 16 | 
+
+RUF010.py:13:19: RUF010 [*] Use conversion in f-string
+   |
+13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
+14 | 
+15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+   |                   ^^^^^^^^^ RUF010
+16 | 
+17 | f"{foo(bla)}"  # OK
+   |
+   = help: Replace f-string function call with conversion
+
+ℹ Fix
+10 10 | 
+11 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
+12 12 | 
+13    |-f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+   13 |+f"{(str(bla))}, {bla!r}, {(ascii(bla))}"  # RUF010
+14 14 | 
+15 15 | f"{foo(bla)}"  # OK
+16 16 | 
+
+RUF010.py:13:34: RUF010 [*] Use conversion in f-string
+   |
+13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
+14 | 
+15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+   |                                  ^^^^^^^^^^ RUF010
+16 | 
+17 | f"{foo(bla)}"  # OK
+   |
+   = help: Replace f-string function call with conversion
+
+ℹ Fix
+10 10 | 
+11 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
+12 12 | 
+13    |-f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+   13 |+f"{(str(bla))}, {(repr(bla))}, {bla!a}"  # RUF010
+14 14 | 
+15 15 | f"{foo(bla)}"  # OK
+16 16 | 
+