Autofix for UP032 swallows comment

[WhyNotHugo]

Given this snippet:

        return "data:image/{};base64,{}".format(
            ext[1:], data.decode()  # Remove the leading dot.
        )

Ruff autofix produces this output:

        return f"data:image/{ext[1:]};base64,{data.decode()}"

Note that the autofixed code "swallows" the comment. See here for original context: WhyNotHugo/django-afip@2974184

This is reproducible with ruff 0.0.281. The relevant pyproject.toml is here.

[harupy]

@charliermarsh Can I work on fixing this?

[charliermarsh]

Sure. How are you thinking of resolving? What's the expected behavior here?

[harupy]

diff --git a/crates/ruff/src/rules/pyupgrade/rules/f_strings.rs b/crates/ruff/src/rules/pyupgrade/rules/f_strings.rs
index 1739207cb..2dbf10061 100644
--- a/crates/ruff/src/rules/pyupgrade/rules/f_strings.rs
+++ b/crates/ruff/src/rules/pyupgrade/rules/f_strings.rs
@@ -320,7 +320,7 @@ pub(crate) fn f_strings(
         return;
     };
 
-    let Expr::Attribute(ast::ExprAttribute { value, .. }) = func.as_ref() else {
+    let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func.as_ref() else {
         return;
     };
 
@@ -334,6 +334,25 @@ pub(crate) fn f_strings(
         return;
     };
 
+    // Exit if comments are present in the argument range:
+    // ```
+    // "{} {}".format(
+    //   1,  # 1
+    //   2,  # 2
+    // )
+    // ```
+    if lexer::lex(
+        checker
+            .locator()
+            .slice(TextRange::new(attr.start(), expr.end())),
+        Mode::Expression,
+    )
+    .flatten()
+    .any(|(tok, _)| matches!(tok, Tok::Comment(..)))
+    {
+        return;
+    }
+
     let Some(mut summary) = FormatSummaryValues::try_from_expr(expr, checker.locator()) else {
         return;
     };

I think preserving comments is not easy. Can we avoid raising UP032 if comments are present in the argument range like this^?

[WhyNotHugo]

Can we avoid raising UP032 if comments are present in the argument range like this^?

That's definitely one approach. Another possible approach is to raise the warning but skip it when auto-fixing.

I'll let you decide which one is best :)

[charliermarsh]

Yeah I'd say raise-but-not-fix if there are comments within the call.

[harupy]

+1 to raise-but-not-fix.

[WhyNotHugo]

That was quick! Thanks