Avoid auto-fixing UP031 if there are comments within the right-hand s…

…ide (#6364)
astral-sh · Aug 5, 2023 · 501f537 · 501f537
1 parent 1ac2699
commit 501f537
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 5 deletions.
diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP031_0.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP031_0.py
@@ -106,3 +106,7 @@
     """
     % (x,)
 )
+
+"%s" % (
+    x,  # comment
+)
diff --git a/crates/ruff/src/rules/pyupgrade/rules/printf_string_formatting.rs b/crates/ruff/src/rules/pyupgrade/rules/printf_string_formatting.rs
@@ -7,7 +7,7 @@ use ruff_python_literal::cformat::{
 use ruff_python_parser::{lexer, AsMode, Tok};
 use ruff_text_size::TextRange;
 
-use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
+use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::str::{leading_quote, trailing_quote};
 use ruff_python_ast::whitespace::indentation;
@@ -43,14 +43,16 @@ use crate::rules::pyupgrade::helpers::curly_escape;
 #[violation]
 pub struct PrintfStringFormatting;
 
-impl AlwaysAutofixableViolation for PrintfStringFormatting {
+impl Violation for PrintfStringFormatting {
+    const AUTOFIX: AutofixKind = AutofixKind::Sometimes;
+
     #[derive_message_formats]
     fn message(&self) -> String {
         format!("Use format specifiers instead of percent format")
     }
 
-    fn autofix_title(&self) -> String {
-        "Replace with format specifiers".to_string()
+    fn autofix_title(&self) -> Option<String> {
+        Some("Replace with format specifiers".to_string())
     }
 }
 
@@ -467,7 +469,15 @@ pub(crate) fn printf_string_formatting(
     contents.push_str(&format!(".format{params_string}"));
 
     let mut diagnostic = Diagnostic::new(PrintfStringFormatting, expr.range());
-    if checker.patch(diagnostic.kind.rule()) {
+    // Avoid autofix if there are comments within the right-hand side:
+    // ```
+    // "%s" % (
+    //     0,  # 0
+    // )
+    // ```
+    if checker.patch(diagnostic.kind.rule())
+        && !checker.indexer().comment_ranges().intersects(right.range())
+    {
         diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
             contents,
             expr.range(),

diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP031_0.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP031_0.py.snap
@@ -893,5 +893,18 @@ UP031_0.py:104:5: UP031 [*] Use format specifiers instead of percent format
     105 |+    foo {}
     106 |+    """.format(x)
 108 107 | )
+109 108 | 
+110 109 | "%s" % (
+
+UP031_0.py:110:1: UP031 Use format specifiers instead of percent format
+    |
+108 |   )
+109 |   
+110 | / "%s" % (
+111 | |     x,  # comment
+112 | | )
+    | |_^ UP031
+    |
+    = help: Replace with format specifiers