Skip to content

Commit

Permalink
Initial implementation for RUF017
Browse files Browse the repository at this point in the history
  • Loading branch information
evanrittenhouse committed Aug 11, 2023
1 parent 9171e97 commit a12a71a
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 0 deletions.
12 changes: 12 additions & 0 deletions crates/ruff/resources/test/fixtures/ruff/RUF017.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
x = [1, 2, 3]
y = [4, 5, 6]

# RUF017
sum([x, y], start=[])
sum([x, y], [])
sum([[1, 2, 3], [4, 5, 6]], start=[])
sum([[1, 2, 3], [4, 5, 6]], [])

# OK
sum([x, y])
sum([[1, 2, 3], [4, 5, 6]])
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::UnsupportedMethodCallOnAll) {
flake8_pyi::rules::unsupported_method_call_on_all(checker, func);
}
if checker.enabled(Rule::QuadraticListSummation) {
ruff::rules::quadratic_list_summation(checker, call);
}
}
Expr::Dict(ast::ExprDict {
keys,
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 @@ -811,6 +811,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "014") => (RuleGroup::Nursery, rules::ruff::rules::UnreachableCode),
(Ruff, "015") => (RuleGroup::Unspecified, rules::ruff::rules::UnnecessaryIterableAllocationForFirstElement),
(Ruff, "016") => (RuleGroup::Unspecified, rules::ruff::rules::InvalidIndexType),
(Ruff, "017") => (RuleGroup::Unspecified, rules::ruff::rules::QuadraticListSummation),
(Ruff, "100") => (RuleGroup::Unspecified, rules::ruff::rules::UnusedNOQA),
(Ruff, "200") => (RuleGroup::Unspecified, rules::ruff::rules::InvalidPyprojectToml),

Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ mod tests {
feature = "unreachable-code",
test_case(Rule::UnreachableCode, Path::new("RUF014.py"))
)]
#[test_case(Rule::QuadraticListSummation, Path::new("RUF017.py"))]
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
3 changes: 3 additions & 0 deletions crates/ruff/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,6 @@ pub(crate) enum Context {
Docstring,
Comment,
}
pub(crate) use quadratic_list_summation::*;

mod quadratic_list_summation;
114 changes: 114 additions & 0 deletions crates/ruff/src/rules/ruff/rules/quadratic_list_summation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Arguments, Expr};
use ruff_text_size::TextRange;

use crate::{checkers::ast::Checker, registry::Rule};

/// ## What it does
/// Avoid quadratic
///
/// ## 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.
///
/// ## Example
/// ```python
/// lists = [[1, 2, 3], [4, 5, 6], [7, 8, 9]]
/// joined = sum(lists, [])
/// ```
///
/// Use instead:
/// ```python
/// lists = [[1, 2, 3], [4, 5, 6], [7, 8, 9]]
/// joined = [el for list in lists for el in list]
/// ```
#[violation]
pub struct QuadraticListSummation;

impl AlwaysAutofixableViolation for QuadraticListSummation {
#[derive_message_formats]
fn message(&self) -> String {
format!("Replace quadratic list summation with list comprehension")
}

fn autofix_title(&self) -> String {
format!("Convert to list comprehension")
}
}

/// RUF017
pub(crate) fn quadratic_list_summation(checker: &mut Checker, call: &ast::ExprCall) {
let ast::ExprCall {
func,
arguments,
range,
} = call;

if !func_is_builtin(func, "sum", checker) {
return;
}

if verify_start_arg(arguments, checker).is_none() {
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 mut diagnostic = Diagnostic::new(QuadraticListSummation, *range);
if checker.patch(Rule::QuadraticListSummation) {
diagnostic.set_fix(convert_to_comprehension(list_string_repr, *range));
}

checker.diagnostics.push(diagnostic);
}

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]"),
range,
))
}

/// 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> {
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)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
---
source: crates/ruff/src/rules/ruff/mod.rs
---
RUF017.py:5:1: RUF017 [*] Replace quadratic list summation with list comprehension
|
4 | # RUF017
5 | sum([x, y], start=[])
| ^^^^^^^^^^^^^^^^^^^^^ RUF017
6 | sum([x, y], [])
7 | sum([[1, 2, 3], [4, 5, 6]], start=[])
|
= help: Convert to list comprehension

Suggested fix
2 2 | y = [4, 5, 6]
3 3 |
4 4 | # RUF017
5 |-sum([x, y], start=[])
5 |+[el for lst in [x, y] for el in lst]
6 6 | sum([x, y], [])
7 7 | sum([[1, 2, 3], [4, 5, 6]], start=[])
8 8 | sum([[1, 2, 3], [4, 5, 6]], [])

RUF017.py:6:1: RUF017 [*] Replace quadratic list summation with list comprehension
|
4 | # RUF017
5 | sum([x, y], start=[])
6 | sum([x, y], [])
| ^^^^^^^^^^^^^^^ RUF017
7 | sum([[1, 2, 3], [4, 5, 6]], start=[])
8 | sum([[1, 2, 3], [4, 5, 6]], [])
|
= help: Convert to list comprehension

Suggested fix
3 3 |
4 4 | # RUF017
5 5 | sum([x, y], start=[])
6 |-sum([x, y], [])
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 |

RUF017.py:7:1: RUF017 [*] Replace quadratic list summation with list comprehension
|
5 | sum([x, y], start=[])
6 | sum([x, y], [])
7 | sum([[1, 2, 3], [4, 5, 6]], start=[])
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF017
8 | sum([[1, 2, 3], [4, 5, 6]], [])
|
= help: Convert to list comprehension

Suggested fix
4 4 | # RUF017
5 5 | sum([x, y], start=[])
6 6 | sum([x, y], [])
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

RUF017.py:8:1: RUF017 [*] Replace quadratic list summation with list comprehension
|
6 | sum([x, y], [])
7 | sum([[1, 2, 3], [4, 5, 6]], start=[])
8 | sum([[1, 2, 3], [4, 5, 6]], [])
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF017
9 |
10 | # OK
|
= help: Convert to list comprehension

Suggested fix
5 5 | sum([x, y], start=[])
6 6 | sum([x, y], [])
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])


0 comments on commit a12a71a

Please sign in to comment.