diff --git a/crates/ruff/resources/test/fixtures/ruff/RUF017.py b/crates/ruff/resources/test/fixtures/ruff/RUF017.py index cc7929e30d..f7c062d894 100644 --- a/crates/ruff/resources/test/fixtures/ruff/RUF017.py +++ b/crates/ruff/resources/test/fixtures/ruff/RUF017.py @@ -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]) diff --git a/crates/ruff/src/rules/ruff/rules/quadratic_list_summation.rs b/crates/ruff/src/rules/ruff/rules/quadratic_list_summation.rs index b8cd326521..531644a02a 100644 --- a/crates/ruff/src/rules/ruff/rules/quadratic_list_summation.rs +++ b/crates/ruff/src/rules/ruff/rules/quadratic_list_summation.rs @@ -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, + } } diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF017_RUF017.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF017_RUF017.py.snap index 62d8dac10a..3eca7a648e 100644 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF017_RUF017.py.snap +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF017_RUF017.py.snap @@ -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]) diff --git a/ruff.schema.json b/ruff.schema.json index 2c5bfc3090..770a976bcf 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2456,6 +2456,7 @@ "RUF013", "RUF015", "RUF016", + "RUF017", "RUF1", "RUF10", "RUF100",