Skip to content

Commit cf102ff

Browse files
Fix a false positive for consider-using-dict-items (#9594) (#9597)
When iterating ``os.environ`` using the ``os.environ.keys()`` operation and then deleting an item using the key as a lookup. Closes #9554 Co-authored-by: Pierre Sassoulas <[email protected]>> (cherry picked from commit c864cd4) Co-authored-by: Mark Byrne <[email protected]>
1 parent b8b9abd commit cf102ff

File tree

4 files changed

+48
-24
lines changed

4 files changed

+48
-24
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a false positive for ``consider-using-dict-items`` when iterating using ``keys()`` and then deleting an item using the key as a lookup.
2+
3+
Closes #9554

pylint/checkers/refactoring/recommendation_checker.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,10 @@ def _check_consider_using_dict_items(self, node: nodes.For) -> None:
308308
# Ignore this subscript if it is the target of an assignment
309309
# Early termination as dict index lookup is necessary
310310
return
311+
if isinstance(subscript.parent, nodes.Delete):
312+
# Ignore this subscript if the index is used to delete a
313+
# dictionary item.
314+
return
311315

312316
self.add_message("consider-using-dict-items", node=node)
313317
return

tests/functional/c/consider/consider_using_dict_items.py

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
"""Emit a message for iteration through dict keys and subscripting dict with key."""
2+
23
# pylint: disable=line-too-long,missing-docstring,unsubscriptable-object,too-few-public-methods,redefined-outer-name,use-dict-literal,modified-iterating-dict
34

5+
import os
6+
7+
48
def bad():
59
a_dict = {1: 1, 2: 2, 3: 3}
610
for k in a_dict: # [consider-using-dict-items]
@@ -15,12 +19,15 @@ def good():
1519
for k in a_dict:
1620
print(k)
1721

22+
1823
out_of_scope_dict = dict()
1924

25+
2026
def another_bad():
2127
for k in out_of_scope_dict: # [consider-using-dict-items]
2228
print(out_of_scope_dict[k])
2329

30+
2431
def another_good():
2532
for k in out_of_scope_dict:
2633
k = 1
@@ -47,9 +54,11 @@ def another_good():
4754
for k4 in b_dict.keys(): # [consider-iterating-dictionary,consider-using-dict-items]
4855
val = b_dict[k4]
4956

57+
5058
class Foo:
5159
c_dict = {}
5260

61+
5362
# Should emit warning when iterating over a dict attribute of a class
5463
for k5 in Foo.c_dict: # [consider-using-dict-items]
5564
val = Foo.c_dict[k5]
@@ -88,18 +97,25 @@ class Foo:
8897
# Test false positive described in #4630
8998
# (https://github.com/pylint-dev/pylint/issues/4630)
9099

91-
d = {'key': 'value'}
100+
d = {"key": "value"}
92101

93102
for k in d: # this is fine, with the reassignment of d[k], d[k] is necessary
94-
d[k] += '123'
95-
if '1' in d[k]: # index lookup necessary here, do not emit error
96-
print('found 1')
103+
d[k] += "123"
104+
if "1" in d[k]: # index lookup necessary here, do not emit error
105+
print("found 1")
97106

98107
for k in d: # if this gets rewritten to d.items(), we are back to the above problem
99108
d[k] = d[k] + 1
100-
if '1' in d[k]: # index lookup necessary here, do not emit error
101-
print('found 1')
109+
if "1" in d[k]: # index lookup necessary here, do not emit error
110+
print("found 1")
102111

103112
for k in d: # [consider-using-dict-items]
104-
if '1' in d[k]: # index lookup necessary here, do not emit error
105-
print('found 1')
113+
if "1" in d[k]: # index lookup necessary here, do not emit error
114+
print("found 1")
115+
116+
117+
# False positive in issue #9554
118+
# https://github.com/pylint-dev/pylint/issues/9554
119+
for var in os.environ.keys(): # [consider-iterating-dictionary]
120+
if var.startswith("foo_"):
121+
del os.environ[var] # index lookup necessary here, do not emit error
Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
1-
consider-using-dict-items:6:4:7:24:bad:Consider iterating with .items():UNDEFINED
2-
consider-using-dict-items:9:4:10:30:bad:Consider iterating with .items():UNDEFINED
3-
consider-using-dict-items:21:4:22:35:another_bad:Consider iterating with .items():UNDEFINED
4-
consider-using-dict-items:40:0:42:18::Consider iterating with .items():UNDEFINED
5-
consider-using-dict-items:44:0:45:20::Consider iterating with .items():UNDEFINED
6-
consider-iterating-dictionary:47:10:47:23::Consider iterating the dictionary directly instead of calling .keys():INFERENCE
7-
consider-using-dict-items:47:0:48:20::Consider iterating with .items():UNDEFINED
8-
consider-using-dict-items:54:0:55:24::Consider iterating with .items():UNDEFINED
9-
consider-using-dict-items:67:0:None:None::Consider iterating with .items():UNDEFINED
10-
consider-using-dict-items:68:0:None:None::Consider iterating with .items():UNDEFINED
11-
consider-using-dict-items:71:0:None:None::Consider iterating with .items():UNDEFINED
12-
consider-using-dict-items:72:0:None:None::Consider iterating with .items():UNDEFINED
13-
consider-using-dict-items:75:0:None:None::Consider iterating with .items():UNDEFINED
14-
consider-iterating-dictionary:86:25:86:42::Consider iterating the dictionary directly instead of calling .keys():INFERENCE
15-
consider-using-dict-items:86:0:None:None::Consider iterating with .items():UNDEFINED
16-
consider-using-dict-items:103:0:105:24::Consider iterating with .items():UNDEFINED
1+
consider-using-dict-items:10:4:11:24:bad:Consider iterating with .items():UNDEFINED
2+
consider-using-dict-items:13:4:14:30:bad:Consider iterating with .items():UNDEFINED
3+
consider-using-dict-items:27:4:28:35:another_bad:Consider iterating with .items():UNDEFINED
4+
consider-using-dict-items:47:0:49:18::Consider iterating with .items():UNDEFINED
5+
consider-using-dict-items:51:0:52:20::Consider iterating with .items():UNDEFINED
6+
consider-iterating-dictionary:54:10:54:23::Consider iterating the dictionary directly instead of calling .keys():INFERENCE
7+
consider-using-dict-items:54:0:55:20::Consider iterating with .items():UNDEFINED
8+
consider-using-dict-items:63:0:64:24::Consider iterating with .items():UNDEFINED
9+
consider-using-dict-items:76:0:None:None::Consider iterating with .items():UNDEFINED
10+
consider-using-dict-items:77:0:None:None::Consider iterating with .items():UNDEFINED
11+
consider-using-dict-items:80:0:None:None::Consider iterating with .items():UNDEFINED
12+
consider-using-dict-items:81:0:None:None::Consider iterating with .items():UNDEFINED
13+
consider-using-dict-items:84:0:None:None::Consider iterating with .items():UNDEFINED
14+
consider-iterating-dictionary:95:25:95:42::Consider iterating the dictionary directly instead of calling .keys():INFERENCE
15+
consider-using-dict-items:95:0:None:None::Consider iterating with .items():UNDEFINED
16+
consider-using-dict-items:112:0:114:24::Consider iterating with .items():UNDEFINED
17+
consider-iterating-dictionary:119:11:119:28::Consider iterating the dictionary directly instead of calling .keys():INFERENCE

0 commit comments

Comments
 (0)