Skip to content

Commit

Permalink
Improvements to RUF015 (#7848)
Browse files Browse the repository at this point in the history
## Summary

Resolves #7618. 

The list of builtin iterator is not exhaustive.

## Test Plan

`cargo test`

``` python
a = [1, 2]

examples = [
    enumerate(a),
    filter(lambda x: x, a),
    map(int, a),
    reversed(a),
    zip(a),
    iter(a),
]

for example in examples:
    print(next(example))
```
  • Loading branch information
hoxbro authored and konstin committed Oct 11, 2023
1 parent 9e54b38 commit f64e10e
Show file tree
Hide file tree
Showing 4 changed files with 247 additions and 25 deletions.
16 changes: 16 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF015.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,26 @@
list(range(10))[0]
list(x.y)[0]
list(x["y"])[0]
[*range(10)][0]
[*x["y"]][0]
[*x.y][0]
[* x.y][0]
[
*x.y
][0]

# RUF015 (multi-line)
revision_heads_map_ast = [
a
for a in revision_heads_map_ast_obj.body
if isinstance(a, ast.Assign) and a.targets[0].id == "REVISION_HEADS_MAP"
][0]

# RUF015 (zip)
list(zip(x, y))[0]
[*zip(x, y)][0]


def test():
zip = list # Overwrite the builtin zip
list(zip(x, y))[0]
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Arguments, Comprehension, Constant, Expr, Int};
use ruff_python_semantic::SemanticModel;
use ruff_python_stdlib::builtins::is_iterator;
use ruff_text_size::{Ranged, TextRange, TextSize};

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -185,6 +186,10 @@ fn match_iteration_target(expr: &Expr, semantic: &SemanticModel) -> Option<Itera
iterable: true,
},
},
Expr::Call(ast::ExprCall { func, .. }) => IterationTarget {
range: arg.range(),
iterable: is_func_builtin_iterator(func, semantic),
},
_ => IterationTarget {
range: arg.range(),
iterable: false,
Expand All @@ -209,7 +214,24 @@ fn match_iteration_target(expr: &Expr, semantic: &SemanticModel) -> Option<Itera
iterable: true,
},
},
Expr::List(ast::ExprList { elts, .. }) => {
let [elt] = elts.as_slice() else {
return None;
};
let Expr::Starred(ast::ExprStarred { value, .. }) = elt else {
return None;
};

let iterable = if value.is_call_expr() {
is_func_builtin_iterator(&value.as_call_expr()?.func, semantic)
} else {
false
};
IterationTarget {
range: value.range(),
iterable,
}
}
_ => return None,
};

Expand Down Expand Up @@ -240,3 +262,10 @@ fn match_simple_comprehension(elt: &Expr, generators: &[Comprehension]) -> Optio

Some(generator.iter.range())
}

