Skip to content

Commit

Permalink
tweak logic for assignments
Browse files Browse the repository at this point in the history
  • Loading branch information
@diceroll123
diceroll123 committed Nov 4, 2023
1 parent f2d1f1e commit ee65ba8
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 66 deletions.
9 changes: 3 additions & 6 deletions crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_list_index_lookup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ def fix_these():
for index, letter in enumerate(letters):
print(letters[index]) # PLR1736
blah = letters[index] # PLR1736
letters[index]: str = letters[index] # PLR1736 (on the right hand)
letters[index] += letters[index] # PLR1736 (on the right hand)
letters[index] = letters[index] # PLR1736 (on the right hand)
assert letters[index] == "d" # PLR1736


def dont_fix_these():
# once there is an assignment to the sequence[index], we stop emitting diagnostics
for index, letter in enumerate(letters):
letters[index] = "d" # Ok
assert letters[index] == "d" # Ok


def value_intentionally_unused():
Expand All @@ -27,7 +27,4 @@ def value_intentionally_unused():
for index, _ in enumerate(letters):
print(letters[index]) # Ok
blah = letters[index] # Ok
letters[index]: str = letters[index] # Ok
letters[index] += letters[index] # Ok
letters[index] = letters[index] # Ok
letters[index] = "d" # Ok
40 changes: 37 additions & 3 deletions crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ struct SubscriptVisitor<'a> {
sequence_name: &'a str,
index_name: &'a str,
diagnostic_ranges: Vec<TextRange>,
is_subcript_modified: bool,
}

impl<'a> SubscriptVisitor<'a> {
Expand All @@ -55,12 +56,35 @@ impl<'a> SubscriptVisitor<'a> {
sequence_name,
index_name,
diagnostic_ranges: Vec::new(),
is_subcript_modified: false,
}
}
}

fn check_target_for_assignment(expr: &Expr, sequence_name: &str, index_name: &str) -> bool {
// if we see the sequence subscript being modified, we'll stop emitting diagnostics
match expr {
Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => {
if let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() {
if id == sequence_name {
if let Expr::Name(ast::ExprName { id, .. }) = slice.as_ref() {
if id == index_name {
return true;
}
}
}
}
false
}
_ => false,
}
}

