Upgrade explicit-type-conversion rule (RUF010) to remove unnecessary str calls (#4971)

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

@@ -34,3 +34,12 @@ f"{ascii(bla)}"  # OK
     " intermediary content "
     f" that flows {repr(obj)} of type {type(obj)}.{additional_message}"  # RUF010
 )
+
+
+f"{str(bla)}"  # RUF010
+
+f"{str(bla):20}"  # RUF010
+
+f"{bla!s}"  # RUF010
+
+f"{bla!s:20}"  # OK

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

@@ -6,6 +6,7 @@ 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::prelude::ConversionFlag;
 use ruff_python_ast::source_code::{Locator, Stylist};
 
 use crate::autofix::codemods::CodegenStylist;
@@ -21,31 +22,62 @@ use crate::registry::AsRule;
 /// f-strings support dedicated conversion flags for these types, which are
 /// more succinct and idiomatic.
 ///
+/// In the case of `str()`, it's also redundant, since `str()` is the default
+/// conversion.
+///
 /// ## Example
 /// ```python
 /// a = "some string"
+/// f"{str(a)}"
 /// f"{repr(a)}"
 /// ```
 ///
 /// Use instead:
 /// ```python
 /// a = "some string"
+/// f"{a}"
 /// f"{a!r}"
 /// ```
 #[violation]
-pub struct ExplicitFStringTypeConversion;
+pub struct ExplicitFStringTypeConversion {
+    operation: Operation,
+}
 
 impl AlwaysAutofixableViolation for ExplicitFStringTypeConversion {
     #[derive_message_formats]
     fn message(&self) -> String {
-        format!("Use conversion in f-string")
+        let ExplicitFStringTypeConversion { operation } = self;
+        match operation {
+            Operation::ConvertCallToConversionFlag => {
+                format!("Use explicit conversion flag")
+            }
+            Operation::RemoveCall => format!("Remove unnecessary `str` conversion"),
+            Operation::RemoveConversionFlag => format!("Remove unnecessary conversion flag"),
+        }
     }
 
     fn autofix_title(&self) -> String {
-        "Replace f-string function call with conversion".to_string()
+        let ExplicitFStringTypeConversion { operation } = self;
+        match operation {
+            Operation::ConvertCallToConversionFlag => {
+                format!("Replace with conversion flag")
+            }
+            Operation::RemoveCall => format!("Remove `str` call"),
+            Operation::RemoveConversionFlag => format!("Remove conversion flag"),
+        }
     }
 }
 
