Enable printf-string-formatting fix with comments on right-hand side (

#9037) ## Summary This was added in #6364 (as a follow-on to #6342), but I don't think it applies in the same way, because we don't _remove_ the right-hand side when converting from `%`-style formatting to `.format` calls. Closes #8107.
astral-sh · Dec 7, 2023 · acab5f3 · acab5f3
1 parent 06c9f62
commit acab5f3
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 13 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_0.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_0.py
@@ -110,3 +110,10 @@
 "%s" % (
     x,  # comment
 )
+
+
+path = "%s-%s-%s.pem" % (
+    safe_domain_name(cn), # common name, which should be filename safe because it is IDNA-encoded, but in case of a malformed cert make sure it's ok to use as a filename
+    cert.not_valid_after.date().isoformat().replace("-", ""), # expiration date
+    hexlify(cert.fingerprint(hashes.SHA256())).decode("ascii")[0:8], # fingerprint prefix
+)
diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/printf_string_formatting.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/printf_string_formatting.rs
@@ -490,18 +490,10 @@ pub(crate) fn printf_string_formatting(checker: &mut Checker, expr: &Expr, right
     contents.push_str(&format!(".format{params_string}"));
 
     let mut diagnostic = Diagnostic::new(PrintfStringFormatting, expr.range());
-    // Avoid fix if there are comments within the right-hand side:
-    // ```
-    // "%s" % (
-    //     0,  # 0
-    // )
-    // ```
-    if !checker.indexer().comment_ranges().intersects(right.range()) {
-        diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
-            contents,
-            expr.range(),
-        )));
-    }
+    diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+        contents,
+        expr.range(),
+    )));
     checker.diagnostics.push(diagnostic);
 }
 

diff --git a/...inter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP031_0.py.snap b/...inter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP031_0.py.snap
@@ -896,7 +896,7 @@ UP031_0.py:104:5: UP031 [*] Use format specifiers instead of percent format
 109 108 | 
 110 109 | "%s" % (
 
-UP031_0.py:110:1: UP031 Use format specifiers instead of percent format
+UP031_0.py:110:1: UP031 [*] Use format specifiers instead of percent format
     |
 108 |   )
 109 |   
@@ -907,4 +907,36 @@ UP031_0.py:110:1: UP031 Use format specifiers instead of percent format
     |
     = help: Replace with format specifiers
 
+ℹ Unsafe fix
+107 107 |     % (x,)
+108 108 | )
+109 109 | 
+110     |-"%s" % (
+    110 |+"{}".format(
+111 111 |     x,  # comment
+112 112 | )
+113 113 | 
+
+UP031_0.py:115:8: UP031 [*] Use format specifiers instead of percent format
+    |
+115 |   path = "%s-%s-%s.pem" % (
+    |  ________^
+116 | |     safe_domain_name(cn), # common name, which should be filename safe because it is IDNA-encoded, but in case of a malformed cert make sure it's ok to use as a filename
+117 | |     cert.not_valid_after.date().isoformat().replace("-", ""), # expiration date
+118 | |     hexlify(cert.fingerprint(hashes.SHA256())).decode("ascii")[0:8], # fingerprint prefix
+119 | | )
+    | |_^ UP031
+    |
+    = help: Replace with format specifiers
+
+ℹ Unsafe fix
+112 112 | )
+113 113 | 
+114 114 | 
+115     |-path = "%s-%s-%s.pem" % (
+    115 |+path = "{}-{}-{}.pem".format(
+116 116 |     safe_domain_name(cn), # common name, which should be filename safe because it is IDNA-encoded, but in case of a malformed cert make sure it's ok to use as a filename
+117 117 |     cert.not_valid_after.date().isoformat().replace("-", ""), # expiration date
+118 118 |     hexlify(cert.fingerprint(hashes.SHA256())).decode("ascii")[0:8], # fingerprint prefix
+