[ruff] Fix false positives and negatives in RUF010

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

@@ -36,5 +36,19 @@ def ascii(arg):
 )
 
 
-# OK
+# https://github.com/astral-sh/ruff/issues/16325
 f"{str({})}"
+
+f"{ascii({} | {})}"
+
+import builtins
+
+f"{builtins.repr(1)}"
+
+f"{ascii(1)=}"
+
+f"{ascii(lambda: 1)}"
+
+f"{ascii(x := 2)}"
+
+f"{str(object=3)}"

crates/ruff_linter/src/cst/matchers.rs

@@ -1,10 +1,9 @@
 use crate::fix::codemods::CodegenStylist;
 use anyhow::{Result, bail};
 use libcst_native::{
-    Arg, Attribute, Call, Comparison, CompoundStatement, Dict, Expression, FormattedString,
-    FormattedStringContent, FormattedStringExpression, FunctionDef, GeneratorExp, If, Import,
-    ImportAlias, ImportFrom, ImportNames, IndentedBlock, Lambda, ListComp, Module, Name,
-    SmallStatement, Statement, Suite, Tuple, With,
+    Arg, Attribute, Call, Comparison, CompoundStatement, Dict, Expression, FunctionDef,
+    GeneratorExp, If, Import, ImportAlias, ImportFrom, ImportNames, IndentedBlock, Lambda,
+    ListComp, Module, SmallStatement, Statement, Suite, Tuple, With,
 };
 use ruff_python_codegen::Stylist;
 
@@ -104,14 +103,6 @@ pub(crate) fn match_attribute<'a, 'b>(
     }
 }
 
-pub(crate) fn match_name<'a, 'b>(expression: &'a Expression<'b>) -> Result<&'a Name<'b>> {
-    if let Expression::Name(name) = expression {
-        Ok(name)
-    } else {
-        bail!("Expected Expression::Name")
-    }
-}
-
 pub(crate) fn match_arg<'a, 'b>(call: &'a Call<'b>) -> Result<&'a Arg<'b>> {
     if let Some(arg) = call.args.first() {
         Ok(arg)
@@ -154,28 +145,6 @@ pub(crate) fn match_lambda<'a, 'b>(expression: &'a Expression<'b>) -> Result<&'a
     }
 }
 
