Skip to content

Commit

Permalink
fix: Fix issues based on PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mimre25 committed Jan 25, 2024
1 parent ba1cc7a commit d32ae77
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,27 @@ use ruff_diagnostics::Violation;
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{
visitor::{self, Visitor},
Expr, ExprAttribute, ExprCall, ExprContext, ExprName, ExprSubscript, Stmt, StmtDelete, StmtFor,
Expr, ExprAttribute, ExprCall, ExprName, ExprSubscript, Stmt, StmtDelete, StmtFor,
};
use ruff_text_size::Ranged;
use ruff_text_size::TextRange;

use crate::checkers::ast::Checker;
use ruff_diagnostics::Diagnostic;

static MUTATING_FUNCTIONS: &'static [&'static str] = &[
"append", "sort", "reverse", "remove", "clear", "extend", "insert", "pop", "popitem",
];

fn is_mutating_function(function_name: &str) -> bool {
matches!(
function_name,
"append"
| "sort"
| "reverse"
| "remove"
| "clear"
| "extend"
| "insert"
| "pop"
| "popitem"
)
}
/// ## What it does
/// Checks for mutation of the iterator of a loop in the loop's body
///
Expand Down Expand Up @@ -104,12 +114,12 @@ pub(crate) fn loop_iterator_mutation(checker: &mut Checker, stmt_for: &StmtFor)
for mutation in visitor.mutations {
checker
.diagnostics
.push(Diagnostic::new(LoopIteratorMutation {}, mutation.range()));
.push(Diagnostic::new(LoopIteratorMutation, mutation));
}
}
struct LoopMutationsVisitor<'a> {
name: &'a String,
mutations: Vec<Box<dyn Ranged + 'a>>,
name: &'a str,
mutations: Vec<TextRange>,
}

/// `Visitor` to collect all used identifiers in a statement.
Expand Down Expand Up @@ -138,7 +148,7 @@ impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> {
}
}
if self.name.eq(&name) {
self.mutations.push(Box::new(range));
self.mutations.push(*range);
}
}
}
Expand All @@ -150,30 +160,20 @@ impl<'a> Visitor<'a> for LoopMutationsVisitor<'a> {

