Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements to RUF015 #7848

Merged
merged 16 commits into from
Oct 8, 2023
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(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can just use the range of the value here. (The previous logic would have failed if there was, e.g., space between the bracket and the star.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not think of that...

I was inspired by the logic done for the Expr::ListComp above.

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"
)
}