From e160a52bfddb9e43398cf5f01b8c49956e0b71f3 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 12 Apr 2023 23:59:20 -0400 Subject: [PATCH] Raise percent-format upgrade rule (`UP031`) for hanging modulos (#3953) --- .../test/fixtures/pyupgrade/UP031_0.py | 23 ++++ .../test/fixtures/pyupgrade/UP031_1.py | 22 ---- crates/ruff/src/checkers/ast/mod.rs | 2 +- .../rules/printf_string_formatting.rs | 12 +- ...__rules__pyupgrade__tests__UP031_0.py.snap | 109 ++++++++++++++++++ scripts/add_rule.py | 17 ++- 6 files changed, 142 insertions(+), 43 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP031_0.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP031_0.py index e1a725a7aa3fd..dadc9bf37d9c5 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP031_0.py +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP031_0.py @@ -83,3 +83,26 @@ print('Hello %(arg)s' % bar) print('Hello %(arg)s' % bar.baz) print('Hello %(arg)s' % bar['bop']) + +# Hanging modulos +( + "foo %s " + "bar %s" +) % (x, y) + +( + "foo %(foo)s " + "bar %(bar)s" +) % {"foo": x, "bar": y} + +( + """foo %s""" + % (x,) +) + +( + """ + foo %s + """ + % (x,) +) diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP031_1.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP031_1.py index ecf7a91407c0e..57f25df712c0b 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP031_1.py +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP031_1.py @@ -34,28 +34,6 @@ "%(and)s" % {"and": 2} # OK (arguably false negatives) -( - "foo %s " - "bar %s" -) % (x, y) - -( - "foo %(foo)s " - "bar %(bar)s" -) % {"foo": x, "bar": y} - -( - """foo %s""" - % (x,) -) - -( - """ - foo %s - """ - % (x,) -) - 'Hello %s' % bar 'Hello %s' % bar.baz diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 268977a00b3b3..132f81f718680 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -3234,7 +3234,7 @@ where } if self.settings.rules.enabled(Rule::PrintfStringFormatting) { - pyupgrade::rules::printf_string_formatting(self, expr, left, right); + pyupgrade::rules::printf_string_formatting(self, expr, right); } if self.settings.rules.enabled(Rule::BadStringFormatType) { pylint::rules::bad_string_format_type(self, expr, right); 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 3f739231b942f..142e4752e0c9e 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/printf_string_formatting.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/printf_string_formatting.rs @@ -296,17 +296,7 @@ fn convertible(format_string: &CFormatString, params: &Expr) -> bool { } /// UP031 -pub(crate) fn printf_string_formatting( - checker: &mut Checker, - expr: &Expr, - left: &Expr, - right: &Expr, -) { - // If the modulo symbol is on a separate line, abort. - if right.location.row() != left.end_location.unwrap().row() { - return; - } - +pub(crate) fn printf_string_formatting(checker: &mut Checker, expr: &Expr, right: &Expr) { // Grab each string segment (in case there's an implicit concatenation). let mut strings: Vec<(Location, Location)> = vec![]; let mut extension = None; 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 10983da7615c3..83ad29bcc9848 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 @@ -750,6 +750,7 @@ UP031_0.py:83:7: UP031 [*] Use format specifiers instead of percent format 83 |+print('Hello {arg}'.format(**bar)) 84 84 | print('Hello %(arg)s' % bar.baz) 85 85 | print('Hello %(arg)s' % bar['bop']) +86 86 | UP031_0.py:84:7: UP031 [*] Use format specifiers instead of percent format | @@ -768,6 +769,8 @@ UP031_0.py:84:7: UP031 [*] Use format specifiers instead of percent format 84 |-print('Hello %(arg)s' % bar.baz) 84 |+print('Hello {arg}'.format(**bar.baz)) 85 85 | print('Hello %(arg)s' % bar['bop']) +86 86 | +87 87 | # Hanging modulos UP031_0.py:85:7: UP031 [*] Use format specifiers instead of percent format | @@ -775,6 +778,8 @@ UP031_0.py:85:7: UP031 [*] Use format specifiers instead of percent format 86 | print('Hello %(arg)s' % bar.baz) 87 | print('Hello %(arg)s' % bar['bop']) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP031 +88 | +89 | # Hanging modulos | = help: Replace with format specifiers @@ -784,5 +789,109 @@ UP031_0.py:85:7: UP031 [*] Use format specifiers instead of percent format 84 84 | print('Hello %(arg)s' % bar.baz) 85 |-print('Hello %(arg)s' % bar['bop']) 85 |+print('Hello {arg}'.format(**bar['bop'])) +86 86 | +87 87 | # Hanging modulos +88 88 | ( + +UP031_0.py:88:1: UP031 [*] Use format specifiers instead of percent format + | +88 | # Hanging modulos +89 | / ( +90 | | "foo %s " +91 | | "bar %s" +92 | | ) % (x, y) + | |__________^ UP031 +93 | +94 | ( + | + = help: Replace with format specifiers + +ℹ Suggested fix +86 86 | +87 87 | # Hanging modulos +88 88 | ( +89 |- "foo %s " +90 |- "bar %s" +91 |-) % (x, y) + 89 |+ "foo {} " + 90 |+ "bar {}" + 91 |+).format(x, y) +92 92 | +93 93 | ( +94 94 | "foo %(foo)s " + +UP031_0.py:93:1: UP031 [*] Use format specifiers instead of percent format + | + 93 | ) % (x, y) + 94 | + 95 | / ( + 96 | | "foo %(foo)s " + 97 | | "bar %(bar)s" + 98 | | ) % {"foo": x, "bar": y} + | |________________________^ UP031 + 99 | +100 | ( + | + = help: Replace with format specifiers + +ℹ Suggested fix +91 91 | ) % (x, y) +92 92 | +93 93 | ( +94 |- "foo %(foo)s " +95 |- "bar %(bar)s" +96 |-) % {"foo": x, "bar": y} + 94 |+ "foo {foo} " + 95 |+ "bar {bar}" + 96 |+).format(foo=x, bar=y) +97 97 | +98 98 | ( +99 99 | """foo %s""" + +UP031_0.py:99:5: UP031 [*] Use format specifiers instead of percent format + | + 99 | ( +100 | """foo %s""" + | _____^ +101 | | % (x,) + | |__________^ UP031 +102 | ) + | + = help: Replace with format specifiers + +ℹ Suggested fix +96 96 | ) % {"foo": x, "bar": y} +97 97 | +98 98 | ( +99 |- """foo %s""" +100 |- % (x,) + 99 |+ """foo {}""".format(x) +101 100 | ) +102 101 | +103 102 | ( + +UP031_0.py:104:5: UP031 [*] Use format specifiers instead of percent format + | +104 | ( +105 | """ + | _____^ +106 | | foo %s +107 | | """ +108 | | % (x,) + | |__________^ UP031 +109 | ) + | + = help: Replace with format specifiers + +ℹ Suggested fix +102 102 | +103 103 | ( +104 104 | """ +105 |- foo %s +106 |- """ +107 |- % (x,) + 105 |+ foo {} + 106 |+ """.format(x) +108 107 | ) diff --git a/scripts/add_rule.py b/scripts/add_rule.py index 40b277924e175..86ad8a3bf8805 100755 --- a/scripts/add_rule.py +++ b/scripts/add_rule.py @@ -103,23 +103,22 @@ def main(*, name: str, prefix: str, code: str, linter: str) -> None: # Add the relevant rule function. with (rules_dir / f"{rule_name_snake}.rs").open("w") as fp: fp.write( - """\ + f"""\ use ruff_diagnostics::Violation; -use ruff_macros::{derive_message_formats, violation}; +use ruff_macros::{{derive_message_formats, violation}}; use crate::checkers::ast::Checker; #[violation] -pub struct %s; -impl Violation for %s { +pub struct {name}; +impl Violation for {name} {{ #[derive_message_formats] - fn message(&self) -> String { + fn message(&self) -> String {{ todo!("implement message"); format!("TODO: write message") - } -} -""" - % (name, name), + }} +}} +""", ) fp.write( f"""