impl<'a> Visitor<'_> for SubscriptVisitor<'a> {
fn visit_expr(&mut self, expr: &Expr) {
if self.is_subcript_modified {
return;
}
match expr {
Expr::Subscript(ast::ExprSubscript {
value,
Expand All @@ -83,16 +107,26 @@ impl<'a> Visitor<'_> for SubscriptVisitor<'a> {
}

fn visit_stmt(&mut self, stmt: &Stmt) {
if self.is_subcript_modified {
return;
}
match stmt {
Stmt::Assign(ast::StmtAssign { value, .. }) => {
Stmt::Assign(ast::StmtAssign { targets, value, .. }) => {
self.is_subcript_modified = targets.iter().any(|target| {
check_target_for_assignment(target, self.sequence_name, self.index_name)
});
self.visit_expr(value);
}
Stmt::AnnAssign(ast::StmtAnnAssign { value, .. }) => {
Stmt::AnnAssign(ast::StmtAnnAssign { target, value, .. }) => {
if let Some(value) = value {
self.is_subcript_modified =
check_target_for_assignment(target, self.sequence_name, self.index_name);
self.visit_expr(value);
}
}
Stmt::AugAssign(ast::StmtAugAssign { value, .. }) => {
Stmt::AugAssign(ast::StmtAugAssign { target, value, .. }) => {
self.is_subcript_modified =
check_target_for_assignment(target, self.sequence_name, self.index_name);
self.visit_expr(value);
}
_ => visitor::walk_stmt(self, stmt),
Expand Down
72 changes: 15 additions & 57 deletions ...napshots/ruff_linter__rules__pylint__tests__PLR1736_unnecessary_list_index_lookup.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ unnecessary_list_index_lookup.py:10:15: PLR1736 [*] Unnecessary list index looku
10 | print(letters[index]) # PLR1736
| ^^^^^^^^^^^^^^ PLR1736
11 | blah = letters[index] # PLR1736
12 | letters[index]: str = letters[index] # PLR1736 (on the right hand)
12 | assert letters[index] == "d" # PLR1736
|
= help: Remove unnecessary list index lookup

Expand All @@ -79,17 +79,16 @@ unnecessary_list_index_lookup.py:10:15: PLR1736 [*] Unnecessary list index looku
10 |- print(letters[index]) # PLR1736
10 |+ print(letter) # PLR1736
11 11 | blah = letters[index] # PLR1736
12 12 | letters[index]: str = letters[index] # PLR1736 (on the right hand)
13 13 | letters[index] += letters[index] # PLR1736 (on the right hand)
12 12 | assert letters[index] == "d" # PLR1736
13 13 |

unnecessary_list_index_lookup.py:11:16: PLR1736 [*] Unnecessary list index lookup
|
9 | for index, letter in enumerate(letters):
10 | print(letters[index]) # PLR1736
11 | blah = letters[index] # PLR1736
| ^^^^^^^^^^^^^^ PLR1736
12 | letters[index]: str = letters[index] # PLR1736 (on the right hand)
13 | letters[index] += letters[index] # PLR1736 (on the right hand)
12 | assert letters[index] == "d" # PLR1736
|
= help: Remove unnecessary list index lookup

Expand All @@ -99,68 +98,27 @@ unnecessary_list_index_lookup.py:11:16: PLR1736 [*] Unnecessary list index looku
10 10 | print(letters[index]) # PLR1736
11 |- blah = letters[index] # PLR1736
11 |+ blah = letter # PLR1736
12 12 | letters[index]: str = letters[index] # PLR1736 (on the right hand)
13 13 | letters[index] += letters[index] # PLR1736 (on the right hand)
14 14 | letters[index] = letters[index] # PLR1736 (on the right hand)
12 12 | assert letters[index] == "d" # PLR1736
13 13 |
14 14 |

unnecessary_list_index_lookup.py:12:31: PLR1736 [*] Unnecessary list index lookup
unnecessary_list_index_lookup.py:12:16: PLR1736 [*] Unnecessary list index lookup
|
10 | print(letters[index]) # PLR1736
11 | blah = letters[index] # PLR1736
12 | letters[index]: str = letters[index] # PLR1736 (on the right hand)
| ^^^^^^^^^^^^^^ PLR1736
13 | letters[index] += letters[index] # PLR1736 (on the right hand)
14 | letters[index] = letters[index] # PLR1736 (on the right hand)
12 | assert letters[index] == "d" # PLR1736
| ^^^^^^^^^^^^^^ PLR1736
|
= help: Remove unnecessary list index lookup

ℹ Fix
9 9 | for index, letter in enumerate(letters):
10 10 | print(letters[index]) # PLR1736
11 11 | blah = letters[index] # PLR1736
12 |- letters[index]: str = letters[index] # PLR1736 (on the right hand)
12 |+ letters[index]: str = letter # PLR1736 (on the right hand)
13 13 | letters[index] += letters[index] # PLR1736 (on the right hand)
14 14 | letters[index] = letters[index] # PLR1736 (on the right hand)
15 15 |

unnecessary_list_index_lookup.py:13:27: PLR1736 [*] Unnecessary list index lookup
|
11 | blah = letters[index] # PLR1736
12 | letters[index]: str = letters[index] # PLR1736 (on the right hand)
13 | letters[index] += letters[index] # PLR1736 (on the right hand)
| ^^^^^^^^^^^^^^ PLR1736
14 | letters[index] = letters[index] # PLR1736 (on the right hand)
|
= help: Remove unnecessary list index lookup

ℹ Fix
10 10 | print(letters[index]) # PLR1736
11 11 | blah = letters[index] # PLR1736
12 12 | letters[index]: str = letters[index] # PLR1736 (on the right hand)
13 |- letters[index] += letters[index] # PLR1736 (on the right hand)
13 |+ letters[index] += letter # PLR1736 (on the right hand)
14 14 | letters[index] = letters[index] # PLR1736 (on the right hand)
15 15 |
16 16 |

unnecessary_list_index_lookup.py:14:26: PLR1736 [*] Unnecessary list index lookup
|
12 | letters[index]: str = letters[index] # PLR1736 (on the right hand)
13 | letters[index] += letters[index] # PLR1736 (on the right hand)
14 | letters[index] = letters[index] # PLR1736 (on the right hand)
| ^^^^^^^^^^^^^^ PLR1736
|
= help: Remove unnecessary list index lookup

ℹ Fix
11 11 | blah = letters[index] # PLR1736
12 12 | letters[index]: str = letters[index] # PLR1736 (on the right hand)
13 13 | letters[index] += letters[index] # PLR1736 (on the right hand)
14 |- letters[index] = letters[index] # PLR1736 (on the right hand)
14 |+ letters[index] = letter # PLR1736 (on the right hand)
15 15 |
16 16 |
17 17 | def dont_fix_these():
12 |- assert letters[index] == "d" # PLR1736
12 |+ assert letter == "d" # PLR1736
13 13 |
14 14 |
15 15 | def dont_fix_these():


0 comments on commit ee65ba8

Please sign in to comment.