Cyclic0007/ruff
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Implement reviewer comments

Browse Source
This commit is contained in:
Evan Rittenhouse
2023-08-11 09:52:28 -05:00
parent a12a71a845
commit b6d786fb10
4 changed files with 75 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
crates/ruff/resources/test/fixtures/ruff/RUF017.py vendored
View File

@@ -6,6 +6,8 @@ sum([x, y], start=[])
sum([x, y], [])
sum([[1, 2, 3], [4, 5, 6]], start=[])
sum([[1, 2, 3], [4, 5, 6]], [])
sum([[1, 2, 3], [4, 5, 6]],
[])
# OK
sum([x, y])

83
crates/ruff/src/rules/ruff/rules/quadratic_list_summation.rs
View File

@@ -6,11 +6,12 @@ use ruff_text_size::TextRange;
use crate::{checkers::ast::Checker, registry::Rule};
/// ## What it does
/// Avoid quadratic
/// Avoid quadratic list summation, or the flattening of multiple lists into one via the use of the
/// `sum()` built-in function.
///
/// ## Why is this bad?
/// Quadratic list summation is slower than other methods of list concatenation.
/// A list comprehension can perform the same operation but in a much shorter time period.
/// Quadratic list summation is slower than other methods of list flattening. See the link in
/// [`References`](#references) for `timeit` results on other ways of flattening lists.
///
/// ## Example
/// ```python
@@ -23,6 +24,9 @@ use crate::{checkers::ast::Checker, registry::Rule};
/// lists = [[1, 2, 3], [4, 5, 6], [7, 8, 9]]
/// joined = [el for list in lists for el in list]
/// ```
///
/// ## References
/// [How do I make a flat list out of a list of lists?](https://stackoverflow.com/questions/952914/how-do-i-make-a-flat-list-out-of-a-list-of-lists/953097#953097)
#[violation]
pub struct QuadraticListSummation;
@@ -49,29 +53,42 @@ pub(crate) fn quadratic_list_summation(checker: &mut Checker, call: &ast::ExprCa
return;
}
if verify_start_arg(arguments, checker).is_none() {
if !start_is_list(arguments, checker) {
return;
}
};
let Arguments { args, .. } = arguments;
let Some(list_string_repr) = args
.first()
.and_then(|arg| match arg {
Expr::Name(ast::ExprName { id, .. }) => Some(id.as_str()),
Expr::List(ast::ExprList { range, .. }) => Some(checker.locator().slice(*range)),
_ => None,
}) else {
return;
};
let Some(first) = args.first() else {
return;
};
let list_as_str = match first {
Expr::Name(ast::ExprName { id, .. }) => Some(id.as_str()),
Expr::List(ast::ExprList { range, .. }) => Some(checker.locator().slice(*range)),
_ => return,
};
let mut diagnostic = Diagnostic::new(QuadraticListSummation, *range);
if checker.patch(Rule::QuadraticListSummation) {
diagnostic.set_fix(convert_to_comprehension(list_string_repr, *range));
let Some(str_repr) = list_as_str else {
return;
};
diagnostic.set_fix(convert_to_comprehension(str_repr, *range));
}
checker.diagnostics.push(diagnostic);
}
/// Check if a function is a builtin with a given name.
fn func_is_builtin(func: &Expr, name: &str, checker: &mut Checker) -> bool {
let Expr::Name(ast::ExprName { id, .. }) = func else {
return false;
};
id == name && checker.semantic().is_builtin(id)
}
fn convert_to_comprehension(container_list_name: &str, range: TextRange) -> Fix {
Fix::suggested(Edit::range_replacement(
format!("[el for lst in {container_list_name} for el in lst]"),
@@ -79,36 +96,16 @@ fn convert_to_comprehension(container_list_name: &str, range: TextRange) -> Fix
))
}
/// Check that the `start` arg/kwarg is actually a list/list-equivalent.
fn verify_start_arg<'a>(arguments: &'a Arguments, checker: &mut Checker) -> Option<&'a Expr> {
fn start_is_list(arguments: &Arguments, checker: &mut Checker) -> bool {
let Some(start_arg) = arguments.find_argument("start", 1) else {
return None;
};
match start_arg {
Expr::Call(ast::ExprCall { func, .. }) => {
if func_is_builtin(func, "list", checker) {
Some(start_arg)
} else {
None
}
}
Expr::List(ast::ExprList { elts, ctx, .. }) => {
if elts.is_empty() && ctx == &ast::ExprContext::Load {
Some(start_arg)
} else {
None
}
}
_ => None,
}
}
/// Check if a function is builtin with a given name.
fn func_is_builtin(func: &Expr, name: &str, checker: &mut Checker) -> bool {
let Expr::Name(ast::ExprName { id, .. }) = func else {
return false;
};
id == name && checker.semantic().is_builtin(id)
match start_arg {
Expr::Call(ast::ExprCall { func, .. }) => func_is_builtin(func, "list", checker),
Expr::List(ast::ExprList { elts, ctx, .. }) => {
elts.is_empty() && ctx == &ast::ExprContext::Load
}
_ => false,
}
}

