From e9f359ac5e9dc32f0ee4308f91942b0faee766d5 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 20 Mar 2023 23:35:10 -0400 Subject: [PATCH] Convert single-argument %-style format calls (#3600) --- .../test/fixtures/pyupgrade/UP031_0.py | 10 ++ .../test/fixtures/pyupgrade/UP031_1.py | 8 +- .../rules/printf_string_formatting.rs | 44 ++++- ...__rules__pyupgrade__tests__UP031_0.py.snap | 160 ++++++++++++++++++ 4 files changed, 219 insertions(+), 3 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP031_0.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP031_0.py index 9a67469656..e1a725a7aa 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP031_0.py +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP031_0.py @@ -73,3 +73,13 @@ print("%s \N{snowman}" % (a,)) print("%(foo)s \N{snowman}" % {"foo": 1}) print(("foo %s " "bar %s") % (x, y)) + +# Single-value expressions +print('Hello %s' % "World") +print('Hello %s' % f"World") +print('Hello %s (%s)' % bar) +print('Hello %s (%s)' % bar.baz) +print('Hello %s (%s)' % bar['bop']) +print('Hello %(arg)s' % bar) +print('Hello %(arg)s' % bar.baz) +print('Hello %(arg)s' % bar['bop']) diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP031_1.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP031_1.py index ba27262925..ecf7a91407 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP031_1.py +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP031_1.py @@ -1,6 +1,4 @@ # OK -"%s" % unknown_type - b"%s" % (b"bytestring",) "%*s" % (5, "hi") @@ -57,3 +55,9 @@ pytest.param('"%8s" % (None,)', id="unsafe width-string conversion"), """ % (x,) ) + +'Hello %s' % bar + +'Hello %s' % bar.baz + +'Hello %s' % bar['bop'] diff --git a/crates/ruff/src/rules/pyupgrade/rules/printf_string_formatting.rs b/crates/ruff/src/rules/pyupgrade/rules/printf_string_formatting.rs index bda8ef63c7..3b238cb178 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/printf_string_formatting.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/printf_string_formatting.rs @@ -335,7 +335,9 @@ pub(crate) fn printf_string_formatting( } // Parse each string segment. - let mut format_strings = vec![]; + let mut num_positional_arguments = 0; + let mut num_keyword_arguments = 0; + let mut format_strings = Vec::with_capacity(strings.len()); for (start, end) in &strings { let string = checker.locator.slice(Range::new(*start, *end)); let (Some(leader), Some(trailer)) = (leading_quote(string), trailing_quote(string)) else { @@ -351,12 +353,52 @@ pub(crate) fn printf_string_formatting( return; } + // Count the number of positional and keyword arguments. + for (.., format_part) in format_string.iter() { + let CFormatPart::Spec(ref fmt) = format_part else { + continue; + }; + if fmt.mapping_key.is_none() { + num_positional_arguments += 1; + } else { + num_keyword_arguments += 1; + } + } + + // Convert the `%`-format string to a `.format` string. let format_string = percent_to_format(&format_string); format_strings.push(format!("{leader}{format_string}{trailer}")); } // Parse the parameters. let params_string = match right.node { + ExprKind::Constant { .. } | ExprKind::JoinedStr { .. } => { + format!("({})", checker.locator.slice(right)) + } + ExprKind::Name { .. } + | ExprKind::Attribute { .. } + | ExprKind::Subscript { .. } + | ExprKind::Call { .. } => { + if num_keyword_arguments > 0 { + // If we have _any_ named fields, assume the right-hand side is a mapping. + format!("(**{})", checker.locator.slice(right)) + } else if num_positional_arguments > 1 { + // If we have multiple fields, but no named fields, assume the right-hand side is a + // tuple. + format!("(*{})", checker.locator.slice(right)) + } else { + // Otherwise, if we have a single field, we can't make any assumptions about the + // right-hand side. It _could_ be a tuple, but it could also be a single value, + // and we can't differentiate between them. + // For example: + // ```python + // x = (1,) + // print("%s" % x) + // print("{}".format(x)) + // ``` + return; + } + } ExprKind::Tuple { .. } => clean_params_tuple(checker, right), ExprKind::Dict { .. } => { if let Some(params_string) = clean_params_dictionary(checker, right) { 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 index 5fc3d25dc6..acb8c53121 100644 --- 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 @@ -582,4 +582,164 @@ expression: diagnostics row: 75 column: 35 parent: ~ +- kind: + name: PrintfStringFormatting + body: Use format specifiers instead of percent format + suggestion: Replace with format specifiers + fixable: true + location: + row: 78 + column: 6 + end_location: + row: 78 + column: 26 + fix: + content: "'Hello {}'.format(\"World\")" + location: + row: 78 + column: 6 + end_location: + row: 78 + column: 26 + parent: ~ +- kind: + name: PrintfStringFormatting + body: Use format specifiers instead of percent format + suggestion: Replace with format specifiers + fixable: true + location: + row: 79 + column: 6 + end_location: + row: 79 + column: 27 + fix: + content: "'Hello {}'.format(f\"World\")" + location: + row: 79 + column: 6 + end_location: + row: 79 + column: 27 + parent: ~ +- kind: + name: PrintfStringFormatting + body: Use format specifiers instead of percent format + suggestion: Replace with format specifiers + fixable: true + location: + row: 80 + column: 6 + end_location: + row: 80 + column: 27 + fix: + content: "'Hello {} ({})'.format(*bar)" + location: + row: 80 + column: 6 + end_location: + row: 80 + column: 27 + parent: ~ +- kind: + name: PrintfStringFormatting + body: Use format specifiers instead of percent format + suggestion: Replace with format specifiers + fixable: true + location: + row: 81 + column: 6 + end_location: + row: 81 + column: 31 + fix: + content: "'Hello {} ({})'.format(*bar.baz)" + location: + row: 81 + column: 6 + end_location: + row: 81 + column: 31 + parent: ~ +- kind: + name: PrintfStringFormatting + body: Use format specifiers instead of percent format + suggestion: Replace with format specifiers + fixable: true + location: + row: 82 + column: 6 + end_location: + row: 82 + column: 34 + fix: + content: "'Hello {} ({})'.format(*bar['bop'])" + location: + row: 82 + column: 6 + end_location: + row: 82 + column: 34 + parent: ~ +- kind: + name: PrintfStringFormatting + body: Use format specifiers instead of percent format + suggestion: Replace with format specifiers + fixable: true + location: + row: 83 + column: 6 + end_location: + row: 83 + column: 27 + fix: + content: "'Hello {arg}'.format(**bar)" + location: + row: 83 + column: 6 + end_location: + row: 83 + column: 27 + parent: ~ +- kind: + name: PrintfStringFormatting + body: Use format specifiers instead of percent format + suggestion: Replace with format specifiers + fixable: true + location: + row: 84 + column: 6 + end_location: + row: 84 + column: 31 + fix: + content: "'Hello {arg}'.format(**bar.baz)" + location: + row: 84 + column: 6 + end_location: + row: 84 + column: 31 + parent: ~ +- kind: + name: PrintfStringFormatting + body: Use format specifiers instead of percent format + suggestion: Replace with format specifiers + fixable: true + location: + row: 85 + column: 6 + end_location: + row: 85 + column: 34 + fix: + content: "'Hello {arg}'.format(**bar['bop'])" + location: + row: 85 + column: 6 + end_location: + row: 85 + column: 34 + parent: ~