Skip to content

Commit

Permalink
Make FLY002 autofix into a constant string instead of an f-string if …
Browse files Browse the repository at this point in the history
…all `join()` arguments are strings (#4834)
  • Loading branch information
evanrittenhouse authored and konstin committed Jun 13, 2023
1 parent d42d0ab commit 9d6bb4f
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 7 deletions.
42 changes: 38 additions & 4 deletions crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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<Expr> {
// 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;
Expand All @@ -58,7 +92,7 @@ pub(crate) fn static_join_to_fstring(checker: &mut Checker, expr: &Expr, joiner:
args,
keywords,
..
})= expr else {
}) = expr else {
return;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down

0 comments on commit 9d6bb4f

Please sign in to comment.