From 2ba56777006b1f9e354acedbe10643751361098c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20H=C3=B8xbro=20Hansen?= Date: Sun, 8 Oct 2023 16:49:45 +0200 Subject: [PATCH] Improvements to RUF015 (#7848) ## Summary Resolves https://github.com/astral-sh/ruff/issues/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)) ``` --- .../resources/test/fixtures/ruff/RUF015.py | 16 ++ ...y_iterable_allocation_for_first_element.rs | 29 +++ ..._rules__ruff__tests__RUF015_RUF015.py.snap | 219 ++++++++++++++++-- crates/ruff_python_stdlib/src/builtins.rs | 8 + 4 files changed, 247 insertions(+), 25 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF015.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF015.py index 2aa92f467cfe8c..9ce388f6ceaba7 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF015.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF015.py @@ -38,6 +38,13 @@ 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 = [ @@ -45,3 +52,12 @@ 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] diff --git a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_iterable_allocation_for_first_element.rs b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_iterable_allocation_for_first_element.rs index 963240ac792c45..6c5265fadc087a 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_iterable_allocation_for_first_element.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_iterable_allocation_for_first_element.rs @@ -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; @@ -185,6 +186,10 @@ fn match_iteration_target(expr: &Expr, semantic: &SemanticModel) -> Option IterationTarget { + range: arg.range(), + iterable: is_func_builtin_iterator(func, semantic), + }, _ => IterationTarget { range: arg.range(), iterable: false, @@ -209,7 +214,24 @@ fn match_iteration_target(expr: &Expr, semantic: &SemanticModel) -> Option { + 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, }; @@ -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()) + }) +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF015_RUF015.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF015_RUF015.py.snap index 6e7933a4116267..81c22a35feddd6 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF015_RUF015.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF015_RUF015.py.snap @@ -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 | @@ -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))` @@ -202,8 +203,8 @@ 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 | @@ -211,8 +212,8 @@ RUF015.py:40:1: RUF015 [*] Prefer `next(iter(x["y"]))` over single element slice 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"]))` @@ -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))) diff --git a/crates/ruff_python_stdlib/src/builtins.rs b/crates/ruff_python_stdlib/src/builtins.rs index c39ce21cdfc39a..56412a19f6f873 100644 --- a/crates/ruff_python_stdlib/src/builtins.rs +++ b/crates/ruff_python_stdlib/src/builtins.rs @@ -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" + ) +}