mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-03 02:13:08 +00:00
[perflint] Add PERF101 with autofix (#5121)
## Summary
Adds PERF101 which checks for unnecessary casts to `list` in for loops.
NOTE: Is not fully equal to its upstream implementation as this
implementation does not flag based on type annotations
(i.e.):
```python
def foo(x: List[str]):
for y in list(x):
...
```
With the current set-up it's quite hard to get the annotation from a
function arg from its binding. Problem is best considered broader than
this implementation.
## Test Plan
Added fixture.
## Issue links
Refers: https://github.com/astral-sh/ruff/issues/4789
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
qdegraaf
GitHub
parent
50f0edd2cb
commit
38e618cd18
8 changed files with 373 additions and 1 deletions
52
crates/ruff/resources/test/fixtures/perflint/PERF101.py
vendored
Normal file
52
crates/ruff/resources/test/fixtures/perflint/PERF101.py
vendored
Normal file
|
|
@ -0,0 +1,52 @@
|
|||
foo_tuple = (1, 2, 3)
|
||||
foo_list = [1, 2, 3]
|
||||
foo_set = {1, 2, 3}
|
||||
foo_dict = {1: 2, 3: 4}
|
||||
foo_int = 123
|
||||
|
||||
for i in list(foo_tuple): # PERF101
|
||||
pass
|
||||
|
||||
for i in list(foo_list): # PERF101
|
||||
pass
|
||||
|
||||
for i in list(foo_set): # PERF101
|
||||
pass
|
||||
|
||||
for i in list((1, 2, 3)): # PERF101
|
||||
pass
|
||||
|
||||
for i in list([1, 2, 3]): # PERF101
|
||||
pass
|
||||
|
||||
for i in list({1, 2, 3}): # PERF101
|
||||
pass
|
||||
|
||||
for i in list(
|
||||
{
|
||||
1,
|
||||
2,
|
||||
3,
|
||||
}
|
||||
):
|
||||
pass
|
||||
|
||||
for i in list( # Comment
|
||||
{1, 2, 3}
|
||||
): # PERF101
|
||||
pass
|
||||
|
||||
for i in list(foo_dict): # Ok
|
||||
pass
|
||||
|
||||
for i in list(1): # Ok
|
||||
pass
|
||||
|
||||
for i in list(foo_int): # Ok
|
||||
pass
|
||||
|
||||
|
||||
import itertools
|
||||
|
||||
for i in itertools.product(foo_int): # Ok
|
||||
pass
|
||||
|
|
@ -1481,6 +1481,9 @@ where
|
|||
if self.enabled(Rule::IncorrectDictIterator) {
|
||||
perflint::rules::incorrect_dict_iterator(self, target, iter);
|
||||
}
|
||||
if self.enabled(Rule::UnnecessaryListCast) {
|
||||
perflint::rules::unnecessary_list_cast(self, iter);
|
||||
}
|
||||
}
|
||||
Stmt::Try(ast::StmtTry {
|
||||
body,
|
||||
|
|
|
|||
|
|
@ -784,6 +784,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
|||
(Airflow, "001") => (RuleGroup::Unspecified, rules::airflow::rules::AirflowVariableNameTaskIdMismatch),
|
||||
|
||||
// perflint
|
||||
(Perflint, "101") => (RuleGroup::Unspecified, rules::perflint::rules::UnnecessaryListCast),
|
||||
(Perflint, "102") => (RuleGroup::Unspecified, rules::perflint::rules::IncorrectDictIterator),
|
||||
|
||||
// flake8-fixme
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ mod tests {
|
|||
use crate::settings::Settings;
|
||||
use crate::test::test_path;
|
||||
|
||||
#[test_case(Rule::UnnecessaryListCast, Path::new("PERF101.py"))]
|
||||
#[test_case(Rule::IncorrectDictIterator, Path::new("PERF102.py"))]
|
||||
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
pub(crate) use incorrect_dict_iterator::{incorrect_dict_iterator, IncorrectDictIterator};
|
||||
pub(crate) use incorrect_dict_iterator::*;
|
||||
pub(crate) use unnecessary_list_cast::*;
|
||||
|
||||
mod incorrect_dict_iterator;
|
||||
mod unnecessary_list_cast;
|
||||
|
|
|
|||
129
crates/ruff/src/rules/perflint/rules/unnecessary_list_cast.rs
Normal file
129
crates/ruff/src/rules/perflint/rules/unnecessary_list_cast.rs
Normal file
|
|
@ -0,0 +1,129 @@
|
|||
use ruff_text_size::TextRange;
|
||||
use rustpython_parser::ast::{self, Expr};
|
||||
|
||||
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::prelude::Stmt;
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::registry::AsRule;
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for explicit casts to `list` on for-loop iterables.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// Using a `list()` call to eagerly iterate over an already-iterable type
|
||||
/// (like a tuple, list, or set) is inefficient, as it forces Python to create
|
||||
/// a new list unnecessarily.
|
||||
///
|
||||
/// Removing the `list()` call will not change the behavior of the code, but
|
||||
/// may improve performance.
|
||||
///
|
||||
/// ## Example
|
||||
/// ```python
|
||||
/// items = (1, 2, 3)
|
||||
/// for i in list(items):
|
||||
/// print(i)
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```python
|
||||
/// items = (1, 2, 3)
|
||||
/// for i in items:
|
||||
/// print(i)
|
||||
/// ```
|
||||
#[violation]
|
||||
pub struct UnnecessaryListCast;
|
||||
|
||||
impl AlwaysAutofixableViolation for UnnecessaryListCast {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("Do not cast an iterable to `list` before iterating over it")
|
||||
}
|
||||
|
||||
fn autofix_title(&self) -> String {
|
||||
format!("Remove `list()` cast")
|
||||
}
|
||||
}
|
||||
|
||||
/// PERF101
|
||||
pub(crate) fn unnecessary_list_cast(checker: &mut Checker, iter: &Expr) {
|
||||
let Expr::Call(ast::ExprCall{ func, args, range: list_range, ..}) = iter else {
|
||||
return;
|
||||
};
|
||||
|
||||
if args.len() != 1 {
|
||||
return;
|
||||
}
|
||||
|
||||
let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else{
|
||||
return;
|
||||
};
|
||||
|
||||
if !(id == "list" && checker.semantic().is_builtin("list")) {
|
||||
return;
|
||||
}
|
||||
|
||||
match &args[0] {
|
||||
Expr::Tuple(ast::ExprTuple {
|
||||
range: iterable_range,
|
||||
..
|
||||
})
|
||||
| Expr::List(ast::ExprList {
|
||||
range: iterable_range,
|
||||
..
|
||||
})
|
||||
| Expr::Set(ast::ExprSet {
|
||||
range: iterable_range,
|
||||
..
|
||||
}) => {
|
||||
let mut diagnostic = Diagnostic::new(UnnecessaryListCast, *list_range);
|
||||
if checker.patch(diagnostic.kind.rule()) {
|
||||
diagnostic.set_fix(remove_cast(*list_range, *iterable_range));
|
||||
}
|
||||
checker.diagnostics.push(diagnostic);
|
||||
}
|
||||
Expr::Name(ast::ExprName {
|
||||
id,
|
||||
range: iterable_range,
|
||||
..
|
||||
}) => {
|
||||
let scope = checker.semantic().scope();
|
||||
if let Some(binding_id) = scope.get(id) {
|
||||
let binding = checker.semantic().binding(binding_id);
|
||||
if binding.kind.is_assignment() || binding.kind.is_named_expr_assignment() {
|
||||
if let Some(parent_id) = binding.source {
|
||||
let parent = checker.semantic().stmts[parent_id];
|
||||
if let Stmt::Assign(ast::StmtAssign { value, .. })
|
||||
| Stmt::AnnAssign(ast::StmtAnnAssign {
|
||||
value: Some(value), ..
|
||||
})
|
||||
| Stmt::AugAssign(ast::StmtAugAssign { value, .. }) = parent
|
||||
{
|
||||
if matches!(
|
||||
value.as_ref(),
|
||||
Expr::Tuple(_) | Expr::List(_) | Expr::Set(_)
|
||||
) {
|
||||
let mut diagnostic =
|
||||
Diagnostic::new(UnnecessaryListCast, *list_range);
|
||||
if checker.patch(diagnostic.kind.rule()) {
|
||||
diagnostic.set_fix(remove_cast(*list_range, *iterable_range));
|
||||
}
|
||||
checker.diagnostics.push(diagnostic);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
/// Generate a [`Fix`] to remove a `list` cast from an expression.
|
||||
fn remove_cast(list_range: TextRange, iterable_range: TextRange) -> Fix {
|
||||
Fix::automatic_edits(
|
||||
Edit::deletion(list_range.start(), iterable_range.start()),
|
||||
[Edit::deletion(iterable_range.end(), list_range.end())],
|
||||
)
|
||||
}
|
||||
|
|
@ -0,0 +1,183 @@
|
|||
---
|
||||
source: crates/ruff/src/rules/perflint/mod.rs
|
||||
---
|
||||
PERF101.py:7:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it
|
||||
|
|
||||
5 | foo_int = 123
|
||||
6 |
|
||||
7 | for i in list(foo_tuple): # PERF101
|
||||
| ^^^^^^^^^^^^^^^ PERF101
|
||||
8 | pass
|
||||
|
|
||||
= help: Remove `list()` cast
|
||||
|
||||
ℹ Fix
|
||||
4 4 | foo_dict = {1: 2, 3: 4}
|
||||
5 5 | foo_int = 123
|
||||
6 6 |
|
||||
7 |-for i in list(foo_tuple): # PERF101
|
||||
7 |+for i in foo_tuple: # PERF101
|
||||
8 8 | pass
|
||||
9 9 |
|
||||
10 10 | for i in list(foo_list): # PERF101
|
||||
|
||||
PERF101.py:10:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it
|
||||
|
|
||||
8 | pass
|
||||
9 |
|
||||
10 | for i in list(foo_list): # PERF101
|
||||
| ^^^^^^^^^^^^^^ PERF101
|
||||
11 | pass
|
||||
|
|
||||
= help: Remove `list()` cast
|
||||
|
||||
ℹ Fix
|
||||
7 7 | for i in list(foo_tuple): # PERF101
|
||||
8 8 | pass
|
||||
9 9 |
|
||||
10 |-for i in list(foo_list): # PERF101
|
||||
10 |+for i in foo_list: # PERF101
|
||||
11 11 | pass
|
||||
12 12 |
|
||||
13 13 | for i in list(foo_set): # PERF101
|
||||
|
||||
PERF101.py:13:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it
|
||||
|
|
||||
11 | pass
|
||||
12 |
|
||||
13 | for i in list(foo_set): # PERF101
|
||||
| ^^^^^^^^^^^^^ PERF101
|
||||
14 | pass
|
||||
|
|
||||
= help: Remove `list()` cast
|
||||
|
||||
ℹ Fix
|
||||
10 10 | for i in list(foo_list): # PERF101
|
||||
11 11 | pass
|
||||
12 12 |
|
||||
13 |-for i in list(foo_set): # PERF101
|
||||
13 |+for i in foo_set: # PERF101
|
||||
14 14 | pass
|
||||
15 15 |
|
||||
16 16 | for i in list((1, 2, 3)): # PERF101
|
||||
|
||||
PERF101.py:16:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it
|
||||
|
|
||||
14 | pass
|
||||
15 |
|
||||
16 | for i in list((1, 2, 3)): # PERF101
|
||||
| ^^^^^^^^^^^^^^^ PERF101
|
||||
17 | pass
|
||||
|
|
||||
= help: Remove `list()` cast
|
||||
|
||||
ℹ Fix
|
||||
13 13 | for i in list(foo_set): # PERF101
|
||||
14 14 | pass
|
||||
15 15 |
|
||||
16 |-for i in list((1, 2, 3)): # PERF101
|
||||
16 |+for i in (1, 2, 3): # PERF101
|
||||
17 17 | pass
|
||||
18 18 |
|
||||
19 19 | for i in list([1, 2, 3]): # PERF101
|
||||
|
||||
PERF101.py:19:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it
|
||||
|
|
||||
17 | pass
|
||||
18 |
|
||||
19 | for i in list([1, 2, 3]): # PERF101
|
||||
| ^^^^^^^^^^^^^^^ PERF101
|
||||
20 | pass
|
||||
|
|
||||
= help: Remove `list()` cast
|
||||
|
||||
ℹ Fix
|
||||
16 16 | for i in list((1, 2, 3)): # PERF101
|
||||
17 17 | pass
|
||||
18 18 |
|
||||
19 |-for i in list([1, 2, 3]): # PERF101
|
||||
19 |+for i in [1, 2, 3]: # PERF101
|
||||
20 20 | pass
|
||||
21 21 |
|
||||
22 22 | for i in list({1, 2, 3}): # PERF101
|
||||
|
||||
PERF101.py:22:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it
|
||||
|
|
||||
20 | pass
|
||||
21 |
|
||||
22 | for i in list({1, 2, 3}): # PERF101
|
||||
| ^^^^^^^^^^^^^^^ PERF101
|
||||
23 | pass
|
||||
|
|
||||
= help: Remove `list()` cast
|
||||
|
||||
ℹ Fix
|
||||
19 19 | for i in list([1, 2, 3]): # PERF101
|
||||
20 20 | pass
|
||||
21 21 |
|
||||
22 |-for i in list({1, 2, 3}): # PERF101
|
||||
22 |+for i in {1, 2, 3}: # PERF101
|
||||
23 23 | pass
|
||||
24 24 |
|
||||
25 25 | for i in list(
|
||||
|
||||
PERF101.py:25:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it
|
||||
|
|
||||
23 | pass
|
||||
24 |
|
||||
25 | for i in list(
|
||||
| __________^
|
||||
26 | | {
|
||||
27 | | 1,
|
||||
28 | | 2,
|
||||
29 | | 3,
|
||||
30 | | }
|
||||
31 | | ):
|
||||
| |_^ PERF101
|
||||
32 | pass
|
||||
|
|
||||
= help: Remove `list()` cast
|
||||
|
||||
ℹ Fix
|
||||
22 22 | for i in list({1, 2, 3}): # PERF101
|
||||
23 23 | pass
|
||||
24 24 |
|
||||
25 |-for i in list(
|
||||
26 |- {
|
||||
25 |+for i in {
|
||||
27 26 | 1,
|
||||
28 27 | 2,
|
||||
29 28 | 3,
|
||||
30 |- }
|
||||
31 |-):
|
||||
29 |+ }:
|
||||
32 30 | pass
|
||||
33 31 |
|
||||
34 32 | for i in list( # Comment
|
||||
|
||||
PERF101.py:34:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it
|
||||
|
|
||||
32 | pass
|
||||
33 |
|
||||
34 | for i in list( # Comment
|
||||
| __________^
|
||||
35 | | {1, 2, 3}
|
||||
36 | | ): # PERF101
|
||||
| |_^ PERF101
|
||||
37 | pass
|
||||
|
|
||||
= help: Remove `list()` cast
|
||||
|
||||
ℹ Fix
|
||||
31 31 | ):
|
||||
32 32 | pass
|
||||
33 33 |
|
||||
34 |-for i in list( # Comment
|
||||
35 |- {1, 2, 3}
|
||||
36 |-): # PERF101
|
||||
34 |+for i in {1, 2, 3}: # PERF101
|
||||
37 35 | pass
|
||||
38 36 |
|
||||
39 37 | for i in list(foo_dict): # Ok
|
||||
|
||||
|
||||
1
ruff.schema.json
generated
1
ruff.schema.json
generated
|
|
@ -2063,6 +2063,7 @@
|
|||
"PERF",
|
||||
"PERF1",
|
||||
"PERF10",
|
||||
"PERF101",
|
||||
"PERF102",
|
||||
"PGH",
|
||||
"PGH0",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue