Skip to content

Commit

Permalink
Implement static_join_to_fstring
Browse files Browse the repository at this point in the history
  • Loading branch information
akx committed May 2, 2023
1 parent 776b435 commit e373352
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 0 deletions.
10 changes: 10 additions & 0 deletions crates/ruff/resources/test/fixtures/flynt/FLY002.py
Original file line number Diff line number Diff line change
@@ -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)

8 changes: 8 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/flynt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flynt/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -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};
83 changes: 83 additions & 0 deletions crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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)
|


0 comments on commit e373352

Please sign in to comment.