Implement static_join_to_fstring

Author: akx

crates/ruff/resources/test/fixtures/flynt/FLY002.py

@@ -0,0 +1,10 @@
+a = "Hello"
+msg1 = " ".join([a, " World"])
+msg2 = "".join(["Finally, ", a, " World"])
+msg3 = "x".join(("1", "2", "3"))
+msg4 = "x".join({"4", "5", "yee"})
+msg5 = "y".join([1, 2, 3])  # Should be transformed
+msg6 = a.join(["1", "2", "3"])  # Should not be transformed (not a static joiner)
+msg7 = "a".join(a)  # Should not be transformed (not a static joinee)
+msg8 = "a".join([a, a, *a])  # Should not be transformed (not a static length)
+

crates/ruff/src/checkers/ast/mod.rs

@@ -2436,13 +2436,21 @@ where
                     // pyupgrade
                     Rule::FormatLiterals,
                     Rule::FString,
+                    // flynt
+                    Rule::StaticJoinToFString,
                 ]) {
                     if let ExprKind::Attribute { value, attr, .. } = &func.node {
                         if let ExprKind::Constant {
                             value: Constant::Str(value),
                             ..
                         } = &value.node
                         {
+                            if attr == "join" {
+                                // "...".join(...) call
+                                if self.settings.rules.enabled(Rule::StaticJoinToFString) {
+                                    flynt::rules::static_join_to_fstring(self, expr, value);
+                                }
+                            }
                             if attr == "format" {
                                 // "...".format(...) call
                                 let location = expr.range();

crates/ruff/src/codes.rs

@@ -733,6 +733,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
 
         // flynt
         (Flynt, "001") => Rule::StringConcatenationToFString,
+        (Flynt, "002") => Rule::StaticJoinToFString,
 
         _ => return None,
     })

crates/ruff/src/registry.rs

@@ -667,6 +667,7 @@ ruff_macros::register_rules!(
     rules::flake8_django::rules::DjangoNonLeadingReceiverDecorator,
     // flynt
     rules::flynt::rules::StringConcatenationToFString,
+    rules::flynt::rules::StaticJoinToFString,
 );
 
 pub trait AsRule {

crates/ruff/src/rules/flynt/mod.rs

@@ -12,6 +12,7 @@ mod tests {
     use test_case::test_case;
 
     #[test_case(Rule::StringConcatenationToFString, Path::new("FLY001.py"); "FLY001")]
+    #[test_case(Rule::StaticJoinToFString, Path::new("FLY002.py"); "FLY002")]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff/src/rules/flynt/rules/mod.rs

@@ -1,3 +1,5 @@
 mod concat_to_fstring;
+mod static_join_to_fstring;
 
 pub use concat_to_fstring::{string_concatenation_to_fstring, StringConcatenationToFString};
+pub use static_join_to_fstring::{static_join_to_fstring, StaticJoinToFString};

crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs

@@ -0,0 +1,83 @@
+use rustpython_parser::ast::{Expr, ExprKind};
+
+use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::helpers::{create_expr, has_comments, unparse_expr};
+
+use crate::checkers::ast::Checker;
+use crate::registry::AsRule;
+use crate::rules::flynt::helpers;
+use crate::rules::flynt::helpers::to_constant_string;
+
+#[violation]
+pub struct StaticJoinToFString {
+    pub expr: String,
+    pub fixable: bool,
+}
+
+impl Violation for StaticJoinToFString {
+    const AUTOFIX: AutofixKind = AutofixKind::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let StaticJoinToFString { expr, .. } = self;
+        format!("Consider `{expr}` instead of string join")
+    }
+
+    fn autofix_title_formatter(&self) -> Option<fn(&Self) -> String> {
+        self.fixable
+            .then_some(|StaticJoinToFString { expr, .. }| format!("Replace with `{expr}`"))
+    }
+}
+
+fn is_static_length(elts: &[Expr]) -> bool {
+    elts.iter()
+        .all(|e| !matches!(e.node, ExprKind::Starred { .. }))
+}
+
+pub fn static_join_to_fstring(checker: &mut Checker, expr: &Expr, joiner: &str) {
+    let ExprKind::Call {
+        func: _,
+        args,
+        keywords,
+    } = &expr.node else {
+        return;
+    };
+    if !keywords.is_empty() || args.len() != 1 {
+        // Not a string join call we know of, this...
+        return;
+    }
+    let joinees = match &args[0].node {
+        ExprKind::List { elts, .. } if is_static_length(elts) => elts,
+        ExprKind::Tuple { elts, .. } if is_static_length(elts) => elts,
+        _ => return,
+    };
+    let mut fstring_elems = Vec::with_capacity(joinees.len() * 2);
+    for (i, expr) in joinees.iter().enumerate() {
+        let elem = helpers::to_fstring_elem(expr.clone());
+        if i != 0 {
+            fstring_elems.push(to_constant_string(joiner));
+        }
+        fstring_elems.push(elem);
+    }
+    let new_expr = create_expr(ExprKind::JoinedStr {
+        values: fstring_elems,
+    });
+
+    let contents = unparse_expr(&new_expr, checker.stylist);
+    let fixable = !has_comments(expr, checker.locator);
+
+    let mut diagnostic = Diagnostic::new(
+        StaticJoinToFString {
+            expr: contents.clone(),
+            fixable,
+        },
+        expr.range(),
+    );
+    if checker.patch(diagnostic.kind.rule()) {
+        if fixable {
+            diagnostic.set_fix(Edit::range_replacement(contents, expr.range()));
+        }
+    }
+    checker.diagnostics.push(diagnostic);
+}

crates/ruff/src/rules/flynt/snapshots/ruff__rules__flynt__tests__FLY002_FLY002.py.snap

@@ -0,0 +1,73 @@
+---
+source: crates/ruff/src/rules/flynt/mod.rs
+---
+FLY002.py:2:8: FLY002 [*] Consider `f"{a}  World"` instead of string join
+  |
+2 | a = "Hello"
+3 | msg1 = " ".join([a, " World"])
+  |        ^^^^^^^^^^^^^^^^^^^^^^^ FLY002
+4 | msg2 = "".join(["Finally, ", a, " World"])
+5 | msg3 = "x".join(("1", "2", "3"))
+  |
+  = help: Replace with `f"{a}  World"`
+
+ℹ Suggested fix
+1 1 | a = "Hello"
+2   |-msg1 = " ".join([a, " World"])
+  2 |+msg1 = f"{a}  World"
+3 3 | msg2 = "".join(["Finally, ", a, " World"])
+4 4 | msg3 = "x".join(("1", "2", "3"))
+5 5 | msg4 = "x".join({"4", "5", "yee"})
+
+FLY002.py:3:8: FLY002 [*] Consider `f"Finally, {a} World"` instead of string join
+  |
+3 | a = "Hello"
+4 | msg1 = " ".join([a, " World"])
+5 | msg2 = "".join(["Finally, ", a, " World"])
+  |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FLY002
+6 | msg3 = "x".join(("1", "2", "3"))
+7 | msg4 = "x".join({"4", "5", "yee"})
+  |
+  = help: Replace with `f"Finally, {a} World"`
+
+ℹ Suggested fix
+1 1 | a = "Hello"
+2 2 | msg1 = " ".join([a, " World"])
+3   |-msg2 = "".join(["Finally, ", a, " World"])
+  3 |+msg2 = f"Finally, {a} World"
+4 4 | msg3 = "x".join(("1", "2", "3"))
+5 5 | msg4 = "x".join({"4", "5", "yee"})
+6 6 | msg5 = "y".join([1, 2, 3])  # Should be transformed
+
+FLY002.py:4:8: FLY002 [*] Consider `f"1x2x3"` instead of string join
+  |
+4 | msg1 = " ".join([a, " World"])
+5 | msg2 = "".join(["Finally, ", a, " World"])
+6 | msg3 = "x".join(("1", "2", "3"))
+  |        ^^^^^^^^^^^^^^^^^^^^^^^^^ FLY002
+7 | msg4 = "x".join({"4", "5", "yee"})
+8 | msg5 = "y".join([1, 2, 3])  # Should be transformed
+  |
+  = help: Replace with `f"1x2x3"`
+
+ℹ Suggested fix
+1 1 | a = "Hello"
+2 2 | msg1 = " ".join([a, " World"])
+3 3 | msg2 = "".join(["Finally, ", a, " World"])
+4   |-msg3 = "x".join(("1", "2", "3"))
+  4 |+msg3 = f"1x2x3"
+5 5 | msg4 = "x".join({"4", "5", "yee"})
+6 6 | msg5 = "y".join([1, 2, 3])  # Should be transformed
+7 7 | msg6 = a.join(["1", "2", "3"])  # Should not be transformed (not a static joiner)
+
+FLY002.py:6:8: FLY002 Consider `f"{1}y{2}y{3}"` instead of string join
+   |
+ 6 | msg3 = "x".join(("1", "2", "3"))
+ 7 | msg4 = "x".join({"4", "5", "yee"})
+ 8 | msg5 = "y".join([1, 2, 3])  # Should be transformed
+   |        ^^^^^^^^^^^^^^^^^^^ FLY002
+ 9 | msg6 = a.join(["1", "2", "3"])  # Should not be transformed (not a static joiner)
+10 | msg7 = "a".join(a)  # Should not be transformed (not a static joinee)
+   |
+
+