fix false positive of `consider-using-dict-items`

4 files changed, 42 insertions, 17 deletions

diff --git a/ChangeLog b/ChangeLog
index ef043efa1..3482e61fe 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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