From e3733526ed62335b848812173f4b1c502a33e6c9 Mon Sep 17 00:00:00 2001 From: Aarni Koskela Date: Tue, 2 May 2023 23:07:30 +0300 Subject: [PATCH] Implement static_join_to_fstring --- .../resources/test/fixtures/flynt/FLY002.py | 10 +++ crates/ruff/src/checkers/ast/mod.rs | 8 ++ crates/ruff/src/codes.rs | 1 + crates/ruff/src/registry.rs | 1 + crates/ruff/src/rules/flynt/mod.rs | 1 + crates/ruff/src/rules/flynt/rules/mod.rs | 2 + .../flynt/rules/static_join_to_fstring.rs | 83 +++++++++++++++++++ ...rules__flynt__tests__FLY002_FLY002.py.snap | 73 ++++++++++++++++ 8 files changed, 179 insertions(+) create mode 100644 crates/ruff/resources/test/fixtures/flynt/FLY002.py create mode 100644 crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs create mode 100644 crates/ruff/src/rules/flynt/snapshots/ruff__rules__flynt__tests__FLY002_FLY002.py.snap diff --git a/crates/ruff/resources/test/fixtures/flynt/FLY002.py b/crates/ruff/resources/test/fixtures/flynt/FLY002.py new file mode 100644 index 00000000000000..c5eb5a3b695adb --- /dev/null +++ b/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) + diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index ce9c913f4c3c8d..2b42d9adcf23ed 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -2436,6 +2436,8 @@ where // pyupgrade Rule::FormatLiterals, Rule::FString, + // flynt + Rule::StaticJoinToFString, ]) { if let ExprKind::Attribute { value, attr, .. } = &func.node { if let ExprKind::Constant { @@ -2443,6 +2445,12 @@ where .. } = &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(); diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 3bb0269fb8af06..52ed253a67b1e9 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -733,6 +733,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { // flynt (Flynt, "001") => Rule::StringConcatenationToFString, + (Flynt, "002") => Rule::StaticJoinToFString, _ => return None, }) diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 86ee244e24646a..65d44d4e3520bf 100644 --- a/crates/ruff/src/registry.rs +++ b/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 { diff --git a/crates/ruff/src/rules/flynt/mod.rs b/crates/ruff/src/rules/flynt/mod.rs index b86ea3b26ce42a..96707091dd8c6e 100644 --- a/crates/ruff/src/rules/flynt/mod.rs +++ b/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( diff --git a/crates/ruff/src/rules/flynt/rules/mod.rs b/crates/ruff/src/rules/flynt/rules/mod.rs index c8c92e4a863f9b..d422d5b6f3deca 100644 --- a/crates/ruff/src/rules/flynt/rules/mod.rs +++ b/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}; diff --git a/crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs b/crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs new file mode 100644 index 00000000000000..1f50bd6a3135a7 --- /dev/null +++ b/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 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); +} diff --git a/crates/ruff/src/rules/flynt/snapshots/ruff__rules__flynt__tests__FLY002_FLY002.py.snap b/crates/ruff/src/rules/flynt/snapshots/ruff__rules__flynt__tests__FLY002_FLY002.py.snap new file mode 100644 index 00000000000000..554e7247a07d1d --- /dev/null +++ b/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) + | + +