Compare commits

...

3 Commits

Author SHA1 Message Date
Robin Caloudis
0f53feee00 Extend doc 2024-02-27 23:08:21 +01:00
Robin Caloudis
b793611fa6 Extend rule
This implementation makes the test
pass again.
2024-02-27 23:08:02 +01:00
Robin Caloudis
be61cb3a68 Reproduce lint rule limitation by a test
This little test makes sure that we have
something to test against when extending
the rule.
2024-02-27 22:49:09 +01:00
4 changed files with 96 additions and 39 deletions

View File

@@ -57,6 +57,11 @@ revision_heads_map_ast = [
list(zip(x, y))[0]
[*zip(x, y)][0]
# RUF015 (pop)
list(x).pop(0)
# OK
list(x).pop(1)
def test():
zip = list # Overwrite the builtin zip

View File

@@ -116,7 +116,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
flake8_simplify::rules::use_capital_environment_variables(checker, expr);
}
if checker.enabled(Rule::UnnecessaryIterableAllocationForFirstElement) {
ruff::rules::unnecessary_iterable_allocation_for_first_element(checker, subscript);
ruff::rules::unnecessary_iterable_allocation_for_first_element(checker, expr);
}
if checker.enabled(Rule::InvalidIndexType) {
ruff::rules::invalid_index_type(checker, subscript);
@@ -965,6 +965,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::DefaultFactoryKwarg) {
ruff::rules::default_factory_kwarg(checker, call);
}
if checker.enabled(Rule::UnnecessaryIterableAllocationForFirstElement) {
ruff::rules::unnecessary_iterable_allocation_for_first_element(checker, expr);
}
}
Expr::Dict(dict) => {
if checker.any_enabled(&[

View File

@@ -11,14 +11,21 @@ use crate::checkers::ast::Checker;
use crate::fix::snippet::SourceCodeSnippet;
/// ## What it does
/// Checks for uses of `list(...)[0]` that can be replaced with
/// `next(iter(...))`.
/// Checks the following constructs, all of which can be replaced by
/// `next(iter(...))`:
///
/// - `list(...)[0]`
/// - `tuple(...)[0]`
/// - `list(i for i in ...)[0]`
/// - `[i for i in ...][0]`
/// - `list(...).pop(0)`
///
/// ## Why is this bad?
/// Calling `list(...)` will create a new list of the entire collection, which
/// can be very expensive for large collections. If you only need the first
/// element of the collection, you can use `next(...)` or `next(iter(...)` to
/// lazily fetch the first element.
/// Calling e.g. `list(...)` will create a new list of the entire collection,
/// which can be very expensive for large collections. If you only need the
/// first element of the collection, you can use `next(...)` or
/// `next(iter(...)` to lazily fetch the first element. The same is true for
/// the other constructs.
///
/// ## Example
/// ```python
@@ -33,14 +40,16 @@ use crate::fix::snippet::SourceCodeSnippet;
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as unsafe, as migrating from `list(...)[0]` to
/// `next(iter(...))` can change the behavior of your program in two ways:
/// This rule's fix is marked as unsafe, as migrating from e.g. `list(...)[0]`
/// to `next(iter(...))` can change the behavior of your program in two ways:
///
/// 1. First, `list(...)` will eagerly evaluate the entire collection, while
/// `next(iter(...))` will only evaluate the first element. As such, any
/// side effects that occur during iteration will be delayed.
/// 2. Second, `list(...)[0]` will raise `IndexError` if the collection is
/// empty, while `next(iter(...))` will raise `StopIteration`.
/// 1. First, all above mentioned constructs will eagerly evaluate the entire
/// collection, while `next(iter(...))` will only evaluate the first
/// element. As such, any side effects that occur during iteration will be
/// delayed.
/// 2. Second, accessing members of a collection via square bracket notation
/// `[0]` of the `pop()` function will raise `IndexError` if the collection
/// is empty, while `next(iter(...))` will raise `StopIteration`.
///
/// ## References
/// - [Iterators and Iterables in Python: Run Efficient Iterations](https://realpython.com/python-iterators-iterables/#when-to-use-an-iterator-in-python)
@@ -67,18 +76,37 @@ impl AlwaysFixableViolation for UnnecessaryIterableAllocationForFirstElement {
/// RUF015
pub(crate) fn unnecessary_iterable_allocation_for_first_element(
checker: &mut Checker,
subscript: &ast::ExprSubscript,
expr: &ast::Expr,
) {
let ast::ExprSubscript {
value,
slice,
range,
..
} = subscript;
if !is_head_slice(slice) {
return;
}
let (value, range) = match expr {
ast::Expr::Subscript(ast::ExprSubscript {
value,
slice,
range,
..
}) => {
if !is_head_slice(slice) {
return;
}
(value, range)
}
ast::Expr::Call(ast::ExprCall {
func, arguments, ..
}) => {
let Some(arg) = arguments.args.first() else {
return;
};
if !is_head_slice(arg) {
return;
}
let ast::Expr::Attribute(ast::ExprAttribute { range, value, .. }) = func.as_ref()
else {
return;
};
(value, range)
}
_ => return,
};
let Some(target) = match_iteration_target(value, checker.semantic()) else {
return;

View File

@@ -1,5 +1,6 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
assertion_line: 58
---
RUF015.py:4:1: RUF015 [*] Prefer `next(iter(x))` over single element slice
|
@@ -383,7 +384,7 @@ RUF015.py:57:1: RUF015 [*] Prefer `next(zip(x, y))` over single element slice
57 |+next(zip(x, y))
58 58 | [*zip(x, y)][0]
59 59 |
60 60 |
60 60 | # RUF015 (pop)
RUF015.py:58:1: RUF015 [*] Prefer `next(zip(x, y))` over single element slice
|
@@ -391,6 +392,8 @@ RUF015.py:58:1: RUF015 [*] Prefer `next(zip(x, y))` over single element slice
57 | list(zip(x, y))[0]
58 | [*zip(x, y)][0]
| ^^^^^^^^^^^^^^^ RUF015
59 |
60 | # RUF015 (pop)
|
= help: Replace with `next(zip(x, y))`
@@ -401,23 +404,41 @@ RUF015.py:58:1: RUF015 [*] Prefer `next(zip(x, y))` over single element slice
58 |-[*zip(x, y)][0]
58 |+next(zip(x, y))
59 59 |
60 60 |
61 61 | def test():
60 60 | # RUF015 (pop)
61 61 | list(x).pop(0)
RUF015.py:63:5: RUF015 [*] Prefer `next(iter(zip(x, y)))` over single element slice
RUF015.py:61:1: RUF015 [*] Prefer `next(iter(x))` over single element slice
|
61 | def test():
62 | zip = list # Overwrite the builtin zip
63 | list(zip(x, y))[0]
60 | # RUF015 (pop)
61 | list(x).pop(0)
| ^^^^^^^^^^^ RUF015
62 |
63 | # OK
|
= help: Replace with `next(iter(x))`
Unsafe fix
58 58 | [*zip(x, y)][0]
59 59 |
60 60 | # RUF015 (pop)
61 |-list(x).pop(0)
61 |+next(iter(x))(0)
62 62 |
63 63 | # OK
64 64 | list(x).pop(1)
RUF015.py:68:5: RUF015 [*] Prefer `next(iter(zip(x, y)))` over single element slice
|
66 | def test():
67 | zip = list # Overwrite the builtin zip
68 | list(zip(x, y))[0]
| ^^^^^^^^^^^^^^^^^^ RUF015
|
= help: Replace with `next(iter(zip(x, y)))`
Unsafe 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)))
65 65 |
66 66 | def test():
67 67 | zip = list # Overwrite the builtin zip
68 |- list(zip(x, y))[0]
68 |+ next(iter(zip(x, y)))