Handle implicit string concatenations in conversion-flag rewrites (#4947

)

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

@@ -28,3 +28,9 @@ def ascii(arg):
 
 
 f"{ascii(bla)}"  # OK
+
+(
+    f"Member of tuple mismatches type at index {i}. Expected {of_shape_i}. Got "
+    " intermediary content "
+    f" that flows {repr(obj)} of type {type(obj)}.{additional_message}"  # RUF010
+)

crates/ruff/src/cst/matchers.rs

@@ -1,9 +1,8 @@
 use anyhow::{bail, Result};
 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, Name, SmallStatement, Statement, Suite, Tuple, With,
 };
 
 pub(crate) fn match_module(module_text: &str) -> Result<Module> {
@@ -109,28 +108,6 @@ pub(crate) fn match_attribute<'a, 'b>(
     }
 }
 
-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 Expression<'b>) -> Result<&'a Name<'b>> {
     if let Expression::Name(name) = expression {
         Ok(name)

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

@@ -1,16 +1,16 @@
 use anyhow::{bail, Result};
+use libcst_native::{
+    ConcatenatedString, Expression, FormattedStringContent, FormattedStringExpression,
+};
 use rustpython_parser::ast::{self, Expr, Ranged};
 
-use crate::autofix::codemods::CodegenStylist;
 use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::source_code::{Locator, Stylist};
 
+use crate::autofix::codemods::CodegenStylist;
 use crate::checkers::ast::Checker;
-use crate::cst::matchers::{
-    match_call_mut, match_expression, match_formatted_string, match_formatted_string_expression,
-    match_name,
-};
+use crate::cst::matchers::{match_call_mut, match_expression, match_name};
 use crate::registry::AsRule;
 
 /// ## What it does
@@ -46,57 +46,26 @@ 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 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(Fix::automatic(Edit::range_replacement(
-        expression.codegen_stylist(stylist),
-        range,
-    )))
-}
-
 /// RUF010
 pub(crate) fn explicit_f_string_type_conversion(
     checker: &mut Checker,
     expr: &Expr,
     values: &[Expr],
 ) {
-    for (index, formatted_value) in values.iter().enumerate() {
-        let Expr::FormattedValue(ast::ExprFormattedValue {
-            conversion,
-            value,
-            ..
-        }) = &formatted_value else {
-            continue;
-        };
+    for (index, formatted_value) in values
+        .iter()
+        .filter_map(|expr| {
+            if let Expr::FormattedValue(expr) = &expr {
+                Some(expr)
+            } else {
+                None
+            }
+        })
+        .enumerate()
+    {
+        let ast::ExprFormattedValue {
+            value, conversion, ..
+        } = formatted_value;
 
         // Skip if there's already a conversion flag.
         if !conversion.is_none() {
@@ -138,3 +107,100 @@ pub(crate) fn explicit_f_string_type_conversion(
         checker.diagnostics.push(diagnostic);
     }
 }
+
+/// Generate a [`Fix`] to replace an explicit type conversion with a conversion flag.
+fn fix_explicit_f_string_type_conversion(
+    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)?;
+    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();
+
+    // 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)))
+}
+
+/// Return the [`FormattedStringContent`] at the given index in an f-string or implicit
+/// string concatenation.
+fn match_part<'a, 'b>(
+    index: usize,
+    expr: &'a mut Expression<'b>,
+) -> Result<&'a mut FormattedStringExpression<'b>> {
+    match expr {
+        Expression::ConcatenatedString(expr) => Ok(collect_parts(expr).remove(index)),
+        Expression::FormattedString(expr) => {
+            // Find the formatted expression at the given index. The `parts` field contains a mix
+            // of string literals and expressions, but our `index` only counts expressions. All
+            // the boxing and mutability makes this difficult to write in a functional style.
+            let mut format_index = 0;
+            for part in &mut expr.parts {
+                if let FormattedStringContent::Expression(expr) = part {
+                    if format_index == index {
+                        return Ok(expr);
+                    }
+                    format_index += 1;
+                }
+            }
+
+            bail!("Index out of bounds: `{index}`")
+        }
+        _ => bail!("Unexpected expression: `{:?}`", expr),
+    }
+}
+
+/// Given an implicit string concatenation, return a list of all the formatted expressions.
+fn collect_parts<'a, 'b>(
+    expr: &'a mut ConcatenatedString<'b>,
+) -> Vec<&'a mut FormattedStringExpression<'b>> {
+    fn inner<'a, 'b>(
+        string: &'a mut libcst_native::String<'b>,
+        formatted_expressions: &mut Vec<&'a mut FormattedStringExpression<'b>>,
+    ) {
+        match string {
+            libcst_native::String::Formatted(expr) => {
+                for part in &mut expr.parts {
+                    if let FormattedStringContent::Expression(expr) = part {
+                        formatted_expressions.push(expr);
+                    }
+                }
+            }
+            libcst_native::String::Concatenated(expr) => {
+                inner(&mut expr.left, formatted_expressions);
+                inner(&mut expr.right, formatted_expressions);
+            }
+            libcst_native::String::Simple(_) => {}
+        }
+    }
+
+    let mut formatted_expressions = vec![];
+    inner(&mut expr.left, &mut formatted_expressions);
+    inner(&mut expr.right, &mut formatted_expressions);
+    formatted_expressions
+}

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

@@ -226,4 +226,22 @@ 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
+   |
+35 |     f"Member of tuple mismatches type at index {i}. Expected {of_shape_i}. Got "
+36 |     " intermediary content "
+37 |     f" that flows {repr(obj)} of type {type(obj)}.{additional_message}"  # RUF010
+   |                    ^^^^^^^^^ RUF010
+38 | )
+   |
+   = help: Replace f-string function call with conversion
+
+ℹ Fix
+32 32 | (
+33 33 |     f"Member of tuple mismatches type at index {i}. Expected {of_shape_i}. Got "
+34 34 |     " intermediary content "
+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 | )
+