+#[derive(Debug, PartialEq, Eq)]
+enum Operation {
+    /// Ex) Convert `f"{repr(bla)}"` to `f"{bla!r}"`
+    ConvertCallToConversionFlag,
+    /// Ex) Convert `f"{bla!s}"` to `f"{bla}"`
+    RemoveConversionFlag,
+    /// Ex) Convert `f"{str(bla)}"` to `f"{bla}"`
+    RemoveCall,
+}
+
 /// RUF010
 pub(crate) fn explicit_f_string_type_conversion(
     checker: &mut Checker,
@@ -64,52 +96,158 @@ pub(crate) fn explicit_f_string_type_conversion(
         .enumerate()
     {
         let ast::ExprFormattedValue {
-            value, conversion, ..
+            value,
+            conversion,
+            format_spec,
+            range: _,
         } = formatted_value;
 
-        // Skip if there's already a conversion flag.
+        match conversion {
-        if !conversion.is_none() {
+            ConversionFlag::Ascii | ConversionFlag::Repr => {
-            continue;
+                // Nothing to do.
+                continue;
+            }
+            ConversionFlag::Str => {
+                // Skip if there's a format spec.
+                if format_spec.is_some() {
+                    continue;
+                }
+
+                // Remove the conversion flag entirely.
+                // Ex) `f"{bla!s}"`
+                let mut diagnostic = Diagnostic::new(
+                    ExplicitFStringTypeConversion {
+                        operation: Operation::RemoveConversionFlag,
+                    },
+                    value.range(),
+                );
+                if checker.patch(diagnostic.kind.rule()) {
+                    diagnostic.try_set_fix(|| {
+                        remove_conversion_flag(expr, index, checker.locator, checker.stylist)
+                    });
+                }
+                checker.diagnostics.push(diagnostic);
+            }
+            ConversionFlag::None => {
+                // Replace with the appropriate conversion flag.
+                let Expr::Call(ast::ExprCall {
+                    func,
+                    args,
+                    keywords,
+                    ..
+                }) = value.as_ref() else {
+                    continue;
+                };
+
+                // Can't be a conversion otherwise.
+                if args.len() != 1 || !keywords.is_empty() {
+                    continue;
+                }
+
+                let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else {
+                    continue;
+                };
+
+                if !matches!(id.as_str(), "str" | "repr" | "ascii") {
+                    continue;
+                };
+
+                if !checker.semantic_model().is_builtin(id) {
+                    continue;
+                }
+
+                if id == "str" && format_spec.is_none() {
+                    // Ex) `f"{str(bla)}"`
+                    let mut diagnostic = Diagnostic::new(
+                        ExplicitFStringTypeConversion {
+                            operation: Operation::RemoveCall,
+                        },
+                        value.range(),
+                    );
+                    if checker.patch(diagnostic.kind.rule()) {
+                        diagnostic.try_set_fix(|| {
+                            remove_conversion_call(expr, index, checker.locator, checker.stylist)
+                        });
+                    }
+                    checker.diagnostics.push(diagnostic);
+                } else {
+                    // Ex) `f"{repr(bla)}"`
+                    let mut diagnostic = Diagnostic::new(
+                        ExplicitFStringTypeConversion {
+                            operation: Operation::ConvertCallToConversionFlag,
+                        },
+                        value.range(),
+                    );
+                    if checker.patch(diagnostic.kind.rule()) {
+                        diagnostic.try_set_fix(|| {
+                            convert_call_to_conversion_flag(
+                                expr,
+                                index,
+                                checker.locator,
+                                checker.stylist,
+                            )
+                        });
+                    }
+                    checker.diagnostics.push(diagnostic);
+                }
+            }
         }
-
-        let Expr::Call(ast::ExprCall {
-            func,
-            args,
-            keywords,
-            ..
-        }) = value.as_ref() else {
-            continue;
-        };
-
-        // Can't be a conversion otherwise.
-        if args.len() != 1 || !keywords.is_empty() {
-            continue;
-        }
-
-        let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else {
-            continue;
-        };
-
-        if !matches!(id.as_str(), "str" | "repr" | "ascii") {
-            continue;
-        };
-
-        if !checker.semantic_model().is_builtin(id) {
-            continue;
-        }
-
-        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);
     }
 }
 
+/// Generate a [`Fix`] to remove a conversion flag from a formatted expression.
+fn remove_conversion_flag(
+    expr: &Expr,
+    index: usize,
+    locator: &Locator,
+    stylist: &Stylist,
+) -> Result<Fix> {
+    // Parenthesize the expression, to support implicit concatenation.
+    let range = expr.range();
+    let content = locator.slice(range);
+    let parenthesized_content = format!("({content})");
+    let mut expression = match_expression(&parenthesized_content)?;
+
+    // Replace the formatted call expression at `index` with a conversion flag.
+    let mut formatted_string_expression = match_part(index, &mut expression)?;
+    formatted_string_expression.conversion = None;
+
+    // Remove the parentheses (first and last characters).
+    let mut content = expression.codegen_stylist(stylist);
+    content.remove(0);
+    content.pop();
+
+    Ok(Fix::automatic(Edit::range_replacement(content, range)))
+}
+
+/// Generate a [`Fix`] to remove a call from a formatted expression.
+fn remove_conversion_call(
+    expr: &Expr,
+    index: usize,
+    locator: &Locator,
+    stylist: &Stylist,
+) -> Result<Fix> {
+    // Parenthesize the expression, to support implicit concatenation.
+    let range = expr.range();
+    let content = locator.slice(range);
+    let parenthesized_content = format!("({content})");
+    let mut expression = match_expression(&parenthesized_content)?;
+
+    // Replace the formatted call expression at `index` with a conversion flag.
+    let mut formatted_string_expression = match_part(index, &mut expression)?;
+    let call = match_call_mut(&mut formatted_string_expression.expression)?;
+    formatted_string_expression.expression = call.args[0].value.clone();
+
+    // Remove the parentheses (first and last characters).
+    let mut content = expression.codegen_stylist(stylist);
+    content.remove(0);
+    content.pop();
+
+    Ok(Fix::automatic(Edit::range_replacement(content, range)))
+}
+
 /// Generate a [`Fix`] to replace an explicit type conversion with a conversion flag.
-fn fix_explicit_f_string_type_conversion(
+fn convert_call_to_conversion_flag(
     expr: &Expr,
     index: usize,
     locator: &Locator,

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

@@ -1,33 +1,33 @@
 ---
 source: crates/ruff/src/rules/ruff/mod.rs
 ---
-RUF010.py:9:4: RUF010 [*] Use conversion in f-string
+RUF010.py:9:4: RUF010 [*] Remove unnecessary `str` conversion
    |
  9 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
    |    ^^^^^^^^ RUF010
 10 | 
 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
    |
-   = help: Replace f-string function call with conversion
+   = help: Remove `str` call
 
 ℹ Fix
 6  6  |     pass
 7  7  | 
 8  8  | 
 9     |-f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
-   9  |+f"{bla!s}, {repr(bla)}, {ascii(bla)}"  # RUF010
+   9  |+f"{bla}, {repr(bla)}, {ascii(bla)}"  # RUF010
 10 10 | 
 11 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
 12 12 | 
 
-RUF010.py:9:16: RUF010 [*] Use conversion in f-string
+RUF010.py:9:16: RUF010 [*] Use explicit conversion flag
    |
  9 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
    |                ^^^^^^^^^ RUF010
 10 | 
 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
    |
-   = help: Replace f-string function call with conversion
+   = help: Replace with conversion flag
 
 ℹ Fix
 6  6  |     pass
@@ -39,14 +39,14 @@ RUF010.py:9:16: RUF010 [*] Use conversion in f-string
 11 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
 12 12 | 
 
-RUF010.py:9:29: RUF010 [*] Use conversion in f-string
+RUF010.py:9:29: RUF010 [*] Use explicit conversion flag
    |
  9 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
    |                             ^^^^^^^^^^ RUF010
 10 | 
 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
    |
-   = help: Replace f-string function call with conversion
+   = help: Replace with conversion flag
 
 ℹ Fix
 6  6  |     pass
@@ -58,7 +58,7 @@ RUF010.py:9:29: RUF010 [*] Use conversion in f-string
 11 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
 12 12 | 
 
-RUF010.py:11:4: RUF010 [*] Use conversion in f-string
+RUF010.py:11:4: RUF010 [*] Remove unnecessary `str` conversion
    |
 11 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
 12 | 
@@ -67,19 +67,19 @@ RUF010.py:11:4: RUF010 [*] Use conversion in f-string
 14 | 
 15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
    |
-   = help: Replace f-string function call with conversion
+   = help: Remove `str` call
 
 ℹ Fix
 8  8  | 
 9  9  | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
 10 10 | 
 11    |-f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
-   11 |+f"{d['a']!s}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
+   11 |+f"{d['a']}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
 12 12 | 
 13 13 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
 14 14 | 
 
-RUF010.py:11:19: RUF010 [*] Use conversion in f-string
+RUF010.py:11:19: RUF010 [*] Use explicit conversion flag
    |
 11 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
 12 | 
@@ -88,7 +88,7 @@ RUF010.py:11:19: RUF010 [*] Use conversion in f-string
 14 | 
 15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
    |
-   = help: Replace f-string function call with conversion
+   = help: Replace with conversion flag
 
 ℹ Fix
 8  8  | 
@@ -100,7 +100,7 @@ RUF010.py:11:19: RUF010 [*] Use conversion in f-string
 13 13 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
 14 14 | 
 
-RUF010.py:11:35: RUF010 [*] Use conversion in f-string
+RUF010.py:11:35: RUF010 [*] Use explicit conversion flag
    |
 11 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
 12 | 
@@ -109,7 +109,7 @@ RUF010.py:11:35: RUF010 [*] Use conversion in f-string
 14 | 
 15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
    |
-   = help: Replace f-string function call with conversion
+   = help: Replace with conversion flag
 
 ℹ Fix
 8  8  | 
@@ -121,7 +121,7 @@ RUF010.py:11:35: RUF010 [*] Use conversion in f-string
 13 13 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
 14 14 | 
 
-RUF010.py:13:5: RUF010 [*] Use conversion in f-string
+RUF010.py:13:5: RUF010 [*] Remove unnecessary `str` conversion
    |
 13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
 14 | 
@@ -130,19 +130,19 @@ RUF010.py:13:5: RUF010 [*] Use conversion in f-string
 16 | 
 17 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
    |
-   = help: Replace f-string function call with conversion
+   = help: Remove `str` call
 
 ℹ 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
+   13 |+f"{bla}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
 14 14 | 
 15 15 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
 16 16 | 
 
-RUF010.py:13:19: RUF010 [*] Use conversion in f-string
+RUF010.py:13:19: RUF010 [*] Use explicit conversion flag
    |
 13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
 14 | 
@@ -151,7 +151,7 @@ RUF010.py:13:19: RUF010 [*] Use conversion in f-string
 16 | 
 17 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
    |
-   = help: Replace f-string function call with conversion
+   = help: Replace with conversion flag
 
 ℹ Fix
 10 10 | 
@@ -163,7 +163,7 @@ RUF010.py:13:19: RUF010 [*] Use conversion in f-string
 15 15 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
 16 16 | 
 
-RUF010.py:13:34: RUF010 [*] Use conversion in f-string
+RUF010.py:13:34: RUF010 [*] Use explicit conversion flag
    |
 13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
 14 | 
@@ -172,7 +172,7 @@ RUF010.py:13:34: RUF010 [*] Use conversion in f-string
 16 | 
 17 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
    |
-   = help: Replace f-string function call with conversion
+   = help: Replace with conversion flag
 
 ℹ Fix
 10 10 | 
@@ -184,7 +184,28 @@ RUF010.py:13:34: RUF010 [*] Use conversion in f-string
 15 15 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
 16 16 | 
 
-RUF010.py:15:14: RUF010 [*] Use conversion in f-string
+RUF010.py:15:4: RUF010 [*] Remove unnecessary conversion flag
+   |
+15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+16 | 
+17 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+   |    ^^^ RUF010
+18 | 
+19 | f"{foo(bla)}"  # OK
+   |
+   = help: Remove conversion flag
+
+ℹ Fix
+12 12 | 
+13 13 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+14 14 | 
+15    |-f"{bla!s}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+   15 |+f"{bla}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+16 16 | 
+17 17 | f"{foo(bla)}"  # OK
+18 18 | 
+
+RUF010.py:15:14: RUF010 [*] Use explicit conversion flag
    |
 15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
 16 | 
@@ -193,7 +214,7 @@ RUF010.py:15:14: RUF010 [*] Use conversion in f-string
 18 | 
 19 | f"{foo(bla)}"  # OK
    |
-   = help: Replace f-string function call with conversion
+   = help: Replace with conversion flag
 
 ℹ Fix
 12 12 | 
@@ -205,7 +226,7 @@ RUF010.py:15:14: RUF010 [*] Use conversion in f-string
 17 17 | f"{foo(bla)}"  # OK
 18 18 | 
 
-RUF010.py:15:29: RUF010 [*] Use conversion in f-string
+RUF010.py:15:29: RUF010 [*] Use explicit conversion flag
    |
 15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
 16 | 
@@ -214,7 +235,7 @@ RUF010.py:15:29: RUF010 [*] Use conversion in f-string
 18 | 
 19 | f"{foo(bla)}"  # OK
    |
-   = help: Replace f-string function call with conversion
+   = help: Replace with conversion flag
 
 ℹ Fix
 12 12 | 
@@ -226,7 +247,28 @@ RUF010.py:15:29: RUF010 [*] Use conversion in f-string
 17 17 | f"{foo(bla)}"  # OK
 18 18 | 
 
-RUF010.py:35:20: RUF010 [*] Use conversion in f-string
+RUF010.py:21:4: RUF010 [*] Remove unnecessary conversion flag
+   |
+21 | f"{str(bla, 'ascii')}, {str(bla, encoding='cp1255')}"  # OK
+22 | 
+23 | f"{bla!s} {[]!r} {'bar'!a}"  # OK
+   |    ^^^ RUF010
+24 | 
+25 | "Not an f-string {str(bla)}, {repr(bla)}, {ascii(bla)}"  # OK
+   |
+   = help: Remove conversion flag
+
+ℹ Fix
+18 18 | 
+19 19 | f"{str(bla, 'ascii')}, {str(bla, encoding='cp1255')}"  # OK
+20 20 | 
+21    |-f"{bla!s} {[]!r} {'bar'!a}"  # OK
+   21 |+f"{bla} {[]!r} {'bar'!a}"  # OK
+22 22 | 
+23 23 | "Not an f-string {str(bla)}, {repr(bla)}, {ascii(bla)}"  # OK
+24 24 | 
+
+RUF010.py:35:20: RUF010 [*] Use explicit conversion flag
    |
 35 |     f"Member of tuple mismatches type at index {i}. Expected {of_shape_i}. Got "
 36 |     " intermediary content "
@@ -234,7 +276,7 @@ RUF010.py:35:20: RUF010 [*] Use conversion in f-string
    |                    ^^^^^^^^^ RUF010
 38 | )
    |
-   = help: Replace f-string function call with conversion
+   = help: Replace with conversion flag
 
 ℹ Fix
 32 32 | (
@@ -243,5 +285,67 @@ RUF010.py:35:20: RUF010 [*] Use conversion in f-string
 35    |-    f" that flows {repr(obj)} of type {type(obj)}.{additional_message}"  # RUF010
    35 |+    f" that flows {obj!r} of type {type(obj)}.{additional_message}"  # RUF010
 36 36 | )
+37 37 | 
+38 38 | 
+
+RUF010.py:39:4: RUF010 [*] Remove unnecessary `str` conversion
+   |
+39 | f"{str(bla)}"  # RUF010
+   |    ^^^^^^^^ RUF010
+40 | 
+41 | f"{str(bla):20}"  # RUF010
+   |
+   = help: Remove `str` call
+
+ℹ Fix
+36 36 | )
+37 37 | 
+38 38 | 
+39    |-f"{str(bla)}"  # RUF010
+   39 |+f"{bla}"  # RUF010
+40 40 | 
+41 41 | f"{str(bla):20}"  # RUF010
+42 42 | 
+
+RUF010.py:41:4: RUF010 [*] Use explicit conversion flag
+   |
+41 | f"{str(bla)}"  # RUF010
+42 | 
+43 | f"{str(bla):20}"  # RUF010
+   |    ^^^^^^^^ RUF010
+44 | 
+45 | f"{bla!s}"  # RUF010
+   |
+   = help: Replace with conversion flag
+
+ℹ Fix
+38 38 | 
+39 39 | f"{str(bla)}"  # RUF010
+40 40 | 
+41    |-f"{str(bla):20}"  # RUF010
+   41 |+f"{bla!s:20}"  # RUF010
+42 42 | 
+43 43 | f"{bla!s}"  # RUF010
+44 44 | 
+
+RUF010.py:43:4: RUF010 [*] Remove unnecessary conversion flag
+   |
+43 | f"{str(bla):20}"  # RUF010
+44 | 
+45 | f"{bla!s}"  # RUF010
+   |    ^^^ RUF010
+46 | 
+47 | f"{bla!s:20}"  # OK
+   |
+   = help: Remove conversion flag
+
+ℹ Fix
+40 40 | 
+41 41 | f"{str(bla):20}"  # RUF010
+42 42 | 
+43    |-f"{bla!s}"  # RUF010
+   43 |+f"{bla}"  # RUF010
+44 44 | 
+45 45 | f"{bla!s:20}"  # OK