[UP031] When encountering "%s" % var offer unsafe fix

crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_0.py

@@ -117,3 +117,10 @@
     cert.not_valid_after.date().isoformat().replace("-", ""), # expiration date
     hexlify(cert.fingerprint(hashes.SHA256())).decode("ascii")[0:8], # fingerprint prefix
 )
+
+# UP031 (no longer false negatives; now offer potentially unsafe fixes)
+'Hello %s' % bar
+
+'Hello %s' % bar.baz
+
+'Hello %s' % bar['bop']

crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_1.py

@@ -32,10 +32,3 @@
 "%(1)s" % {1: 2, "1": 2}
 
 "%(and)s" % {"and": 2}
-
-# OK (arguably false negatives)
-'Hello %s' % bar
-
-'Hello %s' % bar.baz
-
-'Hello %s' % bar['bop']

crates/ruff_linter/src/rules/pyupgrade/rules/printf_string_formatting.rs

@@ -19,46 +19,48 @@ use crate::checkers::ast::Checker;
 use crate::rules::pyupgrade::helpers::curly_escape;
 
 /// ## What it does
-/// Checks for `printf`-style string formatting.
+/// Checks for `printf`-style string formatting, and offers to replace it with
+/// `str.format` calls.
 ///
 /// ## Why is this bad?
 /// `printf`-style string formatting has a number of quirks, and leads to less
 /// readable code than using `str.format` calls or f-strings. In general, prefer
 /// the newer `str.format` and f-strings constructs over `printf`-style string
 /// formatting.
 ///
-/// ## Known problems
-/// This rule is unable to detect cases in which the format string contains
-/// a single, generic format specifier (e.g. `%s`), and the right-hand side
-/// is an ambiguous expression.
-///
-/// For example, given:
+/// ## Example
 /// ```python
-/// "%s" % value
+/// "%s, %s" % ("Hello", "World")  # "Hello, World"
 /// ```
 ///
-/// `value` could be a single-element tuple, _or_ it could be a single value.
-/// Both of these would resolve to the same formatted string when using
-/// `printf`-style formatting, but _not_ when using f-strings:
-///
+/// Use instead:
 /// ```python
-/// value = 1
-/// print("%s" % value)  # "1"
-/// print("{}".format(value))  # "1"
-///
-/// value = (1,)
-/// print("%s" % value)  # "1"
-/// print("{}".format(value))  # "(1,)"
+/// "{}, {}".format("Hello", "World")  # "Hello, World"
 /// ```
 ///
-/// ## Example
+/// ## Fix safety
+/// In cases where the format string contains a single generic format specifier
+/// (e.g. `%s`), and the right-hand side is an ambiguous expression,
+/// we cannot offer a safe fix.
+///
+/// For example, given:
 /// ```python
-/// "%s, %s" % ("Hello", "World")  # "Hello, World"
+/// "%s" % val
 /// ```
 ///
-/// Use instead:
+/// `val` could be a single-element tuple, _or_ a single value (not
+/// contained in a tuple). Both of these would resolve to the same
+/// formatted string when using `printf`-style formatting, but
+/// resolve differently when using f-strings:
+///
 /// ```python
-/// "{}, {}".format("Hello", "World")  # "Hello, World"
+/// val = 1
+/// print("%s" % val)  # "1"
+/// print("{}".format(val))  # "1"
+///
+/// val = (1,)
+/// print("%s" % val)  # "1"
+/// print("{}".format(val))  # "(1,)"
 /// ```
 ///
 /// ## References
@@ -434,7 +436,8 @@ pub(crate) fn printf_string_formatting(checker: &mut Checker, expr: &Expr, right
                 // print("%s" % x)
                 // print("{}".format(x))
                 // ```
-                return;
+                // So we offer an unsafe fix:
+                Cow::Owned(format!("({})", checker.locator().slice(right)))
             }
         }
         Expr::Tuple(_) => clean_params_tuple(right, checker.locator()),

crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP031_0.py.snap

@@ -926,6 +926,8 @@ UP031_0.py:115:8: UP031 [*] Use format specifiers instead of percent format
 118 | |     hexlify(cert.fingerprint(hashes.SHA256())).decode("ascii")[0:8], # fingerprint prefix
 119 | | )
     | |_^ UP031
+120 |   
+121 |   # UP031 (no longer false negatives; now offer potentially unsafe fixes)
     |
     = help: Replace with format specifiers
 
@@ -939,4 +941,58 @@ UP031_0.py:115:8: UP031 [*] Use format specifiers instead of percent format
 117 117 |     cert.not_valid_after.date().isoformat().replace("-", ""), # expiration date
 118 118 |     hexlify(cert.fingerprint(hashes.SHA256())).decode("ascii")[0:8], # fingerprint prefix
 
+UP031_0.py:122:1: UP031 [*] Use format specifiers instead of percent format
+    |
+121 | # UP031 (no longer false negatives; now offer potentially unsafe fixes)
+122 | 'Hello %s' % bar
+    | ^^^^^^^^^^^^^^^^ UP031
+123 | 
+124 | 'Hello %s' % bar.baz
+    |
+    = help: Replace with format specifiers
+
+ℹ Unsafe fix
+119 119 | )
+120 120 | 
+121 121 | # UP031 (no longer false negatives; now offer potentially unsafe fixes)
+122     |-'Hello %s' % bar
+    122 |+'Hello {}'.format(bar)
+123 123 | 
+124 124 | 'Hello %s' % bar.baz
+125 125 | 
+
+UP031_0.py:124:1: UP031 [*] Use format specifiers instead of percent format
+    |
+122 | 'Hello %s' % bar
+123 | 
+124 | 'Hello %s' % bar.baz
+    | ^^^^^^^^^^^^^^^^^^^^ UP031
+125 | 
+126 | 'Hello %s' % bar['bop']
+    |
+    = help: Replace with format specifiers
+
+ℹ Unsafe fix
+121 121 | # UP031 (no longer false negatives; now offer potentially unsafe fixes)
+122 122 | 'Hello %s' % bar
+123 123 | 
+124     |-'Hello %s' % bar.baz
+    124 |+'Hello {}'.format(bar.baz)
+125 125 | 
+126 126 | 'Hello %s' % bar['bop']
+
+UP031_0.py:126:1: UP031 [*] Use format specifiers instead of percent format
+    |
+124 | 'Hello %s' % bar.baz
+125 | 
+126 | 'Hello %s' % bar['bop']
+    | ^^^^^^^^^^^^^^^^^^^^^^^ UP031
+    |
+    = help: Replace with format specifiers
 
+ℹ Unsafe fix
+123 123 | 
+124 124 | 'Hello %s' % bar.baz
+125 125 | 
+126     |-'Hello %s' % bar['bop']
+    126 |+'Hello {}'.format(bar['bop'])