fix(consider-interating-dictionary): fix false negatives

5 files changed, 57 insertions, 31 deletions

diff --git a/ChangeLog b/ChangeLog
index 53f238b33..fe943eaad 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -57,6 +57,10 @@ Release date: 2021-11-24
 
   Closes #4412 #5287
 
+* Fix false negative ``consider-iterating-dictionary`` during membership checks
+
+  Closes #5323
+
 * Fix ``install graphiz`` message which isn't needed for puml output format.
 
 * ``MessageTest`` of the unittest ``testutil`` now requires the ``confidence`` attribute
diff --git a/doc/whatsnew/2.12.rst b/doc/whatsnew/2.12.rst
index be4559af8..690f032a6 100644
--- a/doc/whatsnew/2.12.rst
+++ b/doc/whatsnew/2.12.rst
@@ -97,6 +97,10 @@ Extensions
 Other Changes
 =============
 
+* Fix false negative ``consider-iterating-dictionary`` during membership checks
+
+  Closes #5323
+
 * Fix ``install graphiz`` message which isn't needed for puml output format.
 
 * ``pylint`` no longer crashes when checking assignment expressions within if-statements
diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py
index 210513ddc..a767794fa 100644
--- a/pylint/checkers/refactoring/recommendation_checker.py
+++ b/pylint/checkers/refactoring/recommendation_checker.py
@@ -83,21 +83,23 @@ class RecommendationChecker(checkers.BaseChecker):
             return
         if node.func.attrname != "keys":
             return
-        if (
-            isinstance(node.parent, (nodes.For, nodes.Comprehension))
-            or isinstance(node.parent, nodes.Compare)
-            and any(
-                op
-                for op, comparator in node.parent.ops
-                if op == "in" and comparator is node
-            )
+        inferred = utils.safe_infer(node.func)
+        if not isinstance(inferred, astroid.BoundMethod) or not isinstance(
+            inferred.bound, nodes.Dict
         ):
-            inferred = utils.safe_infer(node.func)
-            if not isinstance(inferred, astroid.BoundMethod) or not isinstance(
-                inferred.bound, nodes.Dict
-            ):
-                return
+            return
 
+        if isinstance(node.parent, (nodes.For, nodes.Comprehension)):
+            self.add_message("consider-iterating-dictionary", node=node)
+
+        while not isinstance(node.parent, (nodes.Compare, nodes.Module)):
+            node = node.parent
+
+        if isinstance(node.parent, nodes.Compare) and any(
+            op
+            for op, comparator in node.parent.ops
+            if op in ["in", "not in"] and comparator is node
+        ):
             self.add_message("consider-iterating-dictionary", node=node)
 
     def _check_use_maxsplit_arg(self, node: nodes.Call) -> None:
diff --git a/tests/functional/c/consider/consider_iterating_dictionary.py b/tests/functional/c/consider/consider_iterating_dictionary.py
index d3cc75f4d..469a274b4 100644
--- a/tests/functional/c/consider/consider_iterating_dictionary.py
+++ b/tests/functional/c/consider/consider_iterating_dictionary.py
@@ -66,3 +66,15 @@ VAR = 1 in {}.keys() # [consider-iterating-dictionary]
 VAR = 1 in {}
 VAR = 1 in dict()
 VAR = [1, 2] == {}.keys() in {False}
+
+# Additional membership checks
+# Issue #5323
+metadata = {}
+if "a" not in list(metadata.keys()): # [consider-iterating-dictionary]
+    print(1)
+if "a" not in metadata.keys(): # [consider-iterating-dictionary]
+    print(1)
+if "a" in list(metadata.keys()): # [consider-iterating-dictionary]
+    print(1)
+if "a" in metadata.keys(): # [consider-iterating-dictionary]
+    print(1)
diff --git a/tests/functional/c/consider/consider_iterating_dictionary.txt b/tests/functional/c/consider/consider_iterating_dictionary.txt
index 305b38e52..c2187aed7 100644
--- a/tests/functional/c/consider/consider_iterating_dictionary.txt
+++ b/tests/functional/c/consider/consider_iterating_dictionary.txt
@@ -1,18 +1,22 @@
-consider-iterating-dictionary:25:16:25:25::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:26:16:26:25::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:27:16:27:25::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:28:21:28:30::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:29:24:29:33::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:30:24:30:33::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:31:24:31:33::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:32:29:32:38::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:33:11:33:20::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:38:24:38:35::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:38:55:38:66::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:39:31:39:42::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:39:61:39:72::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:40:30:40:41::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:40:60:40:71::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:43:8:43:21::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:45:8:45:17::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
-consider-iterating-dictionary:65:11:65:20::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:25:16:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:26:16:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:27:16:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:28:21:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:29:24:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:30:24:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:31:24:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:32:29:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:33:11:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:38:24:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:38:55:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:39:31:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:39:61:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:40:30:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:40:60:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:43:8:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:45:8:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:65:11:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:73:14:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:75:14:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:77:10:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED
+consider-iterating-dictionary:79:10:None:None::Consider iterating the dictionary directly instead of calling .keys():UNDEFINED