fix false positive of `unnecessary-dict-index-lookup`

3 files changed, 52 insertions, 26 deletions

diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py
index e771db750..c17545ded 100644
--- a/pylint/checkers/refactoring/refactoring_checker.py
+++ b/pylint/checkers/refactoring/refactoring_checker.py
@@ -1689,7 +1689,14 @@ class RefactoringChecker(checkers.BaseTokenChecker):
                         and subscript == subscript.parent.target
                     ):
                         # Ignore this subscript if it is the target of an assignment
-                        continue
+                        if subscript.as_string() == subscript.parent.value.as_string():
+                            # Fire error as d[k] += d[k] has an unnecessary index lookup
+                            self.add_message(
+                                "unnecessary-dict-index-lookup",
+                                node=subscript,
+                                args=(node.target.elts[1].as_string()),
+                            )
+                        return  # Early termination; after reassignment dict index lookup will be necessary
 
                     # Case where .items is assigned to k,v (i.e., for k, v in d.items())
                     if isinstance(value, astroid.Name):
diff --git a/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.py b/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.py
index 901e176b3..1e56b046b 100644
--- a/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.py
+++ b/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.py
@@ -8,9 +8,11 @@ for k, v in a_dict.items():
     print(b_dict[k])  # Should not emit warning, accessing other dictionary
     a_dict[k] = 123  # Should not emit warning, key access necessary
     a_dict[k] += 123  # Should not emit warning, key access necessary
-    print(a_dict[k])  # [unnecessary-dict-index-lookup]
+    print(a_dict[k])  # Should not emit warning, v != a_dict[k]
+
+for k, v in b_dict.items():
     k = "another key"
-    print(a_dict[k])  # This is fine, key reassigned
+    print(b_dict[k])  # This is fine, key reassigned
 
 
 # Tests on comprehensions
@@ -61,3 +63,20 @@ for item in d.items():
     print(d[item[0]])  # [unnecessary-dict-index-lookup]
     item = (2, "b")
     print(d[item[0]])  # This is fine, no warning thrown as key has been reassigned
+
+
+# Test false positive described in #4630
+# (https://github.com/PyCQA/pylint/issues/4630)
+
+d = {'key': 'value'}
+
+for k, _ in d.items():
+    d[k] += 'VALUE'
+    if 'V' in d[k]:  # This is fine, if d[k] is replaced with _, the semantics change
+        print('found V')
+
+
+for k, _ in d.items():
+    if 'V' in d[k]:  # [unnecessary-dict-index-lookup]
+        d[k] = "value"
+        print(d[k])  # This is fine
diff --git a/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.txt b/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.txt
index dedf9e7cc..c0d5d6d16 100644
--- a/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.txt
+++ b/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.txt
@@ -1,23 +1,23 @@
-unnecessary-dict-index-lookup:7:10::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:11:10::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:17:36::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:19:1::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:20:1::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:20:44::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:22:33::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:24:1::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:25:1::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:25:41::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:34:10::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:35:21::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:39:40::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:41:1::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:42:1::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:42:52::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:44:37::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:46:1::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:47:1::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:47:49::Unnecessary dictionary index lookup, use 'v' instead
-unnecessary-dict-index-lookup:53:10::Unnecessary dictionary index lookup, use 'item[1]' instead
-unnecessary-dict-index-lookup:56:1::Unnecessary dictionary index lookup, use 'item[1]' instead
-unnecessary-dict-index-lookup:61:10::Unnecessary dictionary index lookup, use 'item[1]' instead
+unnecessary-dict-index-lookup:7:10::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:19:36::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:21:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:22:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:22:44::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:24:33::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:26:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:27:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:27:41::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:36:10::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:37:4::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:41:40::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:43:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:44:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:44:52::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:46:37::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:48:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:49:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:49:49::Unnecessary dictionary index lookup, use 'v' instead:HIGH
+unnecessary-dict-index-lookup:55:10::Unnecessary dictionary index lookup, use 'item[1]' instead:HIGH
+unnecessary-dict-index-lookup:58:1::Unnecessary dictionary index lookup, use 'item[1]' instead:HIGH
+unnecessary-dict-index-lookup:63:10::Unnecessary dictionary index lookup, use 'item[1]' instead:HIGH
+unnecessary-dict-index-lookup:80:14::Unnecessary dictionary index lookup, use '_' instead:HIGH