diff options
-rw-r--r-- | ChangeLog | 5 | ||||
-rw-r--r-- | pylint/checkers/refactoring/recommendation_checker.py | 2 | ||||
-rw-r--r-- | tests/functional/c/consider/consider_using_dict_items.py | 21 | ||||
-rw-r--r-- | tests/functional/c/consider/consider_using_dict_items.txt | 31 |
4 files changed, 42 insertions, 17 deletions
@@ -27,6 +27,11 @@ Release date: TBA Closes #4644 +* Fix false-positive of ``unnecessary-dict-index-lookup`` and ``consider-using-dict-items`` + for reassigned dict index lookups + + Closes #4630 + What's New in Pylint 2.9.1? =========================== diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py index 6ea387126..d7d48e031 100644 --- a/pylint/checkers/refactoring/recommendation_checker.py +++ b/pylint/checkers/refactoring/recommendation_checker.py @@ -233,7 +233,7 @@ class RecommendationChecker(checkers.BaseChecker): and subscript == subscript.parent.target ): # Ignore this subscript if it is the target of an assignment - continue + return # Early termination as dict index lookup is necessary self.add_message("consider-using-dict-items", node=node) return diff --git a/tests/functional/c/consider/consider_using_dict_items.py b/tests/functional/c/consider/consider_using_dict_items.py index 1d6c86741..b8d230ca5 100644 --- a/tests/functional/c/consider/consider_using_dict_items.py +++ b/tests/functional/c/consider/consider_using_dict_items.py @@ -1,5 +1,5 @@ """Emit a message for iteration through dict keys and subscripting dict with key."""
-# pylint: disable=line-too-long,missing-docstring,unsubscriptable-object,too-few-public-methods
+# pylint: disable=line-too-long,missing-docstring,unsubscriptable-object,too-few-public-methods,redefined-outer-name
def bad():
a_dict = {1: 1, 2: 2, 3: 3}
@@ -84,3 +84,22 @@ val = any(True for k8 in Foo.c_dict if c_dict[k8]) # Should emit warning, using .keys() of Foo.c_dict
val = any(True for k8 in Foo.c_dict.keys() if Foo.c_dict[k8]) # [consider-iterating-dictionary,consider-using-dict-items]
+
+# Test false positive described in #4630
+# (https://github.com/PyCQA/pylint/issues/4630)
+
+d = {'key': 'value'}
+
+for k in d: # this is fine, with the reassignment of d[k], d[k] is necessary
+ d[k] += '123'
+ if '1' in d[k]: # index lookup necessary here, do not emit error
+ print('found 1')
+
+for k in d: # if this gets rewritten to d.items(), we are back to the above problem
+ d[k] = d[k] + 1
+ if '1' in d[k]: # index lookup necessary here, do not emit error
+ print('found 1')
+
+for k in d: # [consider-using-dict-items]
+ if '1' in d[k]: # index lookup necessary here, do not emit error
+ print('found 1')
diff --git a/tests/functional/c/consider/consider_using_dict_items.txt b/tests/functional/c/consider/consider_using_dict_items.txt index 8ead82351..9951d6848 100644 --- a/tests/functional/c/consider/consider_using_dict_items.txt +++ b/tests/functional/c/consider/consider_using_dict_items.txt @@ -1,15 +1,16 @@ -consider-using-dict-items:6:4:bad:Consider iterating with .items() -consider-using-dict-items:9:4:bad:Consider iterating with .items() -consider-using-dict-items:21:4:another_bad:Consider iterating with .items() -consider-using-dict-items:40:0::Consider iterating with .items() -consider-using-dict-items:44:0::Consider iterating with .items() -consider-iterating-dictionary:47:10::Consider iterating the dictionary directly instead of calling .keys() -consider-using-dict-items:47:0::Consider iterating with .items() -consider-using-dict-items:54:0::Consider iterating with .items() -consider-using-dict-items:67:0::Consider iterating with .items() -consider-using-dict-items:68:0::Consider iterating with .items() -consider-using-dict-items:71:0::Consider iterating with .items() -consider-using-dict-items:72:0::Consider iterating with .items() -consider-using-dict-items:75:0::Consider iterating with .items() -consider-iterating-dictionary:86:25::Consider iterating the dictionary directly instead of calling .keys() -consider-using-dict-items:86:0::Consider iterating with .items() +consider-using-dict-items:6:4:bad:Consider iterating with .items():HIGH +consider-using-dict-items:9:4:bad:Consider iterating with .items():HIGH +consider-using-dict-items:21:4:another_bad:Consider iterating with .items():HIGH +consider-using-dict-items:40:0::Consider iterating with .items():HIGH +consider-using-dict-items:44:0::Consider iterating with .items():HIGH +consider-iterating-dictionary:47:10::Consider iterating the dictionary directly instead of calling .keys():HIGH +consider-using-dict-items:47:0::Consider iterating with .items():HIGH +consider-using-dict-items:54:0::Consider iterating with .items():HIGH +consider-using-dict-items:67:0::Consider iterating with .items():HIGH +consider-using-dict-items:68:0::Consider iterating with .items():HIGH +consider-using-dict-items:71:0::Consider iterating with .items():HIGH +consider-using-dict-items:72:0::Consider iterating with .items():HIGH +consider-using-dict-items:75:0::Consider iterating with .items():HIGH +consider-iterating-dictionary:86:25::Consider iterating the dictionary directly instead of calling .keys():HIGH +consider-using-dict-items:86:0::Consider iterating with .items():HIGH +consider-using-dict-items:103:0::Consider iterating with .items():HIGH |