Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Implement Flynt static string join transform as FLY002 (#4196)

This commit is contained in:
Aarni Koskela 2023-05-09 03:46:38 +03:00 committed by GitHub
parent 61f21a6513
commit efdf383f5e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 364 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
crates/ruff/resources/test/fixtures/flynt/FLY002.py vendored Normal file
View File

@ -0,0 +1,18 @@
import secrets
from random import random, choice
a = "Hello"
ok1 = " ".join([a, " World"]) # OK
ok2 = "".join(["Finally, ", a, " World"]) # OK
ok3 = "x".join(("1", "2", "3")) # OK
ok4 = "y".join([1, 2, 3]) # Technically OK, though would've been an error originally
ok5 = "a".join([random(), random()]) # OK (simple calls)
ok6 = "a".join([secrets.token_urlsafe(), secrets.token_hex()]) # OK (attr calls)
nok1 = "x".join({"4", "5", "yee"}) # Not OK (set)
nok2 = a.join(["1", "2", "3"]) # Not OK (not a static joiner)
nok3 = "a".join(a) # Not OK (not a static joinee)
nok4 = "a".join([a, a, *a]) # Not OK (not a static length)
nok5 = "a".join([choice("flarp")]) # Not OK (not a simple call)
nok6 = "a".join(x for x in "feefoofum") # Not OK (generator)
nok7 = "a".join([f"foo{8}", "bar"]) # Not OK (contains an f-string)

15
crates/ruff/src/checkers/ast/mod.rs
View File

@ -45,9 +45,9 @@ use crate::rules::{
flake8_django, flake8_errmsg, flake8_gettext, flake8_implicit_str_concat,
flake8_import_conventions, flake8_logging_format, flake8_pie, flake8_print, flake8_pyi,
flake8_pytest_style, flake8_raise, flake8_return, flake8_self, flake8_simplify,
flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, flake8_use_pathlib, mccabe,
numpy, pandas_vet, pep8_naming, pycodestyle, pydocstyle, pyflakes, pygrep_hooks, pylint,
pyupgrade, ruff, tryceratops,
flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, flake8_use_pathlib, flynt,
mccabe, numpy, pandas_vet, pep8_naming, pycodestyle, pydocstyle, pyflakes, pygrep_hooks,
pylint, pyupgrade, ruff, tryceratops,
};
use crate::settings::types::PythonVersion;
use crate::settings::{flags, Settings};
@ -2471,6 +2471,8 @@ where
// pyupgrade
Rule::FormatLiterals,
Rule::FString,
// flynt
Rule::StaticJoinToFString,
]) {
if let ExprKind::Attribute { value, attr, .. } = &func.node {
if let ExprKind::Constant {
@ -2478,7 +2480,12 @@ where
..
} = &value.node
{
if attr == "format" {
if attr == "join" {
// "...".join(...) call
if self.settings.rules.enabled(Rule::StaticJoinToFString) {
flynt::rules::static_join_to_fstring(self, expr, value);
}
} else if attr == "format" {
// "...".format(...) call
let location = expr.range();
match pyflakes::format::FormatSummary::try_from(value.as_ref()) {

4
crates/ruff/src/codes.rs
View File

@ -737,6 +737,10 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Flake8Django, "012") => Rule::DjangoUnorderedBodyContentInModel,
(Flake8Django, "013") => Rule::DjangoNonLeadingReceiverDecorator,
// flynt
// Reserved: (Flynt, "001") => Rule::StringConcatenationToFString,
(Flynt, "002") => Rule::StaticJoinToFString,
_ => return None,
})
}

5
crates/ruff/src/registry.rs
View File

@ -670,6 +670,8 @@ ruff_macros::register_rules!(
rules::flake8_django::rules::DjangoModelWithoutDunderStr,
rules::flake8_django::rules::DjangoUnorderedBodyContentInModel,
rules::flake8_django::rules::DjangoNonLeadingReceiverDecorator,
// flynt
rules::flynt::rules::StaticJoinToFString,
);
pub trait AsRule {
@ -825,6 +827,9 @@ pub enum Linter {
/// [tryceratops](https://pypi.org/project/tryceratops/1.1.0/)
#[prefix = "TRY"]
Tryceratops,
/// [flynt](https://pypi.org/project/flynt/)
#[prefix = "FLY"]
Flynt,
/// NumPy-specific rules
#[prefix = "NPY"]
Numpy,

6
crates/ruff/src/registry/rule_set.rs
View File

@ -3,14 +3,16 @@ use ruff_macros::CacheKey;
use std::fmt::{Debug, Formatter};
use std::iter::FusedIterator;
const RULESET_SIZE: usize = 10;
/// A set of [`Rule`]s.
///
/// Uses a bitset where a bit of one signals that the Rule with that [u16] is in this set.
#[derive(Clone, Default, CacheKey, PartialEq, Eq)]
pub struct RuleSet([u64; 10]);
pub struct RuleSet([u64; RULESET_SIZE]);
impl RuleSet {
const EMPTY: [u64; 10] = [0; 10];
const EMPTY: [u64; RULESET_SIZE] = [0; RULESET_SIZE];
// 64 fits into a u16 without truncation
#[allow(clippy::cast_possible_truncation)]

65
crates/ruff/src/rules/flynt/helpers.rs Normal file
View File

@ -0,0 +1,65 @@
use ruff_python_ast::helpers::create_expr;
use rustpython_parser::ast::{Constant, Expr, ExprKind};
/// Wrap an expression in a `FormattedValue` with no special formatting.
fn to_formatted_value_expr(inner: &Expr) -> Expr {
create_expr(ExprKind::FormattedValue {
value: Box::new(inner.clone()),
conversion: 0,
format_spec: None,
})
}
/// Convert a string to a constant string expression.
pub(crate) fn to_constant_string(s: &str) -> Expr {
create_expr(ExprKind::Constant {
value: Constant::Str(s.to_owned()),
kind: None,
})
}
/// Figure out if `expr` represents a "simple" call
/// (i.e. one that can be safely converted to a formatted value).
fn is_simple_call(expr: &Expr) -> bool {
match &expr.node {
ExprKind::Call {
func,
args,
keywords,
} => args.is_empty() && keywords.is_empty() && is_simple_callee(func),
_ => false,
}
}
/// Figure out if `func` represents a "simple" callee (a bare name, or a chain of simple
/// attribute accesses).
fn is_simple_callee(func: &Expr) -> bool {
match &func.node {
ExprKind::Name { .. } => true,
ExprKind::Attribute { value, .. } => is_simple_callee(value),
_ => false,
}
}
/// Convert an expression to a f-string element (if it looks like a good idea).
pub(crate) fn to_fstring_elem(expr: &Expr) -> Option<Expr> {
match &expr.node {
// These are directly handled by `unparse_fstring_elem`:
ExprKind::Constant {
value: Constant::Str(_),
..
}
| ExprKind::JoinedStr { .. }
| ExprKind::FormattedValue { .. } => Some(expr.clone()),
// These should be pretty safe to wrap in a formatted value.
ExprKind::Constant {
value:
Constant::Int(_) | Constant::Float(_) | Constant::Bool(_) | Constant::Complex { .. },
..
}
| ExprKind::Name { .. }
| ExprKind::Attribute { .. } => Some(to_formatted_value_expr(expr)),
ExprKind::Call { .. } if is_simple_call(expr) => Some(to_formatted_value_expr(expr)),
_ => None,
}
}

24
crates/ruff/src/rules/flynt/mod.rs Normal file
View File

@ -0,0 +1,24 @@
//! Rules from [flynt](https://pypi.org/project/flynt/).
mod helpers;
pub(crate) mod rules;
#[cfg(test)]
mod tests {
use crate::registry::Rule;
use crate::test::test_path;
use crate::{assert_messages, settings};
use anyhow::Result;
use std::path::Path;
use test_case::test_case;
#[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(
Path::new("flynt").join(path).as_path(),
&settings::Settings::for_rule(rule_code),
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
}

3
crates/ruff/src/rules/flynt/rules/mod.rs Normal file
View File

@ -0,0 +1,3 @@
mod static_join_to_fstring;
pub use static_join_to_fstring::{static_join_to_fstring, StaticJoinToFString};

97
crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs Normal file
View File

@ -0,0 +1,97 @@
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, unparse_expr};
use crate::checkers::ast::Checker;
use crate::registry::AsRule;
use crate::rules::flynt::helpers;
#[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 { .. }))
}
fn build_fstring(joiner: &str, joinees: &[Expr]) -> Option<Expr> {
let mut fstring_elems = Vec::with_capacity(joinees.len() * 2);
let mut first = true;
for expr in joinees {
if matches!(expr.node, ExprKind::JoinedStr { .. }) {
// Oops, already an f-string. We don't know how to handle those
// gracefully right now.
return None;
}
if !first {
fstring_elems.push(helpers::to_constant_string(joiner));
}
fstring_elems.push(helpers::to_fstring_elem(expr)?);
first = false;
}
Some(create_expr(ExprKind::JoinedStr {
values: fstring_elems,
}))
}
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 {
// If there are kwargs or more than one argument,
// this is some non-standard string join call.
return;
}
// Get the elements to join; skip e.g. generators, sets, etc.
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,
};
// Try to build the fstring (internally checks whether e.g. the elements are
// convertible to f-string parts).
let Some(new_expr) = build_fstring(joiner, joinees) else { return };
let contents = unparse_expr(&new_expr, checker.stylist);
let mut diagnostic = Diagnostic::new(
StaticJoinToFString {
expr: contents.clone(),
fixable: true,
},
expr.range(),
);
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Edit::range_replacement(contents, expr.range()));
}
checker.diagnostics.push(diagnostic);
}