fn visit_expr(&mut self, expr: &'a Expr) {
match expr {
Expr::Name(ExprName { range: _, id, ctx }) => {
if self.name.eq(id) {
match ctx {
ExprContext::Del => {
self.mutations.push(Box::new(expr));
}
_ => {}
}
}
}
Expr::Call(ExprCall {
range: _,
func,
arguments: _,
}) => match func.as_ref() {
Expr::Attribute(ExprAttribute {
range: _,
range,
value,
attr,
ctx: _,
}) => {
let name = _to_name_str(value);
if self.name.eq(&name) && MUTATING_FUNCTIONS.contains(&attr.as_str()) {
self.mutations.push(Box::new(expr));
if self.name.eq(&name) && is_mutating_function(&attr.as_str()) {
self.mutations.push(*range);
}
}
_ => {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ B038.py:11:9: B038 editing a loop's mutable iterable often leads to unexpected r
9 | print(elem)
10 | if elem % 2 == 0:
11 | some_list.remove(elem) # should error
| ^^^^^^^^^^^^^^^^^^^^^^ B038
| ^^^^^^^^^^^^^^^^ B038
12 |
13 | some_list = [1, 2, 3]
|
Expand All @@ -33,7 +33,7 @@ B038.py:41:9: B038 editing a loop's mutable iterable often leads to unexpected r
39 | print(elem)
40 | if elem % 2 == 0:
41 | a.some_list.remove(elem) # should error
| ^^^^^^^^^^^^^^^^^^^^^^^^ B038
| ^^^^^^^^^^^^^^^^^^ B038
42 |
43 | a = A((1, 2, 3))
|
Expand All @@ -51,7 +51,7 @@ B038.py:56:9: B038 editing a loop's mutable iterable often leads to unexpected r
54 | if elem == 2:
55 | found_idx = some_list.index(elem) # should not error
56 | some_list.append(elem) # should error
| ^^^^^^^^^^^^^^^^^^^^^^ B038
| ^^^^^^^^^^^^^^^^ B038
57 | some_list.sort() # should error
58 | some_list.reverse() # should error
|
Expand All @@ -61,7 +61,7 @@ B038.py:57:9: B038 editing a loop's mutable iterable often leads to unexpected r
55 | found_idx = some_list.index(elem) # should not error
56 | some_list.append(elem) # should error
57 | some_list.sort() # should error
| ^^^^^^^^^^^^^^^^ B038
| ^^^^^^^^^^^^^^ B038
58 | some_list.reverse() # should error
59 | some_list.clear() # should error
|
Expand All @@ -71,7 +71,7 @@ B038.py:58:9: B038 editing a loop's mutable iterable often leads to unexpected r
56 | some_list.append(elem) # should error
57 | some_list.sort() # should error
58 | some_list.reverse() # should error
| ^^^^^^^^^^^^^^^^^^^ B038
| ^^^^^^^^^^^^^^^^^ B038
59 | some_list.clear() # should error
60 | some_list.extend([1,2]) # should error
|
Expand All @@ -81,7 +81,7 @@ B038.py:59:9: B038 editing a loop's mutable iterable often leads to unexpected r
57 | some_list.sort() # should error
58 | some_list.reverse() # should error
59 | some_list.clear() # should error
| ^^^^^^^^^^^^^^^^^ B038
| ^^^^^^^^^^^^^^^ B038
60 | some_list.extend([1,2]) # should error
61 | some_list.insert(1, 1) # should error
|
Expand All @@ -91,7 +91,7 @@ B038.py:60:9: B038 editing a loop's mutable iterable often leads to unexpected r
58 | some_list.reverse() # should error
59 | some_list.clear() # should error
60 | some_list.extend([1,2]) # should error
| ^^^^^^^^^^^^^^^^^^^^^^^ B038
| ^^^^^^^^^^^^^^^^ B038
61 | some_list.insert(1, 1) # should error
62 | some_list.pop(1) # should error
|
Expand All @@ -101,7 +101,7 @@ B038.py:61:9: B038 editing a loop's mutable iterable often leads to unexpected r
59 | some_list.clear() # should error
60 | some_list.extend([1,2]) # should error
61 | some_list.insert(1, 1) # should error
| ^^^^^^^^^^^^^^^^^^^^^^ B038
| ^^^^^^^^^^^^^^^^ B038
62 | some_list.pop(1) # should error
63 | some_list.pop() # should error
|
Expand All @@ -111,7 +111,7 @@ B038.py:62:9: B038 editing a loop's mutable iterable often leads to unexpected r
60 | some_list.extend([1,2]) # should error
61 | some_list.insert(1, 1) # should error
62 | some_list.pop(1) # should error
| ^^^^^^^^^^^^^^^^ B038
| ^^^^^^^^^^^^^ B038
63 | some_list.pop() # should error
64 | some_list = 3 # should error
|
Expand All @@ -121,7 +121,7 @@ B038.py:63:9: B038 editing a loop's mutable iterable often leads to unexpected r
61 | some_list.insert(1, 1) # should error
62 | some_list.pop(1) # should error
63 | some_list.pop() # should error
| ^^^^^^^^^^^^^^^ B038
| ^^^^^^^^^^^^^ B038
64 | some_list = 3 # should error
65 | break
|
Expand All @@ -131,7 +131,7 @@ B038.py:74:9: B038 editing a loop's mutable iterable often leads to unexpected r
72 | if mydicts.get('a', ''):
73 | print(mydict['foo']) # should not error
74 | mydicts.popitem() # should error
| ^^^^^^^^^^^^^^^^^ B038
| ^^^^^^^^^^^^^^^ B038
|


0 comments on commit d32ae77

Please sign in to comment.