/// Returns `true` if the function is a builtin iterator.
fn is_func_builtin_iterator(func: &Expr, semantic: &SemanticModel) -> bool {
func.as_name_expr().map_or(false, |func_name| {
is_iterator(func_name.id.as_str()) && semantic.is_builtin(func_name.id.as_str())
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ RUF015.py:38:1: RUF015 [*] Prefer `next(iter(range(10)))` over single element sl
38 |+next(iter(range(10)))
39 39 | list(x.y)[0]
40 40 | list(x["y"])[0]
41 41 |
41 41 | [*range(10)][0]

RUF015.py:39:1: RUF015 [*] Prefer `next(iter(x.y))` over single element slice
|
Expand All @@ -192,6 +192,7 @@ RUF015.py:39:1: RUF015 [*] Prefer `next(iter(x.y))` over single element slice
39 | list(x.y)[0]
| ^^^^^^^^^^^^ RUF015
40 | list(x["y"])[0]
41 | [*range(10)][0]
|
= help: Replace with `next(iter(x.y))`

Expand All @@ -202,17 +203,17 @@ RUF015.py:39:1: RUF015 [*] Prefer `next(iter(x.y))` over single element slice
39 |-list(x.y)[0]
39 |+next(iter(x.y))
40 40 | list(x["y"])[0]
41 41 |
42 42 | # RUF015 (multi-line)
41 41 | [*range(10)][0]
42 42 | [*x["y"]][0]

RUF015.py:40:1: RUF015 [*] Prefer `next(iter(x["y"]))` over single element slice
|
38 | list(range(10))[0]
39 | list(x.y)[0]
40 | list(x["y"])[0]
| ^^^^^^^^^^^^^^^ RUF015
41 |
42 | # RUF015 (multi-line)
41 | [*range(10)][0]
42 | [*x["y"]][0]
|
= help: Replace with `next(iter(x["y"]))`

Expand All @@ -222,33 +223,201 @@ RUF015.py:40:1: RUF015 [*] Prefer `next(iter(x["y"]))` over single element slice
39 39 | list(x.y)[0]
40 |-list(x["y"])[0]
40 |+next(iter(x["y"]))
41 41 |
42 42 | # RUF015 (multi-line)
43 43 | revision_heads_map_ast = [
41 41 | [*range(10)][0]
42 42 | [*x["y"]][0]
43 43 | [*x.y][0]

RUF015.py:43:26: RUF015 [*] Prefer `next(...)` over single element slice
RUF015.py:41:1: RUF015 [*] Prefer `next(iter(range(10)))` over single element slice
|
42 | # RUF015 (multi-line)
43 | revision_heads_map_ast = [
| __________________________^
44 | | a
45 | | for a in revision_heads_map_ast_obj.body
46 | | if isinstance(a, ast.Assign) and a.targets[0].id == "REVISION_HEADS_MAP"
39 | list(x.y)[0]
40 | list(x["y"])[0]
41 | [*range(10)][0]
| ^^^^^^^^^^^^^^^ RUF015
42 | [*x["y"]][0]
43 | [*x.y][0]
|
= help: Replace with `next(iter(range(10)))`

Suggested fix
38 38 | list(range(10))[0]
39 39 | list(x.y)[0]
40 40 | list(x["y"])[0]
41 |-[*range(10)][0]
41 |+next(iter(range(10)))
42 42 | [*x["y"]][0]
43 43 | [*x.y][0]
44 44 | [* x.y][0]

RUF015.py:42:1: RUF015 [*] Prefer `next(iter(x["y"]))` over single element slice
|
40 | list(x["y"])[0]
41 | [*range(10)][0]
42 | [*x["y"]][0]
| ^^^^^^^^^^^^ RUF015
43 | [*x.y][0]
44 | [* x.y][0]
|
= help: Replace with `next(iter(x["y"]))`

Suggested fix
39 39 | list(x.y)[0]
40 40 | list(x["y"])[0]
41 41 | [*range(10)][0]
42 |-[*x["y"]][0]
42 |+next(iter(x["y"]))
43 43 | [*x.y][0]
44 44 | [* x.y][0]
45 45 | [

RUF015.py:43:1: RUF015 [*] Prefer `next(iter(x.y))` over single element slice
|
41 | [*range(10)][0]
42 | [*x["y"]][0]
43 | [*x.y][0]
| ^^^^^^^^^ RUF015
44 | [* x.y][0]
45 | [
|
= help: Replace with `next(iter(x.y))`

Suggested fix
40 40 | list(x["y"])[0]
41 41 | [*range(10)][0]
42 42 | [*x["y"]][0]
43 |-[*x.y][0]
43 |+next(iter(x.y))
44 44 | [* x.y][0]
45 45 | [
46 46 | *x.y

RUF015.py:44:1: RUF015 [*] Prefer `next(iter(x.y))` over single element slice
|
42 | [*x["y"]][0]
43 | [*x.y][0]
44 | [* x.y][0]
| ^^^^^^^^^^ RUF015
45 | [
46 | *x.y
|
= help: Replace with `next(iter(x.y))`

Suggested fix
41 41 | [*range(10)][0]
42 42 | [*x["y"]][0]
43 43 | [*x.y][0]
44 |-[* x.y][0]
44 |+next(iter(x.y))
45 45 | [
46 46 | *x.y
47 47 | ][0]

RUF015.py:45:1: RUF015 [*] Prefer `next(iter(x.y))` over single element slice
|
43 | [*x.y][0]
44 | [* x.y][0]
45 | / [
46 | | *x.y
47 | | ][0]
| |____^ RUF015
48 |
49 | # RUF015 (multi-line)
|
= help: Replace with `next(...)`
= help: Replace with `next(iter(x.y))`

Suggested fix
40 40 | list(x["y"])[0]
41 41 |
42 42 | # RUF015 (multi-line)
43 |-revision_heads_map_ast = [
43 |+revision_heads_map_ast = next(
44 44 | a
45 45 | for a in revision_heads_map_ast_obj.body
46 46 | if isinstance(a, ast.Assign) and a.targets[0].id == "REVISION_HEADS_MAP"
42 42 | [*x["y"]][0]
43 43 | [*x.y][0]
44 44 | [* x.y][0]
45 |-[
46 |- *x.y
47 |-][0]
47 |+)
45 |+next(iter(x.y))
48 46 |
49 47 | # RUF015 (multi-line)
50 48 | revision_heads_map_ast = [

RUF015.py:50:26: RUF015 [*] Prefer `next(...)` over single element slice
|
49 | # RUF015 (multi-line)
50 | revision_heads_map_ast = [
| __________________________^
51 | | a
52 | | for a in revision_heads_map_ast_obj.body
53 | | if isinstance(a, ast.Assign) and a.targets[0].id == "REVISION_HEADS_MAP"
54 | | ][0]
| |____^ RUF015
55 |
56 | # RUF015 (zip)
|
= help: Replace with `next(...)`

Suggested fix
47 47 | ][0]
48 48 |
49 49 | # RUF015 (multi-line)
50 |-revision_heads_map_ast = [
50 |+revision_heads_map_ast = next(
51 51 | a
52 52 | for a in revision_heads_map_ast_obj.body
53 53 | if isinstance(a, ast.Assign) and a.targets[0].id == "REVISION_HEADS_MAP"
54 |-][0]
54 |+)
55 55 |
56 56 | # RUF015 (zip)
57 57 | list(zip(x, y))[0]

RUF015.py:57:1: RUF015 [*] Prefer `next(zip(x, y))` over single element slice
|
56 | # RUF015 (zip)
57 | list(zip(x, y))[0]
| ^^^^^^^^^^^^^^^^^^ RUF015
58 | [*zip(x, y)][0]
|
= help: Replace with `next(zip(x, y))`

Suggested fix
54 54 | ][0]
55 55 |
56 56 | # RUF015 (zip)
57 |-list(zip(x, y))[0]
57 |+next(zip(x, y))
58 58 | [*zip(x, y)][0]
59 59 |
60 60 |

RUF015.py:58:1: RUF015 [*] Prefer `next(zip(x, y))` over single element slice
|
56 | # RUF015 (zip)
57 | list(zip(x, y))[0]
58 | [*zip(x, y)][0]
| ^^^^^^^^^^^^^^^ RUF015
|
= help: Replace with `next(zip(x, y))`

Suggested fix
55 55 |
56 56 | # RUF015 (zip)
57 57 | list(zip(x, y))[0]
58 |-[*zip(x, y)][0]
58 |+next(zip(x, y))
59 59 |
60 60 |
61 61 | def test():

RUF015.py:63:5: RUF015 [*] Prefer `next(iter(zip(x, y)))` over single element slice
|
61 | def test():
62 | zip = list # Overwrite the builtin zip
63 | list(zip(x, y))[0]
| ^^^^^^^^^^^^^^^^^^ RUF015
|
= help: Replace with `next(iter(zip(x, y)))`

Suggested fix
60 60 |
61 61 | def test():
62 62 | zip = list # Overwrite the builtin zip
63 |- list(zip(x, y))[0]
63 |+ next(iter(zip(x, y)))


8 changes: 8 additions & 0 deletions crates/ruff_python_stdlib/src/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,3 +337,11 @@ pub fn is_builtin(name: &str) -> bool {
| "zip"
)
}

/// Returns `true` if the given name is that of a Python builtin iterator.
pub fn is_iterator(name: &str) -> bool {
matches!(
name,
"enumerate" | "filter" | "map" | "reversed" | "zip" | "iter"
)
}

0 comments on commit f64e10e

Please sign in to comment.