Pyupgrade: Printf string formatting (#1803)

Cargo.toml

@@ -49,9 +49,9 @@ regex = { version = "1.6.0" }
 ropey = { version = "1.5.0", features = ["cr_lines", "simd"], default-features = false }
 ruff_macros = { version = "0.0.228", path = "ruff_macros" }
 rustc-hash = { version = "1.1.0" }
-rustpython-ast = { features = ["unparse"], git = "https://github.com/RustPython/RustPython.git", rev = "ff90fe52eea578c8ebdd9d95e078cc041a5959fa" }
-rustpython-common = { git = "https://github.com/RustPython/RustPython.git", rev = "ff90fe52eea578c8ebdd9d95e078cc041a5959fa" }
-rustpython-parser = { features = ["lalrpop"], git = "https://github.com/RustPython/RustPython.git", rev = "ff90fe52eea578c8ebdd9d95e078cc041a5959fa" }
+rustpython-ast = { features = ["unparse"], git = "https://github.com/RustPython/RustPython.git", rev = "62aa942bf506ea3d41ed0503b947b84141fdaa3c" }
+rustpython-common = { git = "https://github.com/RustPython/RustPython.git", rev = "62aa942bf506ea3d41ed0503b947b84141fdaa3c" }
+rustpython-parser = { features = ["lalrpop"], git = "https://github.com/RustPython/RustPython.git", rev = "62aa942bf506ea3d41ed0503b947b84141fdaa3c" }
 schemars = { version = "0.8.11" }
 semver = { version = "1.0.16" }
 serde = { version = "1.0.147", features = ["derive"] }

README.md

@@ -727,6 +727,7 @@ For more, see [pyupgrade](https://pypi.org/project/pyupgrade/3.2.0/) on PyPI.
 | UP028 | rewrite-yield-from | Replace `yield` over `for` loop with `yield from` | 🛠 |
 | UP029 | unnecessary-builtin-import | Unnecessary builtin import: `{import}` | 🛠 |
 | UP030 | format-literals | Use implicit references for positional format fields | 🛠 |
+| UP031 | printf-string-formatting | Use format specifiers instead of percent format | 🛠 |
 | UP032 | f-string | Use f-string instead of `format` call | 🛠 |
 | UP033 | functools-cache | Use `@functools.cache` instead of `@functools.lru_cache(maxsize=None)` | 🛠 |
 | UP034 | extraneous-parentheses | Avoid extraneous parentheses | 🛠 |

resources/test/fixtures/pyupgrade/UP031_0.py

@@ -0,0 +1,69 @@
+a, b, x, y = 1, 2, 3, 4
+
+# UP031
+print('%s %s' % (a, b))
+
+print('%s%s' % (a, b))
+
+print("trivial" % ())
+
+print("%s" % ("simple",))
+
+print("%s" % ("%s" % ("nested",),))
+
+print("%s%% percent" % (15,))
+
+print("%f" % (15,))
+
+print("%.f" % (15,))
+
+print("%.3f" % (15,))
+
+print("%3f" % (15,))
+
+print("%-5f" % (5,))
+
+print("%9f" % (5,))
+
+print("%#o" % (123,))
+
+print("brace {} %s" % (1,))
+
+print(
+  "%s" % (
+    "trailing comma",
+        )
+)
+
+print("foo %s " % (x,))
+
+print("%(k)s" % {"k": "v"})
+
+print("%(k)s" % {
+    "k": "v",
+    "i": "j"
+})
+
+print("%(to_list)s" % {"to_list": []})
+
+print("%(k)s" % {"k": "v", "i": 1, "j": []})
+
+print("%(ab)s" % {"a" "b": 1})
+
+print("%(a)s" % {"a"  :  1})
+
+print((
+    "foo %s "
+    "bar %s" % (x, y)
+))
+
+print(
+    "foo %(foo)s "
+    "bar %(bar)s" % {"foo": x, "bar": y}
+)
+
+print("%s \N{snowman}" % (a,))
+
+print("%(foo)s \N{snowman}" % {"foo": 1})
+
+print(("foo %s " "bar %s") % (x, y))

resources/test/fixtures/pyupgrade/UP031_1.py

@@ -0,0 +1,59 @@
+# OK
+"%s" % unknown_type
+
+b"%s" % (b"bytestring",)
+
+"%*s" % (5, "hi")
+
+"%d" % (flt,)
+
+"%c" % (some_string,)
+
+"%4%" % ()
+
+"%.2r" % (1.25)
+
+i % 3
+
+"%.*s" % (5, "hi")
+
+"%i" % (flt,)
+
+"%()s" % {"": "empty"}
+
+"%s" % {"k": "v"}
+
+"%(1)s" % {"1": "bar"}
+
+"%(a)s" % {"a": 1, "a": 2}
+
+pytest.param('"%8s" % (None,)', id="unsafe width-string conversion"),
+
+"%()s" % {"": "bar"}
+
+"%(1)s" % {1: 2, "1": 2}
+
+"%(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,)
+)

ruff.schema.json

@@ -1780,6 +1780,7 @@
         "UP029",
         "UP03",
         "UP030",
+        "UP031",
         "UP032",
         "UP033",
         "UP034",

setup.py

@@ -13,6 +13,7 @@
 )
 sys.exit(1)
 
+"abc".isidentifier()
 
 # The below code will never execute, however GitHub is particularly
 # picky about where it finds Python packaging metadata.

src/checkers/ast.rs

@@ -2627,7 +2627,7 @@ where
                             .enabled(&Rule::PercentFormatUnsupportedFormatCharacter)
                     {
                         let location = Range::from_located(expr);
-                        match pyflakes::cformat::CFormatSummary::try_from(value.as_ref()) {
+                        match pyflakes::cformat::CFormatSummary::try_from(value.as_str()) {
                             Err(CFormatError {
                                 typ: CFormatErrorType::UnsupportedFormatChar(c),
                                 ..
@@ -2722,6 +2722,10 @@ where
                             }
                         }
                     }
+
+                    if self.settings.rules.enabled(&Rule::PrintfStringFormatting) {
+                        pyupgrade::rules::printf_string_formatting(self, expr, left, right);
+                    }
                 }
             }
             ExprKind::BinOp {

src/python/identifiers.rs

@@ -1,5 +1,15 @@
-use once_cell::sync::Lazy;
-use regex::Regex;
+/// Returns `true` if a string is a valid Python identifier (e.g., variable
+/// name).
+pub fn is_identifier(s: &str) -> bool {
+    // Is the first character a letter or underscore?
+    if !s
+        .chars()
+        .next()
+        .map_or(false, |c| c.is_alphabetic() || c == '_')
+    {
+        return false;
+    }
 
-pub static IDENTIFIER_REGEX: Lazy<Regex> =
-    Lazy::new(|| Regex::new(r"^[A-Za-z_][A-Za-z0-9_]*$").unwrap());
+    // Are the rest of the characters letters, digits, or underscores?
+    s.chars().skip(1).all(|c| c.is_alphanumeric() || c == '_')
+}

src/registry.rs

@@ -252,6 +252,7 @@ ruff_macros::define_rule_mapping!(
     UP028 => violations::RewriteYieldFrom,
     UP029 => violations::UnnecessaryBuiltinImport,
     UP030 => violations::FormatLiterals,
+    UP031 => violations::PrintfStringFormatting,
     UP032 => violations::FString,
     UP033 => violations::FunctoolsCache,
     UP034 => violations::ExtraneousParentheses,

src/rules/flake8_bugbear/rules/getattr_with_constant.rs

@@ -3,7 +3,7 @@ use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Location};
 use crate::ast::types::Range;
 use crate::checkers::ast::Checker;
 use crate::fix::Fix;
-use crate::python::identifiers::IDENTIFIER_REGEX;
+use crate::python::identifiers::is_identifier;
 use crate::python::keyword::KWLIST;
 use crate::registry::Diagnostic;
 use crate::source_code::Generator;
@@ -38,7 +38,7 @@ pub fn getattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar
     } = &arg.node else {
         return;
     };
-    if !IDENTIFIER_REGEX.is_match(value) {
+    if !is_identifier(value) {
         return;
     }
     if KWLIST.contains(&value.as_str()) {

src/rules/flake8_bugbear/rules/setattr_with_constant.rs

@@ -3,7 +3,7 @@ use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Location, Stmt, Stmt
 use crate::ast::types::Range;
 use crate::checkers::ast::Checker;
 use crate::fix::Fix;
-use crate::python::identifiers::IDENTIFIER_REGEX;
+use crate::python::identifiers::is_identifier;
 use crate::python::keyword::KWLIST;
 use crate::registry::Diagnostic;
 use crate::source_code::{Generator, Stylist};
@@ -49,7 +49,7 @@ pub fn setattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar
     } = &name.node else {
         return;
     };
-    if !IDENTIFIER_REGEX.is_match(name) {
+    if !is_identifier(name) {
         return;
     }
     if KWLIST.contains(&name.as_str()) {