From 67b43ab72aef2db5635a12e5a43e32324ae326f9 Mon Sep 17 00:00:00 2001 From: Evan Rittenhouse Date: Sat, 3 Jun 2023 15:35:06 -0500 Subject: [PATCH] Make FLY002 autofix into a constant string instead of an f-string if all `join()` arguments are strings (#4834) --- .../flynt/rules/static_join_to_fstring.rs | 42 +++++++++++++++++-- ...rules__flynt__tests__FLY002_FLY002.py.snap | 6 +-- 2 files changed, 41 insertions(+), 7 deletions(-) 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 index cd05bace7c..2faab59911 100644 --- a/crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs +++ b/crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs @@ -1,5 +1,6 @@ +use itertools::Itertools; use ruff_text_size::TextRange; -use rustpython_parser::ast::{self, Expr, Ranged}; +use rustpython_parser::ast::{self, Constant, Expr, Ranged}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; @@ -27,15 +28,48 @@ impl AlwaysAutofixableViolation for StaticJoinToFString { } fn is_static_length(elts: &[Expr]) -> bool { - elts.iter().all(|e| !matches!(e, Expr::Starred(_))) + elts.iter().all(|e| !e.is_starred_expr()) } fn build_fstring(joiner: &str, joinees: &[Expr]) -> Option { + // If all elements are string constants, join them into a single string. + if joinees.iter().all(|expr| { + matches!( + expr, + Expr::Constant(ast::ExprConstant { + value: Constant::Str(_), + .. + }) + ) + }) { + let node = ast::ExprConstant { + value: Constant::Str( + joinees + .iter() + .filter_map(|expr| { + if let Expr::Constant(ast::ExprConstant { + value: Constant::Str(string), + .. + }) = expr + { + Some(string.as_str()) + } else { + None + } + }) + .join(joiner), + ), + range: TextRange::default(), + kind: None, + }; + return Some(node.into()); + } + let mut fstring_elems = Vec::with_capacity(joinees.len() * 2); let mut first = true; for expr in joinees { - if matches!(expr, Expr::JoinedStr(_)) { + if expr.is_joined_str_expr() { // Oops, already an f-string. We don't know how to handle those // gracefully right now. return None; @@ -58,7 +92,7 @@ pub(crate) fn static_join_to_fstring(checker: &mut Checker, expr: &Expr, joiner: args, keywords, .. - })= expr else { + }) = expr else { return; }; 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 index d268b251ba..e987f86206 100644 --- 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 @@ -42,7 +42,7 @@ FLY002.py:6:7: FLY002 [*] Consider `f"Finally, {a} World"` instead of string joi 8 8 | ok4 = "y".join([1, 2, 3]) # Technically OK, though would've been an error originally 9 9 | ok5 = "a".join([random(), random()]) # OK (simple calls) -FLY002.py:7:7: FLY002 [*] Consider `f"1x2x3"` instead of string join +FLY002.py:7:7: FLY002 [*] Consider `"1x2x3"` instead of string join | 7 | ok1 = " ".join([a, " World"]) # OK 8 | ok2 = "".join(["Finally, ", a, " World"]) # OK @@ -51,14 +51,14 @@ FLY002.py:7:7: FLY002 [*] Consider `f"1x2x3"` instead of string join 10 | ok4 = "y".join([1, 2, 3]) # Technically OK, though would've been an error originally 11 | ok5 = "a".join([random(), random()]) # OK (simple calls) | - = help: Replace with `f"1x2x3"` + = help: Replace with `"1x2x3"` ℹ Suggested fix 4 4 | a = "Hello" 5 5 | ok1 = " ".join([a, " World"]) # OK 6 6 | ok2 = "".join(["Finally, ", a, " World"]) # OK 7 |-ok3 = "x".join(("1", "2", "3")) # OK - 7 |+ok3 = f"1x2x3" # OK + 7 |+ok3 = "1x2x3" # OK 8 8 | ok4 = "y".join([1, 2, 3]) # Technically OK, though would've been an error originally 9 9 | ok5 = "a".join([random(), random()]) # OK (simple calls) 10 10 | ok6 = "a".join([secrets.token_urlsafe(), secrets.token_hex()]) # OK (attr calls)