Skip to content

Commit

Permalink
Fix range
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Feb 28, 2024
1 parent 2ce293a commit 76d62d8
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 38 deletions.
2 changes: 2 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF015.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@

# RUF015 (pop)
list(x).pop(0)
[i for i in x].pop(0)
list(i for i in x).pop(0)

# OK
list(x).pop(1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use crate::fix::snippet::SourceCodeSnippet;
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as unsafe, as migrating from e.g. `list(...)[0]`
/// 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, all above mentioned constructs will eagerly evaluate the entire
Expand Down Expand Up @@ -76,39 +76,36 @@ impl AlwaysFixableViolation for UnnecessaryIterableAllocationForFirstElement {
/// RUF015
pub(crate) fn unnecessary_iterable_allocation_for_first_element(
checker: &mut Checker,
expr: &ast::Expr,
expr: &Expr,
) {
let (value, range) = match expr {
ast::Expr::Subscript(ast::ExprSubscript {
value,
slice,
range,
..
}) => {
if !is_head_slice(slice) {
let value = match expr {
// Ex) `list(x)[0]`
Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => {
if !is_zero(slice) {
return;
}
(value, range)
value
}
ast::Expr::Call(ast::ExprCall {
// Ex) `list(x).pop(0)`
Expr::Call(ast::ExprCall {
func, arguments, ..
}) => {
let Some(arg) = arguments.args.first() else {
if !arguments.keywords.is_empty() {
return;
}
let [arg] = arguments.args.as_ref() else {
return;
};
if !is_head_slice(arg) {
if !is_zero(arg) {
return;
}
let ast::Expr::Attribute(ast::ExprAttribute {
range, value, attr, ..
}) = func.as_ref()
else {
let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func.as_ref() else {
return;
};
if !matches!(attr.as_str(), "pop") {
return;
}
(value, range)
value
}
_ => return,
};
Expand All @@ -127,19 +124,19 @@ pub(crate) fn unnecessary_iterable_allocation_for_first_element(
UnnecessaryIterableAllocationForFirstElement {
iterable: SourceCodeSnippet::new(iterable.to_string()),
},
*range,
expr.range(),
);

diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
format!("next({iterable})"),
*range,
expr.range(),
)));

checker.diagnostics.push(diagnostic);
}

/// Check that the slice [`Expr`] is a slice of the first element (e.g., `x[0]`).
fn is_head_slice(expr: &Expr) -> bool {
fn is_zero(expr: &Expr) -> bool {
matches!(
expr,
Expr::NumberLiteral(ast::ExprNumberLiteral {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -410,9 +410,9 @@ RUF015.py:61:1: RUF015 [*] Prefer `next(iter(x))` over single element slice
|
60 | # RUF015 (pop)
61 | list(x).pop(0)
| ^^^^^^^^^^^ RUF015
62 |
63 | # OK
| ^^^^^^^^^^^^^^ RUF015
62 | [i for i in x].pop(0)
63 | list(i for i in x).pop(0)
|
= help: Replace with `next(iter(x))`

Expand All @@ -421,23 +421,64 @@ RUF015.py:61:1: RUF015 [*] Prefer `next(iter(x))` over single element slice
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)
61 |+next(iter(x))
62 62 | [i for i in x].pop(0)
63 63 | list(i for i in x).pop(0)
64 64 |

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

Unsafe fix
59 59 |
60 60 | # RUF015 (pop)
61 61 | list(x).pop(0)
62 |-[i for i in x].pop(0)
62 |+next(iter(x))
63 63 | list(i for i in x).pop(0)
64 64 |
65 65 | # OK

RUF015.py:63:1: RUF015 [*] Prefer `next(iter(x))` over single element slice
|
61 | list(x).pop(0)
62 | [i for i in x].pop(0)
63 | list(i for i in x).pop(0)
| ^^^^^^^^^^^^^^^^^^^^^^^^^ RUF015
64 |
65 | # OK
|
= help: Replace with `next(iter(x))`

Unsafe fix
60 60 | # RUF015 (pop)
61 61 | list(x).pop(0)
62 62 | [i for i in x].pop(0)
63 |-list(i for i in x).pop(0)
63 |+next(iter(x))
64 64 |
65 65 | # OK
66 66 | list(x).pop(1)

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

Unsafe fix
68 68 |
69 69 | def test():
70 70 | zip = list # Overwrite the builtin zip
71 |- list(zip(x, y))[0]
71 |+ next(iter(zip(x, y)))
70 70 |
71 71 | def test():
72 72 | zip = list # Overwrite the builtin zip
73 |- list(zip(x, y))[0]
73 |+ next(iter(zip(x, y)))

0 comments on commit 76d62d8

Please sign in to comment.