diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_0.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_0.py index 00fef079b6..1bdcbc88c1 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_0.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_0.py @@ -117,3 +117,10 @@ path = "%s-%s-%s.pem" % ( 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'] diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_1.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_1.py index 57f25df712..a03cbd1d1a 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_1.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_1.py @@ -32,10 +32,3 @@ pytest.param('"%8s" % (None,)', id="unsafe width-string conversion"), "%(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'] 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 index 11b2aedbd8..3c53924d6d 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/printf_string_formatting.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/printf_string_formatting.rs @@ -19,7 +19,8 @@ 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 @@ -27,30 +28,6 @@ use crate::rules::pyupgrade::helpers::curly_escape; /// 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: -/// ```python -/// "%s" % value -/// ``` -/// -/// `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: -/// -/// ```python -/// value = 1 -/// print("%s" % value) # "1" -/// print("{}".format(value)) # "1" -/// -/// value = (1,) -/// print("%s" % value) # "1" -/// print("{}".format(value)) # "(1,)" -/// ``` -/// /// ## Example /// ```python /// "%s, %s" % ("Hello", "World") # "Hello, World" @@ -61,6 +38,31 @@ use crate::rules::pyupgrade::helpers::curly_escape; /// "{}, {}".format("Hello", "World") # "Hello, World" /// ``` /// +/// ## 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" % val +/// ``` +/// +/// `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 +/// val = 1 +/// print("%s" % val) # "1" +/// print("{}".format(val)) # "1" +/// +/// val = (1,) +/// print("%s" % val) # "1" +/// print("{}".format(val)) # "(1,)" +/// ``` +/// /// ## References /// - [Python documentation: `printf`-style String Formatting](https://docs.python.org/3/library/stdtypes.html#old-string-formatting) /// - [Python documentation: `str.format`](https://docs.python.org/3/library/stdtypes.html#str.format) @@ -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()), diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP031_0.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP031_0.py.snap index fd5827fcfa..1a200861b6 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP031_0.py.snap +++ b/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'])