[pylint] Recurse into subscript subexpressions when searching for list/dict lookups (PLR1733, PLR1736) (#13186)

## Summary

The `SequenceIndexVisitor` currently does not recurse into
subexpressions of subscripts when searching for subscript accesses that
would trigger this rule. That means that we don't currently detect
violations of the rule on snippets like this:

```py
data = {"a": 1, "b": 2}
column_names = ["a", "b"]
for index, column_name in enumerate(column_names):
    _ = data[column_names[index]]
```

Fixes #13183

## Test Plan

`cargo test -p ruff_linter`
This commit is contained in:
Alex Waygood 2024-09-01 17:22:45 +01:00 committed by GitHub
parent 2014cba87f
commit 3abd5c08a5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 90 additions and 16 deletions

View file

@ -38,3 +38,16 @@ def value_intentionally_unused():
print(FRUITS[fruit_name]) # OK print(FRUITS[fruit_name]) # OK
blah = FRUITS[fruit_name] # OK blah = FRUITS[fruit_name] # OK
assert FRUITS[fruit_name] == "pear" # OK assert FRUITS[fruit_name] == "pear" # OK
def rewrite_client_arrays(value_arrays: dict[str, list[int]]) -> dict[str, list[int]]:
"""Function from https://github.com/zulip/zulip/blob/3da91e951cd03cfa0b9c67378224e348353f36a6/analytics/views/stats.py#L617C1-L626C25"""
mapped_arrays: dict[str, list[int]] = {}
for label, array in value_arrays.items():
mapped_label = client_label_map(label)
if mapped_label in mapped_arrays:
for i in range(len(array)):
mapped_arrays[mapped_label][i] += value_arrays[label][i] # PLR1733
else:
mapped_arrays[mapped_label] = [value_arrays[label][i] for i in range(len(array))] # PLR1733
return mapped_arrays

View file

@ -76,3 +76,10 @@ def start():
# PLR1736 # PLR1736
for index, list_item in enumerate(some_list): for index, list_item in enumerate(some_list):
print(some_list[index]) print(some_list[index])
def nested_index_lookup():
data = {"a": 1, "b": 2}
column_names = ["a", "b"]
for index, column_name in enumerate(column_names):
_ = data[column_names[index]] # PLR1736

View file

@ -144,27 +144,25 @@ impl<'a> Visitor<'_> for SequenceIndexVisitor<'a> {
if self.modified { if self.modified {
return; return;
} }
match expr { if let Expr::Subscript(ast::ExprSubscript {
Expr::Subscript(ast::ExprSubscript { value,
value, slice,
slice, range,
range, ..
.. }) = expr
}) => { {
let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else { if let Expr::Name(ast::ExprName { id, .. }) = &**value {
return;
};
if id == self.sequence_name { if id == self.sequence_name {
let Expr::Name(ast::ExprName { id, .. }) = slice.as_ref() else { if let Expr::Name(ast::ExprName { id, .. }) = &**slice {
return; if id == self.index_name {
}; self.accesses.push(*range);
if id == self.index_name { }
self.accesses.push(*range);
} }
} }
} }
_ => visitor::walk_expr(self, expr),
} }
visitor::walk_expr(self, expr);
} }
} }

View file

@ -121,4 +121,41 @@ unnecessary_dict_index_lookup.py:11:16: PLR1733 [*] Unnecessary lookup of dictio
13 13 | 13 13 |
14 14 | def dont_fix_these(): 14 14 | def dont_fix_these():
unnecessary_dict_index_lookup.py:50:51: PLR1733 [*] Unnecessary lookup of dictionary value by key
|
48 | if mapped_label in mapped_arrays:
49 | for i in range(len(array)):
50 | mapped_arrays[mapped_label][i] += value_arrays[label][i] # PLR1733
| ^^^^^^^^^^^^^^^^^^^ PLR1733
51 | else:
52 | mapped_arrays[mapped_label] = [value_arrays[label][i] for i in range(len(array))] # PLR1733
|
= help: Use existing variable
Safe fix
47 47 | mapped_label = client_label_map(label)
48 48 | if mapped_label in mapped_arrays:
49 49 | for i in range(len(array)):
50 |- mapped_arrays[mapped_label][i] += value_arrays[label][i] # PLR1733
50 |+ mapped_arrays[mapped_label][i] += array[i] # PLR1733
51 51 | else:
52 52 | mapped_arrays[mapped_label] = [value_arrays[label][i] for i in range(len(array))] # PLR1733
53 53 | return mapped_arrays
unnecessary_dict_index_lookup.py:52:44: PLR1733 [*] Unnecessary lookup of dictionary value by key
|
50 | mapped_arrays[mapped_label][i] += value_arrays[label][i] # PLR1733
51 | else:
52 | mapped_arrays[mapped_label] = [value_arrays[label][i] for i in range(len(array))] # PLR1733
| ^^^^^^^^^^^^^^^^^^^ PLR1733
53 | return mapped_arrays
|
= help: Use existing variable
Safe fix
49 49 | for i in range(len(array)):
50 50 | mapped_arrays[mapped_label][i] += value_arrays[label][i] # PLR1733
51 51 | else:
52 |- mapped_arrays[mapped_label] = [value_arrays[label][i] for i in range(len(array))] # PLR1733
52 |+ mapped_arrays[mapped_label] = [array[i] for i in range(len(array))] # PLR1733
53 53 | return mapped_arrays

View file

@ -218,3 +218,22 @@ unnecessary_list_index_lookup.py:78:15: PLR1736 [*] List index lookup in `enumer
77 77 | for index, list_item in enumerate(some_list): 77 77 | for index, list_item in enumerate(some_list):
78 |- print(some_list[index]) 78 |- print(some_list[index])
78 |+ print(list_item) 78 |+ print(list_item)
79 79 |
80 80 |
81 81 | def nested_index_lookup():
unnecessary_list_index_lookup.py:85:18: PLR1736 [*] List index lookup in `enumerate()` loop
|
83 | column_names = ["a", "b"]
84 | for index, column_name in enumerate(column_names):
85 | _ = data[column_names[index]] # PLR1736
| ^^^^^^^^^^^^^^^^^^^ PLR1736
|
= help: Use the loop variable directly
Safe fix
82 82 | data = {"a": 1, "b": 2}
83 83 | column_names = ["a", "b"]
84 84 | for index, column_name in enumerate(column_names):
85 |- _ = data[column_names[index]] # PLR1736
85 |+ _ = data[column_name] # PLR1736