fix(4716): fix false positive unnecessary_dict_index_lookup emitted…

… when `del` is used (#5344) * fix(4716): fix false positive `unnecessary_dict_index_lookup` emitted when `del` is used Co-authored-by: Pierre Sassoulas <[email protected]>
pylint-dev · Dec 3, 2021 · ef250da · ef250da
1 parent 5bb0095
commit ef250da
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 9 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -14,18 +14,14 @@ Release date: TBA
 * Some files in ``pylint.testutils`` were deprecated. In the future imports should be done from the
   ``pylint.testutils.functional`` namespace directly.
 
-..
-  Insert your changelog randomly, it will reduce merge conflicts
-  (Ie. not necessarily at the end)
+* Fix ``unnecessary_dict_index_lookup`` false positive when deleting a dictionary's entry.
 
+  Closes #4716
 
-What's New in Pylint 2.12.3?
-============================
-Release date: TBA
-
-..
-  Put bug fixes that should not wait for a new minor version here
+* Fix false negative for ``consider-iterating-dictionary`` during membership checks encapsulated in iterables
+  or ``not in`` checks
 
+  Closes #5323
 
 ..
   Insert your changelog randomly, it will reduce merge conflicts

diff --git a/doc/whatsnew/2.13.rst b/doc/whatsnew/2.13.rst
@@ -22,6 +22,15 @@ Extensions
 Other Changes
 =============
 
+* Fix ``unnecessary_dict_index_lookup`` false positive when deleting a dictionary's entry.
+
+  Closes #4716
+
+* Fix false negative for ``consider-iterating-dictionary`` during membership checks encapsulated in iterables
+  or ``not in`` checks
+
+  Closes #5323
+
 * Require Python ``3.6.2`` to run pylint.
 
   Closes #5065
diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py
@@ -1873,6 +1873,9 @@ def _check_unnecessary_dict_index_lookup(
                         # Ignore this subscript if it is the target of an assignment
                         # Early termination; after reassignment dict index lookup will be necessary
                         return
+                    if isinstance(subscript.parent, nodes.Delete):
+                        # Ignore this subscript if it's used with the delete keyword
+                        return
 
                     # Case where .items is assigned to k,v (i.e., for k, v in d.items())
                     if isinstance(value, nodes.Name):

diff --git a/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.py b/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.py
@@ -82,3 +82,21 @@ class Foo:
     if 'V' in d[k]:  # [unnecessary-dict-index-lookup]
         d[k] = "value"
         print(d[k])  # This is fine
+
+# Test false positive described in #4716
+# Should not be emitted for del
+# (https://github.com/PyCQA/pylint/issues/4716)
+d = {}
+for key, val in d.items():
+    del d[key]
+    break
+
+for item in d.items():
+    del d[item[0]]
+    break
+
+outer_dict = {"inner_dict": {}}
+for key, val in outer_dict.items():
+    for key_two, val_two in val.items():
+        del outer_dict[key][key_two]  # [unnecessary-dict-index-lookup]
+        break
diff --git a/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.txt b/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.txt
@@ -21,3 +21,4 @@ unnecessary-dict-index-lookup:57:10:57:20::Unnecessary dictionary index lookup,
 unnecessary-dict-index-lookup:60:1:60:11::Unnecessary dictionary index lookup, use 'item[1]' instead:UNDEFINED
 unnecessary-dict-index-lookup:65:10:65:20::Unnecessary dictionary index lookup, use 'item[1]' instead:UNDEFINED
 unnecessary-dict-index-lookup:82:14:82:18::Unnecessary dictionary index lookup, use '_' instead:UNDEFINED
+unnecessary-dict-index-lookup:101:12:101:27::Unnecessary dictionary index lookup, use 'val' instead:UNDEFINED