128
crates/ruff/src/rules/flynt/snapshots/ruff__rules__flynt__tests__FLY002_FLY002.py.snap Normal file
View File

@ -0,0 +1,128 @@
---
source: crates/ruff/src/rules/flynt/mod.rs
---
FLY002.py:5:7: FLY002 [*] Consider `f"{a} World"` instead of string join
|
5 | a = "Hello"
6 | ok1 = " ".join([a, " World"]) # OK
| ^^^^^^^^^^^^^^^^^^^^^^^ FLY002
7 | ok2 = "".join(["Finally, ", a, " World"]) # OK
8 | ok3 = "x".join(("1", "2", "3")) # OK
|
= help: Replace with `f"{a} World"`
ℹ Suggested fix
2 2 | from random import random, choice
3 3 |
4 4 | a = "Hello"
5 |-ok1 = " ".join([a, " World"]) # OK
5 |+ok1 = f"{a} World" # OK
6 6 | ok2 = "".join(["Finally, ", a, " World"]) # OK
7 7 | ok3 = "x".join(("1", "2", "3")) # OK
8 8 | ok4 = "y".join([1, 2, 3]) # Technically OK, though would've been an error originally
FLY002.py:6:7: FLY002 [*] Consider `f"Finally, {a} World"` instead of string join
|
6 | a = "Hello"
7 | ok1 = " ".join([a, " World"]) # OK
8 | ok2 = "".join(["Finally, ", a, " World"]) # OK
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FLY002
9 | ok3 = "x".join(("1", "2", "3")) # OK
10 | ok4 = "y".join([1, 2, 3]) # Technically OK, though would've been an error originally
|
= help: Replace with `f"Finally, {a} World"`
ℹ Suggested fix
3 3 |
4 4 | a = "Hello"
5 5 | ok1 = " ".join([a, " World"]) # OK
6 |-ok2 = "".join(["Finally, ", a, " World"]) # OK
6 |+ok2 = f"Finally, {a} World" # OK
7 7 | ok3 = "x".join(("1", "2", "3")) # 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)
FLY002.py:7:7: FLY002 [*] Consider `f"1x2x3"` instead of string join
|
7 | ok1 = " ".join([a, " World"]) # OK
8 | ok2 = "".join(["Finally, ", a, " World"]) # OK
9 | ok3 = "x".join(("1", "2", "3")) # OK
| ^^^^^^^^^^^^^^^^^^^^^^^^^ FLY002
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"`
ℹ 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
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)
FLY002.py:8:7: FLY002 [*] Consider `f"{1}y{2}y{3}"` instead of string join
|
8 | ok2 = "".join(["Finally, ", a, " World"]) # OK
9 | ok3 = "x".join(("1", "2", "3")) # OK
10 | ok4 = "y".join([1, 2, 3]) # Technically OK, though would've been an error originally
| ^^^^^^^^^^^^^^^^^^^ FLY002
11 | ok5 = "a".join([random(), random()]) # OK (simple calls)
12 | ok6 = "a".join([secrets.token_urlsafe(), secrets.token_hex()]) # OK (attr calls)
|
= help: Replace with `f"{1}y{2}y{3}"`
ℹ Suggested fix
5 5 | ok1 = " ".join([a, " World"]) # OK
6 6 | ok2 = "".join(["Finally, ", a, " World"]) # OK
7 7 | ok3 = "x".join(("1", "2", "3")) # OK
8 |-ok4 = "y".join([1, 2, 3]) # Technically OK, though would've been an error originally
8 |+ok4 = f"{1}y{2}y{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)
11 11 |
FLY002.py:9:7: FLY002 [*] Consider `f"{random()}a{random()}"` instead of string join
|
9 | ok3 = "x".join(("1", "2", "3")) # OK
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)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FLY002
12 | ok6 = "a".join([secrets.token_urlsafe(), secrets.token_hex()]) # OK (attr calls)
|
= help: Replace with `f"{random()}a{random()}"`
ℹ Suggested fix
6 6 | ok2 = "".join(["Finally, ", a, " World"]) # OK
7 7 | ok3 = "x".join(("1", "2", "3")) # OK
8 8 | ok4 = "y".join([1, 2, 3]) # Technically OK, though would've been an error originally
9 |-ok5 = "a".join([random(), random()]) # OK (simple calls)
9 |+ok5 = f"{random()}a{random()}" # OK (simple calls)
10 10 | ok6 = "a".join([secrets.token_urlsafe(), secrets.token_hex()]) # OK (attr calls)
11 11 |
12 12 | nok1 = "x".join({"4", "5", "yee"}) # Not OK (set)
FLY002.py:10:7: FLY002 [*] Consider `f"{secrets.token_urlsafe()}a{secrets.token_hex()}"` 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)
12 | ok6 = "a".join([secrets.token_urlsafe(), secrets.token_hex()]) # OK (attr calls)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FLY002
13 |
14 | nok1 = "x".join({"4", "5", "yee"}) # Not OK (set)
|
= help: Replace with `f"{secrets.token_urlsafe()}a{secrets.token_hex()}"`
ℹ Suggested fix
7 7 | ok3 = "x".join(("1", "2", "3")) # 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 |-ok6 = "a".join([secrets.token_urlsafe(), secrets.token_hex()]) # OK (attr calls)
10 |+ok6 = f"{secrets.token_urlsafe()}a{secrets.token_hex()}" # OK (attr calls)
11 11 |
12 12 | nok1 = "x".join({"4", "5", "yee"}) # Not OK (set)
13 13 | nok2 = a.join(["1", "2", "3"]) # Not OK (not a static joiner)

1
crates/ruff/src/rules/mod.rs
View File

@ -32,6 +32,7 @@ pub mod flake8_tidy_imports;
pub mod flake8_type_checking;
pub mod flake8_unused_arguments;
pub mod flake8_use_pathlib;
pub mod flynt;
pub mod isort;
pub mod mccabe;
pub mod numpy;

4
ruff.schema.json generated
View File

@ -1829,6 +1829,10 @@
"FBT001",
"FBT002",
"FBT003",
"FLY",
"FLY0",
"FLY00",
"FLY002",
"G",
"G0",
"G00",