Skip to content

Commit

Permalink
Allow mutations with iterator key
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Apr 11, 2024
1 parent 42d4b69 commit 48c23b9
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 14 deletions.
Expand Up @@ -9,7 +9,7 @@
some_other_list = [1, 2, 3]
for elem in some_list:
# errors
some_list.remove(elem)
some_list.remove(0)
del some_list[2]
some_list.append(elem)
some_list.sort()
Expand All @@ -22,7 +22,7 @@

# conditional break should error
if elem == 2:
some_list.remove(elem)
some_list.remove(0)
if elem == 3:
break

Expand Down Expand Up @@ -81,7 +81,7 @@ def __init__(self, ls):
a = A((1, 2, 3))
# ensure member accesses are handled as errors
for elem in a.some_list:
a.some_list.remove(elem)
a.some_list.remove(0)
del a.some_list[2]


Expand Down Expand Up @@ -156,3 +156,5 @@ def __init__(self, ls):
for elem in some_list:
del some_list[elem]
some_list[elem] = 1
some_list.remove(elem)
some_list.discard(elem)
Expand Up @@ -7,8 +7,8 @@ use ruff_python_ast::comparable::ComparableExpr;
use ruff_python_ast::name::UnqualifiedName;
use ruff_python_ast::{
visitor::{self, Visitor},
Expr, ExprAttribute, ExprCall, ExprSubscript, Stmt, StmtAssign, StmtAugAssign, StmtBreak,
StmtDelete, StmtFor, StmtIf,
Arguments, Expr, ExprAttribute, ExprCall, ExprSubscript, Stmt, StmtAssign, StmtAugAssign,
StmtBreak, StmtDelete, StmtFor, StmtIf,
};
use ruff_text_size::TextRange;

Expand Down Expand Up @@ -186,7 +186,7 @@ impl<'a> LoopMutationsVisitor<'a> {
}

/// Handle, e.g., `items.append(1)`.
fn handle_call(&mut self, func: &Expr) {
fn handle_call(&mut self, func: &Expr, arguments: &Arguments) {
if let Expr::Attribute(ExprAttribute {
range,
value,
Expand All @@ -195,7 +195,17 @@ impl<'a> LoopMutationsVisitor<'a> {
}) = func
{
if is_mutating_function(attr.as_str()) {
// Find, e.g., `items.remove(1)`.
if ComparableExpr::from(self.iter) == ComparableExpr::from(value) {
// But allow, e.g., `for item in items: items.remove(item)`.
if arguments.len() == 1 && matches!(attr.as_str(), "remove" | "discard") {
if let [arg] = &*arguments.args {
if ComparableExpr::from(self.target) == ComparableExpr::from(arg) {
return;
}
}
}

self.add_mutation(*range);
}
}
Expand Down Expand Up @@ -271,8 +281,11 @@ impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> {

fn visit_expr(&mut self, expr: &'a Expr) {
// Ex) `items.append(1)`
if let Expr::Call(ExprCall { func, .. }) = expr {
self.handle_call(func);
if let Expr::Call(ExprCall {
func, arguments, ..
}) = expr
{
self.handle_call(func, arguments);
}

visitor::walk_expr(self, expr);
Expand Down
Expand Up @@ -5,7 +5,7 @@ B909.py:12:5: B909 Mutation to loop iterable `some_list` during iteration
|
10 | for elem in some_list:
11 | # errors
12 | some_list.remove(elem)
12 | some_list.remove(0)
| ^^^^^^^^^^^^^^^^ B909
13 | del some_list[2]
14 | some_list.append(elem)
Expand All @@ -14,7 +14,7 @@ B909.py:12:5: B909 Mutation to loop iterable `some_list` during iteration
B909.py:13:5: B909 Mutation to loop iterable `some_list` during iteration
|
11 | # errors
12 | some_list.remove(elem)
12 | some_list.remove(0)
13 | del some_list[2]
| ^^^^^^^^^^^^^^^^ B909
14 | some_list.append(elem)
Expand All @@ -23,7 +23,7 @@ B909.py:13:5: B909 Mutation to loop iterable `some_list` during iteration

B909.py:14:5: B909 Mutation to loop iterable `some_list` during iteration
|
12 | some_list.remove(elem)
12 | some_list.remove(0)
13 | del some_list[2]
14 | some_list.append(elem)
| ^^^^^^^^^^^^^^^^ B909
Expand Down Expand Up @@ -104,7 +104,7 @@ B909.py:25:9: B909 Mutation to loop iterable `some_list` during iteration
|
23 | # conditional break should error
24 | if elem == 2:
25 | some_list.remove(elem)
25 | some_list.remove(0)
| ^^^^^^^^^^^^^^^^ B909
26 | if elem == 3:
27 | break
Expand Down Expand Up @@ -202,15 +202,15 @@ B909.py:84:5: B909 Mutation to loop iterable `a.some_list` during iteration
|
82 | # ensure member accesses are handled as errors
83 | for elem in a.some_list:
84 | a.some_list.remove(elem)
84 | a.some_list.remove(0)
| ^^^^^^^^^^^^^^^^^^ B909
85 | del a.some_list[2]
|

B909.py:85:5: B909 Mutation to loop iterable `a.some_list` during iteration
|
83 | for elem in a.some_list:
84 | a.some_list.remove(elem)
84 | a.some_list.remove(0)
85 | del a.some_list[2]
| ^^^^^^^^^^^^^^^^^^ B909
|
Expand Down

0 comments on commit 48c23b9

Please sign in to comment.