-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_function_def<'a, 'b>(
     statement: &'a mut Statement<'b>,
 ) -> Result<&'a mut FunctionDef<'b>> {

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

@@ -1,16 +1,14 @@
-use anyhow::{Result, bail};
+use std::fmt::Display;
+
+use anyhow::Result;
 
 use ruff_macros::{ViolationMetadata, derive_message_formats};
-use ruff_python_ast::{self as ast, Arguments, Expr};
-use ruff_python_codegen::Stylist;
+use ruff_python_ast::parenthesize::parenthesized_range;
+use ruff_python_ast::{self as ast, Expr};
+use ruff_python_parser::TokenKind;
 use ruff_text_size::Ranged;
 
-use crate::Locator;
 use crate::checkers::ast::Checker;
-use crate::cst::matchers::{
-    match_call_mut, match_formatted_string, match_formatted_string_expression, match_name,
-    transform_expression,
-};
 use crate::{AlwaysFixableViolation, Edit, Fix};
 
 /// ## What it does
@@ -53,7 +51,7 @@ impl AlwaysFixableViolation for ExplicitFStringTypeConversion {
 
 /// RUF010
 pub(crate) fn explicit_f_string_type_conversion(checker: &Checker, f_string: &ast::FString) {
-    for (index, element) in f_string.elements.iter().enumerate() {
+    for element in &f_string.elements {
         let Some(ast::InterpolatedElement {
             expression,
             conversion,
@@ -68,84 +66,126 @@ pub(crate) fn explicit_f_string_type_conversion(checker: &Checker, f_string: &as
             continue;
         }
 
-        let Expr::Call(ast::ExprCall {
-            func,
-            arguments:
-                Arguments {
-                    args,
-                    keywords,
-                    range: _,
-                    node_index: _,
-                },
-            ..
-        }) = expression.as_ref()
-        else {
+        let Expr::Call(call) = expression.as_ref() else {
             continue;
         };
 
-        // Can't be a conversion otherwise.
-        if !keywords.is_empty() {
-            continue;
-        }
-
-        // Can't be a conversion otherwise.
-        let [arg] = &**args else {
+        let Some(conversion) = checker
+            .semantic()
+            .resolve_builtin_symbol(&call.func)
+            .and_then(Conversion::from_str)
+        else {
             continue;
         };
+        let arg = match conversion {
+            // Handles the cases: `f"{str(object=arg)}"` and `f"{str(arg)}"`
+            Conversion::Str if call.arguments.len() == 1 => {
+                let Some(arg) = call.arguments.find_argument_value("object", 0) else {
+                    continue;
+                };
+                arg
+            }
+            Conversion::Str | Conversion::Repr | Conversion::Ascii => {
+                // Can't be a conversion otherwise.
+                if !call.arguments.keywords.is_empty() {
+                    continue;
+                }
 
-        // Avoid attempting to rewrite, e.g., `f"{str({})}"`; the curly braces are problematic.
-        if matches!(
-            arg,
-            Expr::Dict(_) | Expr::Set(_) | Expr::DictComp(_) | Expr::SetComp(_)
-        ) {
-            continue;
-        }
+                // Can't be a conversion otherwise.
+                let [arg] = call.arguments.args.as_ref() else {
+                    continue;
+                };
+                arg
+            }
+        };
 
-        if !checker
-            .semantic()
-            .resolve_builtin_symbol(func)
-            .is_some_and(|builtin| matches!(builtin, "str" | "repr" | "ascii"))
-        {
-            continue;
+        // Suppress lint for starred expressions.
+        if matches!(arg, Expr::Starred(_)) {
+            return;
         }
 
         let mut diagnostic =
             checker.report_diagnostic(ExplicitFStringTypeConversion, expression.range());
         diagnostic.try_set_fix(|| {
-            convert_call_to_conversion_flag(f_string, index, checker.locator(), checker.stylist())
+            convert_call_to_conversion_flag(checker, conversion, element, call, arg)
         });
     }
 }
 
 /// Generate a [`Fix`] to replace an explicit type conversion with a conversion flag.
 fn convert_call_to_conversion_flag(
-    f_string: &ast::FString,
-    index: usize,
-    locator: &Locator,
-    stylist: &Stylist,
+    checker: &Checker,
+    conversion: Conversion,
+    element: &ast::InterpolatedStringElement,
+    call: &ast::ExprCall,
+    arg: &Expr,
 ) -> Result<Fix> {
-    let source_code = locator.slice(f_string);
-    transform_expression(source_code, stylist, |mut expression| {
-        let formatted_string = match_formatted_string(&mut expression)?;
-        // Replace the formatted call expression at `index` with a conversion flag.
-        let formatted_string_expression =
-            match_formatted_string_expression(&mut formatted_string.parts[index])?;
-        let call = match_call_mut(&mut formatted_string_expression.expression)?;
-        let name = match_name(&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();
-        Ok(expression)
-    })
-    .map(|output| Fix::safe_edit(Edit::range_replacement(output, f_string.range())))
+    if element
+        .as_interpolation()
+        .is_some_and(|interpolation| interpolation.debug_text.is_some())
+    {
+        anyhow::bail!("Don't support fixing f-string with debug text!");
+    }
+
+    let arg_str = checker.locator().slice(arg);
+    let contains_curly_brace = checker
+        .tokens()
+        .in_range(arg.range())
+        .iter()
+        .any(|token| token.kind() == TokenKind::Lbrace);
+
+    let output = if contains_curly_brace {
+        format!(" {arg_str}!{conversion}")
+    } else if matches!(arg, Expr::Lambda(_) | Expr::Named(_)) {
+        format!("({arg_str})!{conversion}")
+    } else {
+        format!("{arg_str}!{conversion}")
+    };
+
+    let replace_range = if let Some(range) = parenthesized_range(
+        call.into(),
+        element.into(),
+        checker.comment_ranges(),
+        checker.source(),
+    ) {
+        range
+    } else {
+        call.range()
+    };
+
+    Ok(Fix::safe_edit(Edit::range_replacement(
+        output,
+        replace_range,
+    )))
+}
+
+/// Represents the three built-in Python conversion functions that can be replaced
+/// with f-string conversion flags.
+#[derive(Copy, Clone)]
+enum Conversion {
+    Ascii,
+    Str,
+    Repr,
+}
+
+impl Conversion {
+    fn from_str(value: &str) -> Option<Self> {
+        Some(match value {
+            "ascii" => Self::Ascii,
+            "str" => Self::Str,
+            "repr" => Self::Repr,
+            _ => return None,
+        })
+    }
+}
+
+impl Display for Conversion {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let value = match self {
+            Conversion::Ascii => "a",
+            Conversion::Str => "s",
+            Conversion::Repr => "r",
+        };
+        write!(f, "{value}")
+    }
 }

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF010_RUF010.py.snap

@@ -244,4 +244,61 @@ RUF010.py:35:20: RUF010 [*] Use explicit conversion flag
    35 |+    f" that flows {obj!r} of type {type(obj)}.{additional_message}"  # RUF010
 36 36 | )
 37 37 | 
-38 38 |
+38 38 | 
+
+RUF010.py:40:4: RUF010 [*] Use explicit conversion flag
+   |
+39 | # https://github.com/astral-sh/ruff/issues/16325
+40 | f"{str({})}"
+   |    ^^^^^^^ RUF010
+41 |
+42 | f"{ascii({} | {})}"
+   |
+   = help: Replace with conversion flag
+
+ℹ Safe fix
+37 37 | 
+38 38 | 
+39 39 | # https://github.com/astral-sh/ruff/issues/16325
+40    |-f"{str({})}"
+   40 |+f"{ {}!s}"
+41 41 | 
+42 42 | f"{ascii({} | {})}"
+43 43 | 
+
+RUF010.py:46:4: RUF010 [*] Use explicit conversion flag
+   |
+44 | import builtins
+45 |
+46 | f"{builtins.repr(1)}"
+   |    ^^^^^^^^^^^^^^^^ RUF010
+47 |
+48 | f"{ascii(1)=}"
+   |
+   = help: Replace with conversion flag
+
+ℹ Safe fix
+43 43 | 
+44 44 | import builtins
+45 45 | 
+46    |-f"{builtins.repr(1)}"
+   46 |+f"{1!r}"
+47 47 | 
+48 48 | f"{ascii(1)=}"
+49 49 | 
+
+RUF010.py:54:4: RUF010 [*] Use explicit conversion flag
+   |
+52 | f"{ascii(x := 2)}"
+53 |
+54 | f"{str(object=3)}"
+   |    ^^^^^^^^^^^^^ RUF010
+   |
+   = help: Replace with conversion flag
+
+ℹ Safe fix
+51 51 | 
+52 52 | f"{ascii(x := 2)}"
+53 53 | 
+54    |-f"{str(object=3)}"
+   54 |+f"{3!s}"