Skip to content

Commit

Permalink
Make FLY002 emit a string (instead of an f-string) if all join() argu…
Browse files Browse the repository at this point in the history
…ments are strings
  • Loading branch information
evanrittenhouse committed Jun 3, 2023
1 parent fcfd6ad commit 841714b
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 9 deletions.
32 changes: 26 additions & 6 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 @@ -33,6 +34,7 @@ fn is_static_length(elts: &[Expr]) -> bool {
fn build_fstring(joiner: &str, joinees: &[Expr]) -> Option<Expr> {
let mut fstring_elems = Vec::with_capacity(joinees.len() * 2);
let mut first = true;
let mut strings: Vec<&String> = vec![];

for expr in joinees {
if matches!(expr, Expr::JoinedStr(_)) {
Expand All @@ -44,21 +46,39 @@ fn build_fstring(joiner: &str, joinees: &[Expr]) -> Option<Expr> {
fstring_elems.push(helpers::to_constant_string(joiner));
}
fstring_elems.push(helpers::to_fstring_elem(expr)?);

if let Expr::Constant(ast::ExprConstant {
value: Constant::Str(value),
..
}) = expr
{
strings.push(value);
}
}

let node = ast::ExprJoinedStr {
values: fstring_elems,
range: TextRange::default(),
let node = if strings.len() == joinees.len() {
ast::Expr::Constant(ast::ExprConstant {
value: Constant::Str(strings.iter().join(joiner)),
range: TextRange::default(),
kind: None,
})
} else {
ast::ExprJoinedStr {
values: fstring_elems,
range: TextRange::default(),
}
.into()
};
Some(node.into())

Some(node)
}

pub(crate) fn static_join_to_fstring(checker: &mut Checker, expr: &Expr, joiner: &str) {
let Expr::Call(ast::ExprCall {
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 841714b

Please sign in to comment.