Make consider-iterating-dictionary consider membership check (#4997)

ChangeLog

@@ -97,6 +97,10 @@ Release date: TBA
 
   Closes #5000
 
+* The ``consider-iterating-dictionary`` checker now also considers membership checks
+
+  Closes #4069
+
 
 What's New in Pylint 2.10.3?
 ============================

doc/whatsnew/2.11.rst

@@ -85,3 +85,7 @@ Other Changes
 
   Closes #1375
   Closes #330
+
+* The ``consider-iterating-dictionary`` checker now also considers membership checks
+
+  Closes #4069

pylint/checkers/refactoring/recommendation_checker.py

@@ -24,9 +24,11 @@ class RecommendationChecker(checkers.BaseChecker):
         "C0201": (
             "Consider iterating the dictionary directly instead of calling .keys()",
             "consider-iterating-dictionary",
-            "Emitted when the keys of a dictionary are iterated through the .keys() "
-            "method. It is enough to just iterate through the dictionary itself, as "
-            'in "for key in dictionary".',
+            "Emitted when the keys of a dictionary are iterated through the ``.keys()`` "
+            "method or when ``.keys()`` is used for a membership check. "
+            "It is enough to iterate through the dictionary itself, "
+            "``for key in dictionary``. For membership checks, "
+            "``if key in dictionary`` is faster.",
         ),
         "C0206": (
             "Consider iterating with .items()",
@@ -75,16 +77,21 @@ def _check_consider_iterating_dictionary(self, node: nodes.Call) -> None:
             return
         if node.func.attrname != "keys":
             return
-        if not isinstance(node.parent, (nodes.For, nodes.Comprehension)):
-            return
-
-        inferred = utils.safe_infer(node.func)
-        if not isinstance(inferred, astroid.BoundMethod) or not isinstance(
-            inferred.bound, nodes.Dict
+        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
+            )
         ):
-            return
+            inferred = utils.safe_infer(node.func)
+            if not isinstance(inferred, astroid.BoundMethod) or not isinstance(
+                inferred.bound, nodes.Dict
+            ):
+                return
 
-        if isinstance(node.parent, (nodes.For, nodes.Comprehension)):
             self.add_message("consider-iterating-dictionary", node=node)
 
     def _check_use_maxsplit_arg(self, node: nodes.Call) -> None:

tests/functional/c/consider/consider_iterating_dictionary.py

@@ -1,4 +1,6 @@
-# pylint: disable=missing-docstring, expression-not-assigned, too-few-public-methods, no-member, import-error, no-self-use, line-too-long, useless-object-inheritance, unnecessary-comprehension
+# pylint: disable=missing-docstring, expression-not-assigned, too-few-public-methods
+# pylint: disable=no-member, import-error, no-self-use, line-too-long, useless-object-inheritance
+# pylint: disable=unnecessary-comprehension, use-dict-literal
 
 from unknown import Unknown
 
@@ -36,3 +38,31 @@ def keys(self):
 COMP1 = [k * 2 for k in DICT.keys()] + [k * 3 for k in DICT.keys()]  # [consider-iterating-dictionary,consider-iterating-dictionary]
 COMP2, COMP3 = [k * 2 for k in DICT.keys()], [k * 3 for k in DICT.keys()]  # [consider-iterating-dictionary,consider-iterating-dictionary]
 SOME_TUPLE = ([k * 2 for k in DICT.keys()], [k * 3 for k in DICT.keys()])  # [consider-iterating-dictionary,consider-iterating-dictionary]
+
+# Checks for membership checks
+if 1 in dict().keys(): # [consider-iterating-dictionary]
+    pass
+if 1 in {}.keys(): # [consider-iterating-dictionary]
+    pass
+if 1 in Unknown().keys():
+    pass
+if 1 in Unknown.keys():
+    pass
+if 1 in CustomClass().keys():
+    pass
+if 1 in dict():
+    pass
+if 1 in dict().values():
+    pass
+if (1, 1) in dict().items():
+    pass
+if [1] == {}.keys():
+    pass
+if [1] == {}:
+    pass
+if [1] == dict():
+    pass
+VAR = 1 in {}.keys() # [consider-iterating-dictionary]
+VAR = 1 in {}
+VAR = 1 in dict()
+VAR = [1, 2] == {}.keys() in {False}

tests/functional/c/consider/consider_iterating_dictionary.txt

@@ -1,15 +1,18 @@
-consider-iterating-dictionary:23:16::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:24:16::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:25:16::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:26:21::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:27:24::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:28:24::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:29:24::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:30:29::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:31:11::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:36:24::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:36:55::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:37:31::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:37:61::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:38:30::Consider iterating the dictionary directly instead of calling .keys()
-consider-iterating-dictionary:38:60::Consider iterating the dictionary directly instead of calling .keys()
+consider-iterating-dictionary:25:16::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:26:16::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:27:16::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:28:21::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:29:24::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:30:24::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:31:24::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:32:29::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:33:11::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:38:24::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:38:55::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:39:31::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:39:61::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:40:30::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:40:60::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:43:8::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:45:8::Consider iterating the dictionary directly instead of calling .keys():HIGH
+consider-iterating-dictionary:65:11::Consider iterating the dictionary directly instead of calling .keys():HIGH