40
crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF017_RUF017.py.snap
View File

@@ -40,7 +40,7 @@ RUF017.py:6:1: RUF017 [*] Replace quadratic list summation with list comprehensi
6 |+[el for lst in [x, y] for el in lst]
7 7 | sum([[1, 2, 3], [4, 5, 6]], start=[])
8 8 | sum([[1, 2, 3], [4, 5, 6]], [])
9 9 |
9 9 | sum([[1, 2, 3], [4, 5, 6]],
RUF017.py:7:1: RUF017 [*] Replace quadratic list summation with list comprehension
|
@@ -49,6 +49,7 @@ RUF017.py:7:1: RUF017 [*] Replace quadratic list summation with list comprehensi
7 | sum([[1, 2, 3], [4, 5, 6]], start=[])
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF017
8 | sum([[1, 2, 3], [4, 5, 6]], [])
9 | sum([[1, 2, 3], [4, 5, 6]],
|
= help: Convert to list comprehension
@@ -59,8 +60,8 @@ RUF017.py:7:1: RUF017 [*] Replace quadratic list summation with list comprehensi
7 |-sum([[1, 2, 3], [4, 5, 6]], start=[])
7 |+[el for lst in [[1, 2, 3], [4, 5, 6]] for el in lst]
8 8 | sum([[1, 2, 3], [4, 5, 6]], [])
9 9 |
10 10 | # OK
9 9 | sum([[1, 2, 3], [4, 5, 6]],
10 10 | [])
RUF017.py:8:1: RUF017 [*] Replace quadratic list summation with list comprehension
|
@@ -68,8 +69,8 @@ RUF017.py:8:1: RUF017 [*] Replace quadratic list summation with list comprehensi
7 | sum([[1, 2, 3], [4, 5, 6]], start=[])
8 | sum([[1, 2, 3], [4, 5, 6]], [])
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF017
9 |
10 | # OK
9 | sum([[1, 2, 3], [4, 5, 6]],
10 | [])
|
= help: Convert to list comprehension
@@ -79,8 +80,31 @@ RUF017.py:8:1: RUF017 [*] Replace quadratic list summation with list comprehensi
7 7 | sum([[1, 2, 3], [4, 5, 6]], start=[])
8 |-sum([[1, 2, 3], [4, 5, 6]], [])
8 |+[el for lst in [[1, 2, 3], [4, 5, 6]] for el in lst]
9 9 |
10 10 | # OK
11 11 | sum([x, y])
9 9 | sum([[1, 2, 3], [4, 5, 6]],
10 10 | [])
11 11 |
RUF017.py:9:1: RUF017 [*] Replace quadratic list summation with list comprehension
|
7 | sum([[1, 2, 3], [4, 5, 6]], start=[])
8 | sum([[1, 2, 3], [4, 5, 6]], [])
9 | / sum([[1, 2, 3], [4, 5, 6]],
10 | | [])
| |_______^ RUF017
11 |
12 | # OK
|
= help: Convert to list comprehension
ℹ Suggested fix
6 6 | sum([x, y], [])
7 7 | sum([[1, 2, 3], [4, 5, 6]], start=[])
8 8 | sum([[1, 2, 3], [4, 5, 6]], [])
9 |-sum([[1, 2, 3], [4, 5, 6]],
10 |- [])
9 |+[el for lst in [[1, 2, 3], [4, 5, 6]] for el in lst]
11 10 |
12 11 | # OK
13 12 | sum([x, y])

1
ruff.schema.json generated
View File

@@ -2456,6 +2456,7 @@
"RUF013",
"RUF015",
"RUF016",
"RUF017",
"RUF1",
"RUF10",
"RUF100",