From b011d87cc58d7effe077c52aa587e661fc3670d8 Mon Sep 17 00:00:00 2001
From: Pang Yu Shao <p.yushao2@gmail.com>
Date: Thu, 6 May 2021 18:47:26 +0800
Subject: [PATCH 01/13] Implemented new check `consider-using-dict-items`

---
 CONTRIBUTORS.txt                              |  2 +
 ChangeLog                                     |  5 ++
 doc/whatsnew/2.9.rst                          |  3 +
 .../refactoring/recommendation_checker.py     | 71 ++++++++++++++++++-
 pylint/checkers/typecheck.py                  |  4 +-
 .../c/consider/consider_using_dict_items.py   | 17 +++++
 .../c/consider/consider_using_dict_items.txt  |  2 +
 7 files changed, 101 insertions(+), 3 deletions(-)
 create mode 100644 tests/functional/c/consider/consider_using_dict_items.py
 create mode 100644 tests/functional/c/consider/consider_using_dict_items.txt

diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index fb422cf7ba..7d1cd868b7 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -484,3 +484,5 @@ contributors:
 * Jiajunsu (victor): contributor
 
 * Andrew Haigh (nelfin): contributor
+
+* Pang Yu Shao (yushao2): contributor
diff --git a/ChangeLog b/ChangeLog
index de7a504220..20842af444 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -33,6 +33,11 @@ modules are added.
 
 * Fix raising false-positive ``no-member`` on abstract properties
 
+* ``consider-using-dict-items``, emitted when iterating over dictionary keys and then
+  indexing the same dictionary with the key within loop body.
+
+  Closes #3389
+
 
 What's New in Pylint 2.8.2?
 ===========================
diff --git a/doc/whatsnew/2.9.rst b/doc/whatsnew/2.9.rst
index a0053e98f0..906a6abfc7 100644
--- a/doc/whatsnew/2.9.rst
+++ b/doc/whatsnew/2.9.rst
@@ -12,6 +12,9 @@ Summary -- Release highlights
 New checkers
 ============
 
+* ``consider-using-dict-items``, emitted when iterating over dictionary keys and then
+  indexing the same dictionary with the key within loop body.
+
 Other Changes
 =============
 
diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py
index b1175f03e0..0bcd2cbf15 100644
--- a/pylint/checkers/refactoring/recommendation_checker.py
+++ b/pylint/checkers/refactoring/recommendation_checker.py
@@ -26,6 +26,14 @@ class RecommendationChecker(checkers.BaseChecker):
             "method. It is enough to just iterate through the dictionary itself, as "
             'in "for key in dictionary".',
         ),
+        "C0206": (
+            "Consider iterating with .items()",
+            "consider-using-dict-items",
+            "Emitted when the keys of a dictionary are iterated and the items of the "
+            "dictionary is accessed by indexing with the key in each iteration. "
+            "Both the key and value can be accessed by iterating using the .items() "
+            "method of the dictionary instead.",
+        ),
     }
 
     @staticmethod
@@ -53,8 +61,12 @@ def visit_call(self, node):
         if isinstance(node.parent, (astroid.For, astroid.Comprehension)):
             self.add_message("consider-iterating-dictionary", node=node)
 
-    @utils.check_messages("consider-using-enumerate")
+    @utils.check_messages("consider-using-enumerate", "consider-using-dict-items")
     def visit_for(self, node):
+        self._check_consider_using_enumerate(node)
+        self._check_consider_using_dict_items(node)
+
+    def _check_consider_using_enumerate(self, node):
         """Emit a convention whenever range and len are used for indexing."""
         # Verify that we have a `range([start], len(...), [stop])` call and
         # that the object which is iterated is used as a subscript in the
@@ -119,3 +131,60 @@ def visit_for(self, node):
                     continue
                 self.add_message("consider-using-enumerate", node=node)
                 return
+
+    def _check_consider_using_dict_items(self, node):
+        """Emit a convention whenever range and len are used for indexing."""
+        # Verify that we have a .keys() call and
+        # that the object which is iterated is used as a subscript in the
+        # body of the for.
+
+        if not isinstance(node.iter, astroid.Call):
+            # Is it a dictionary?
+            if not isinstance(node.iter, astroid.Name):
+                return
+            inferred = utils.safe_infer(node.iter)
+            if not isinstance(inferred, astroid.Dict) and not isinstance(
+                inferred, astroid.DictComp
+            ):
+                return
+            iterating_object_name = node.iter.as_string()
+
+        else:
+            # Is it a proper keys call?
+            if isinstance(node.iter.func, astroid.Name):
+                return
+            if node.iter.func.attrname != "keys":
+                return
+            inferred = utils.safe_infer(node.iter.func)
+            if not isinstance(inferred, astroid.BoundMethod) or not isinstance(
+                inferred.bound, astroid.Dict
+            ):
+                return
+            iterating_object_name = node.iter.as_string().split(".")[0]
+
+        # Verify that the body of the for loop uses a subscript
+        # with the object that was iterated. This uses some heuristics
+        # in order to make sure that the same object is used in the
+        # for body.
+        for child in node.body:
+            for subscript in child.nodes_of_class(astroid.Subscript):
+                if not isinstance(subscript.value, astroid.Name):
+                    continue
+
+                value = subscript.slice
+                if isinstance(value, astroid.Index):
+                    value = value.value
+                if not isinstance(value, astroid.Name):
+                    continue
+                if value.name != node.target.name:
+                    continue
+                if iterating_object_name != subscript.value.name:
+                    continue
+                if subscript.value.scope() != node.scope():
+                    # Ignore this subscript if it's not in the same
+                    # scope. This means that in the body of the for
+                    # loop, another scope was created, where the same
+                    # name for the iterating object was used.
+                    continue
+                self.add_message("consider-using-dict-items", node=node)
+                return
diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py
index 8ce0c9171e..f64960a218 100644
--- a/pylint/checkers/typecheck.py
+++ b/pylint/checkers/typecheck.py
@@ -1439,8 +1439,8 @@ def visit_call(self, node):
                         args=(display_name, callable_name),
                     )
 
-        for name in kwparams:
-            defval, assigned = kwparams[name]
+        for name, val in kwparams.items():
+            defval, assigned = val
             if (
                 defval is None
                 and not assigned
diff --git a/tests/functional/c/consider/consider_using_dict_items.py b/tests/functional/c/consider/consider_using_dict_items.py
new file mode 100644
index 0000000000..9b06772834
--- /dev/null
+++ b/tests/functional/c/consider/consider_using_dict_items.py
@@ -0,0 +1,17 @@
+"""Emit a message for iteration through dict keys and subscripting dict with key."""
+
+# pylint: disable=missing-docstring, import-error, useless-object-inheritance, unsubscriptable-object, too-few-public-methods
+
+def bad():
+    a_dict = {1:1, 2:2, 3:3}
+    for k in a_dict:# [consider-using-dict-items]
+        print(a_dict[k])
+    another_dict = dict()
+    for k in another_dict:# [consider-using-dict-items]
+        print(another_dict[k])
+
+
+def good():
+    a_dict = {1:1, 2:2, 3:3}
+    for k in a_dict:
+        print(k)
diff --git a/tests/functional/c/consider/consider_using_dict_items.txt b/tests/functional/c/consider/consider_using_dict_items.txt
new file mode 100644
index 0000000000..46662e5ce9
--- /dev/null
+++ b/tests/functional/c/consider/consider_using_dict_items.txt
@@ -0,0 +1,2 @@
+consider-using-dict-items:7:4:bad:Consider iterating with .items()
+consider-using-dict-items:10:4:bad:Consider iterating with .items()

From b92a5738160572c2089012e6ca18a6cf207c6e6f Mon Sep 17 00:00:00 2001
From: Pang Yu Shao <p.yushao2@gmail.com>
Date: Sun, 9 May 2021 23:12:24 +0800
Subject: [PATCH 02/13] Added additional logic and tests for
 ``consider-using-dict-items``

---
 ChangeLog                                     |  2 +-
 doc/whatsnew/2.9.rst                          |  2 +-
 .../refactoring/recommendation_checker.py     | 53 ++++++++++---------
 .../c/consider/consider_using_dict_items.py   | 19 +++++--
 .../c/consider/consider_using_dict_items.txt  |  1 +
 5 files changed, 46 insertions(+), 31 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 20842af444..b9e279ab9a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -33,7 +33,7 @@ modules are added.
 
 * Fix raising false-positive ``no-member`` on abstract properties
 
-* ``consider-using-dict-items``, emitted when iterating over dictionary keys and then
+* New checker ``consider-using-dict-items``. Emitted  when iterating over dictionary keys and then
   indexing the same dictionary with the key within loop body.
 
   Closes #3389
diff --git a/doc/whatsnew/2.9.rst b/doc/whatsnew/2.9.rst
index 906a6abfc7..8f5bd43f2f 100644
--- a/doc/whatsnew/2.9.rst
+++ b/doc/whatsnew/2.9.rst
@@ -12,7 +12,7 @@ Summary -- Release highlights
 New checkers
 ============
 
-* ``consider-using-dict-items``, emitted when iterating over dictionary keys and then
+* ``consider-using-dict-items``: Emitted when iterating over dictionary keys and then
   indexing the same dictionary with the key within loop body.
 
 Other Changes
diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py
index 0bcd2cbf15..608620726f 100644
--- a/pylint/checkers/refactoring/recommendation_checker.py
+++ b/pylint/checkers/refactoring/recommendation_checker.py
@@ -1,6 +1,8 @@
 # Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
 # For details: https://github.com/PyCQA/pylint/blob/master/LICENSE
 
+from typing import cast
+
 import astroid
 
 from pylint import checkers, interfaces
@@ -29,8 +31,8 @@ class RecommendationChecker(checkers.BaseChecker):
         "C0206": (
             "Consider iterating with .items()",
             "consider-using-dict-items",
-            "Emitted when the keys of a dictionary are iterated and the items of the "
-            "dictionary is accessed by indexing with the key in each iteration. "
+            "Emitted when iterating over the keys of a dictionary and accessing the "
+            "value by index lookup."
             "Both the key and value can be accessed by iterating using the .items() "
             "method of the dictionary instead.",
         ),
@@ -62,11 +64,11 @@ def visit_call(self, node):
             self.add_message("consider-iterating-dictionary", node=node)
 
     @utils.check_messages("consider-using-enumerate", "consider-using-dict-items")
-    def visit_for(self, node):
+    def visit_for(self, node: astroid.For) -> None:
         self._check_consider_using_enumerate(node)
         self._check_consider_using_dict_items(node)
 
-    def _check_consider_using_enumerate(self, node):
+    def _check_consider_using_enumerate(self, node: astroid.For) -> None:
         """Emit a convention whenever range and len are used for indexing."""
         # Verify that we have a `range([start], len(...), [stop])` call and
         # that the object which is iterated is used as a subscript in the
@@ -132,8 +134,8 @@ def _check_consider_using_enumerate(self, node):
                 self.add_message("consider-using-enumerate", node=node)
                 return
 
-    def _check_consider_using_dict_items(self, node):
-        """Emit a convention whenever range and len are used for indexing."""
+    def _check_consider_using_dict_items(self, node: astroid.For) -> None:
+        """Emit a convention when dict key used to index dict."""
         # Verify that we have a .keys() call and
         # that the object which is iterated is used as a subscript in the
         # body of the for.
@@ -143,24 +145,21 @@ def _check_consider_using_dict_items(self, node):
             if not isinstance(node.iter, astroid.Name):
                 return
             inferred = utils.safe_infer(node.iter)
-            if not isinstance(inferred, astroid.Dict) and not isinstance(
-                inferred, astroid.DictComp
-            ):
+            if not isinstance(inferred, (astroid.Dict, astroid.DictComp)):
                 return
             iterating_object_name = node.iter.as_string()
 
         else:
             # Is it a proper keys call?
-            if isinstance(node.iter.func, astroid.Name):
-                return
-            if node.iter.func.attrname != "keys":
+            if not (
+                isinstance(node.iter.func, astroid.Attribute)
+                and node.iter.func.attrname != "keys"
+            ):
                 return
             inferred = utils.safe_infer(node.iter.func)
-            if not isinstance(inferred, astroid.BoundMethod) or not isinstance(
-                inferred.bound, astroid.Dict
-            ):
+            if not isinstance(inferred, (astroid.BoundMethod, astroid.Dict)):
                 return
-            iterating_object_name = node.iter.as_string().split(".")[0]
+            iterating_object_name = node.iter.as_string().partition(".")[0]
 
         # Verify that the body of the for loop uses a subscript
         # with the object that was iterated. This uses some heuristics
@@ -168,23 +167,25 @@ def _check_consider_using_dict_items(self, node):
         # for body.
         for child in node.body:
             for subscript in child.nodes_of_class(astroid.Subscript):
+                subscript = cast(astroid.Subscript, subscript)
                 if not isinstance(subscript.value, astroid.Name):
                     continue
 
                 value = subscript.slice
                 if isinstance(value, astroid.Index):
                     value = value.value
-                if not isinstance(value, astroid.Name):
+                if (
+                    not isinstance(value, astroid.Name)
+                    or value.name != node.target.name
+                    or iterating_object_name != subscript.value.name
+                ):
                     continue
-                if value.name != node.target.name:
-                    continue
-                if iterating_object_name != subscript.value.name:
-                    continue
-                if subscript.value.scope() != node.scope():
-                    # Ignore this subscript if it's not in the same
-                    # scope. This means that in the body of the for
-                    # loop, another scope was created, where the same
-                    # name for the iterating object was used.
+                last_definition_lineno = value.lookup(value.name)[1][-1].lineno
+                if last_definition_lineno > node.lineno:
+                    # Ignore this subscript if it has been redefined after
+                    # the for loop. This checks for the line number using .lookup()
+                    # to get the line number where the iterating object was last
+                    # defined and compare that to the for loop's line number
                     continue
                 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 9b06772834..c8166835c7 100644
--- a/tests/functional/c/consider/consider_using_dict_items.py
+++ b/tests/functional/c/consider/consider_using_dict_items.py
@@ -1,13 +1,13 @@
 """Emit a message for iteration through dict keys and subscripting dict with key."""
 
-# pylint: disable=missing-docstring, import-error, useless-object-inheritance, unsubscriptable-object, too-few-public-methods
+# pylint: disable=missing-docstring,unsubscriptable-object
 
 def bad():
     a_dict = {1:1, 2:2, 3:3}
-    for k in a_dict:# [consider-using-dict-items]
+    for k in a_dict: # [consider-using-dict-items]
         print(a_dict[k])
     another_dict = dict()
-    for k in another_dict:# [consider-using-dict-items]
+    for k in another_dict: # [consider-using-dict-items]
         print(another_dict[k])
 
 
@@ -15,3 +15,16 @@ def good():
     a_dict = {1:1, 2:2, 3:3}
     for k in a_dict:
         print(k)
+
+out_of_scope_dict = dict()
+
+def another_bad():
+    for k in out_of_scope_dict: # [consider-using-dict-items]
+        print(out_of_scope_dict[k])
+
+def another_good():
+    for k in out_of_scope_dict:
+        k = 1
+        k = 2
+        k = 3
+        print(out_of_scope_dict[k])
diff --git a/tests/functional/c/consider/consider_using_dict_items.txt b/tests/functional/c/consider/consider_using_dict_items.txt
index 46662e5ce9..a86b74e978 100644
--- a/tests/functional/c/consider/consider_using_dict_items.txt
+++ b/tests/functional/c/consider/consider_using_dict_items.txt
@@ -1,2 +1,3 @@
 consider-using-dict-items:7:4:bad:Consider iterating with .items()
 consider-using-dict-items:10:4:bad:Consider iterating with .items()
+consider-using-dict-items:22:4:another_bad:Consider iterating with .items()

From b9af784f7f8a7a294ead2fe3051a87e5516e8746 Mon Sep 17 00:00:00 2001
From: Pang Yu Shao <p.yushao2@gmail.com>
Date: Tue, 11 May 2021 01:50:02 +0800
Subject: [PATCH 03/13] Implemented ``consider-using-dict-items`` on
 comprehension

- Also, added accompanying test cases
---
 .../refactoring/recommendation_checker.py     | 91 ++++++++++++++-----
 .../c/consider/consider_using_dict_items.py   | 35 ++++++-
 .../c/consider/consider_using_dict_items.txt  | 10 ++
 3 files changed, 112 insertions(+), 24 deletions(-)

diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py
index 608620726f..01c7f36445 100644
--- a/pylint/checkers/refactoring/recommendation_checker.py
+++ b/pylint/checkers/refactoring/recommendation_checker.py
@@ -1,7 +1,7 @@
 # Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
 # For details: https://github.com/PyCQA/pylint/blob/master/LICENSE
 
-from typing import cast
+from typing import Optional, Union, cast
 
 import astroid
 
@@ -9,6 +9,32 @@
 from pylint.checkers import utils
 
 
+def _check_if_dict_keys_used(
+    node: Union[astroid.For, astroid.Comprehension]
+) -> Optional[str]:
+    if not isinstance(node.iter, astroid.Call):
+        # Is it a dictionary?
+        if not isinstance(node.iter, (astroid.Name, astroid.Attribute)):
+            return None
+        inferred = utils.safe_infer(node.iter)
+        if not isinstance(inferred, (astroid.Dict, astroid.DictComp)):
+            return None
+        iterating_object_name = node.iter.as_string()
+
+    else:
+        # Is it a proper keys call?
+        if (
+            isinstance(node.iter.func, astroid.Name)
+            or node.iter.func.attrname != "keys"
+        ):
+            return None
+        inferred = utils.safe_infer(node.iter.func)
+        if not isinstance(inferred, (astroid.BoundMethod, astroid.Dict)):
+            return None
+        iterating_object_name = node.iter.as_string().partition(".")[0]
+    return iterating_object_name
+
+
 class RecommendationChecker(checkers.BaseChecker):
 
     __implements__ = (interfaces.IAstroidChecker,)
@@ -140,26 +166,9 @@ def _check_consider_using_dict_items(self, node: astroid.For) -> None:
         # that the object which is iterated is used as a subscript in the
         # body of the for.
 
-        if not isinstance(node.iter, astroid.Call):
-            # Is it a dictionary?
-            if not isinstance(node.iter, astroid.Name):
-                return
-            inferred = utils.safe_infer(node.iter)
-            if not isinstance(inferred, (astroid.Dict, astroid.DictComp)):
-                return
-            iterating_object_name = node.iter.as_string()
-
-        else:
-            # Is it a proper keys call?
-            if not (
-                isinstance(node.iter.func, astroid.Attribute)
-                and node.iter.func.attrname != "keys"
-            ):
-                return
-            inferred = utils.safe_infer(node.iter.func)
-            if not isinstance(inferred, (astroid.BoundMethod, astroid.Dict)):
-                return
-            iterating_object_name = node.iter.as_string().partition(".")[0]
+        iterating_object_name = _check_if_dict_keys_used(node)
+        if iterating_object_name is None:
+            return
 
         # Verify that the body of the for loop uses a subscript
         # with the object that was iterated. This uses some heuristics
@@ -168,7 +177,8 @@ def _check_consider_using_dict_items(self, node: astroid.For) -> None:
         for child in node.body:
             for subscript in child.nodes_of_class(astroid.Subscript):
                 subscript = cast(astroid.Subscript, subscript)
-                if not isinstance(subscript.value, astroid.Name):
+
+                if not isinstance(subscript.value, (astroid.Name, astroid.Attribute)):
                     continue
 
                 value = subscript.slice
@@ -177,8 +187,9 @@ def _check_consider_using_dict_items(self, node: astroid.For) -> None:
                 if (
                     not isinstance(value, astroid.Name)
                     or value.name != node.target.name
-                    or iterating_object_name != subscript.value.name
+                    or iterating_object_name != subscript.value.as_string()
                 ):
+
                     continue
                 last_definition_lineno = value.lookup(value.name)[1][-1].lineno
                 if last_definition_lineno > node.lineno:
@@ -187,5 +198,39 @@ def _check_consider_using_dict_items(self, node: astroid.For) -> None:
                     # to get the line number where the iterating object was last
                     # defined and compare that to the for loop's line number
                     continue
+                if (
+                    isinstance(subscript.parent, astroid.Assign)
+                    and subscript in subscript.parent.targets
+                ):
+                    # Ignore this subscript if it is the target of an assignment
+                    continue
+
+                self.add_message("consider-using-dict-items", node=node)
+                return
+
+    @utils.check_messages("consider-using-dict-items")
+    def visit_comprehension(self, node: astroid.Comprehension) -> None:
+        iterating_object_name = _check_if_dict_keys_used(node)
+        if iterating_object_name is None:
+            return
+
+        children = list(node.parent.get_children())
+        if node.ifs:
+            children += node.ifs
+        for child in children:
+            for subscript in child.nodes_of_class(astroid.Subscript):
+                subscript = cast(astroid.Subscript, subscript)
+                if not isinstance(subscript.value, (astroid.Name, astroid.Attribute)):
+                    continue
+
+                value = subscript.slice
+                if isinstance(value, astroid.Index):
+                    value = value.value
+                if (
+                    not isinstance(value, astroid.Name)
+                    or value.name != node.target.name
+                    or iterating_object_name != subscript.value.as_string()
+                ):
+                    continue
                 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 c8166835c7..e1940bbffb 100644
--- a/tests/functional/c/consider/consider_using_dict_items.py
+++ b/tests/functional/c/consider/consider_using_dict_items.py
@@ -1,6 +1,6 @@
 """Emit a message for iteration through dict keys and subscripting dict with key."""
 
-# pylint: disable=missing-docstring,unsubscriptable-object
+# pylint: disable=missing-docstring,unsubscriptable-object,too-few-public-methods
 
 def bad():
     a_dict = {1:1, 2:2, 3:3}
@@ -28,3 +28,36 @@ def another_good():
         k = 2
         k = 3
         print(out_of_scope_dict[k])
+
+
+b_dict = {}
+for k2 in b_dict:  # Should not emit warning, key access necessary
+    b_dict[k2] = 2
+
+for k3 in b_dict:  # [consider-using-dict-items]
+    val = b_dict[k3]
+
+for k4 in b_dict.keys():  # [consider-iterating-dictionary,consider-using-dict-items]
+    val = b_dict[k4]
+
+class Foo:
+    c_dict = {}
+
+for k5 in Foo.c_dict:  # [consider-using-dict-items]
+    val = Foo.c_dict[k5]
+
+for k6 in b_dict:  # [consider-using-dict-items]
+    val = b_dict[k6]
+    b_dict[k6] = 2
+
+c_dict={}
+
+val = [(k7, b_dict[k7]) for k7 in b_dict]  # [consider-using-dict-items]
+val = any(True for k8 in b_dict if b_dict[k8])  # [consider-using-dict-items]
+val = {k9: b_dict[k9] for k9 in b_dict}  # [consider-using-dict-items]
+val = [(k7, b_dict[k7]) for k7 in Foo.c_dict]
+val = any(True for k8 in Foo.c_dict if b_dict[k8])
+val = [(k7, c_dict[k7]) for k7 in Foo.c_dict]
+val = any(True for k8 in Foo.c_dict if c_dict[k8])
+val = [(k7, Foo.c_dict[k7]) for k7 in Foo.c_dict] # [consider-using-dict-items]
+val = any(True for k8 in Foo.c_dict if Foo.c_dict[k8]) # [consider-using-dict-items]
diff --git a/tests/functional/c/consider/consider_using_dict_items.txt b/tests/functional/c/consider/consider_using_dict_items.txt
index a86b74e978..d79fed6886 100644
--- a/tests/functional/c/consider/consider_using_dict_items.txt
+++ b/tests/functional/c/consider/consider_using_dict_items.txt
@@ -1,3 +1,13 @@
 consider-using-dict-items:7:4:bad:Consider iterating with .items()
 consider-using-dict-items:10:4:bad:Consider iterating with .items()
 consider-using-dict-items:22:4:another_bad:Consider iterating with .items()
+consider-using-dict-items:37:0::Consider iterating with .items()
+consider-iterating-dictionary:40:10::Consider iterating the dictionary directly instead of calling .keys()
+consider-using-dict-items:40:0::Consider iterating with .items()
+consider-using-dict-items:46:0::Consider iterating with .items()
+consider-using-dict-items:49:0::Consider iterating with .items()
+consider-using-dict-items:55:0::Consider iterating with .items()
+consider-using-dict-items:56:0::Consider iterating with .items()
+consider-using-dict-items:57:0::Consider iterating with .items()
+consider-using-dict-items:62:0::Consider iterating with .items()
+consider-using-dict-items:63:0::Consider iterating with .items()

From d2a85db588f48a7b259a913cb105353fe06ba87e Mon Sep 17 00:00:00 2001
From: Pang Yu Shao <p.yushao2@gmail.com>
Date: Tue, 11 May 2021 10:11:12 +0800
Subject: [PATCH 04/13] Grouped test items

---
 .../c/consider/consider_using_dict_items.py   | 26 +++++++++++++------
 .../c/consider/consider_using_dict_items.txt  | 16 ++++++------
 2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/tests/functional/c/consider/consider_using_dict_items.py b/tests/functional/c/consider/consider_using_dict_items.py
index e1940bbffb..0a51f2d788 100644
--- a/tests/functional/c/consider/consider_using_dict_items.py
+++ b/tests/functional/c/consider/consider_using_dict_items.py
@@ -34,6 +34,11 @@ def another_good():
 for k2 in b_dict:  # Should not emit warning, key access necessary
     b_dict[k2] = 2
 
+# Warning should be emitted in this case
+for k6 in b_dict:  # [consider-using-dict-items]
+    val = b_dict[k6]
+    b_dict[k6] = 2
+
 for k3 in b_dict:  # [consider-using-dict-items]
     val = b_dict[k3]
 
@@ -43,21 +48,26 @@ def another_good():
 class Foo:
     c_dict = {}
 
+# Should emit warning when iterating over a dict attribute of a class
 for k5 in Foo.c_dict:  # [consider-using-dict-items]
     val = Foo.c_dict[k5]
 
-for k6 in b_dict:  # [consider-using-dict-items]
-    val = b_dict[k6]
-    b_dict[k6] = 2
+# Should emit warning within a list/dict comprehension
+val = {k9: b_dict[k9] for k9 in b_dict}  # [consider-using-dict-items]
+val = [(k7, b_dict[k7]) for k7 in b_dict]  # [consider-using-dict-items]
 
-c_dict={}
+# Should emit warning even when using dict attribute of a class within comprehension
+val = [(k7, Foo.c_dict[k7]) for k7 in Foo.c_dict] # [consider-using-dict-items]
+val = any(True for k8 in Foo.c_dict if Foo.c_dict[k8]) # [consider-using-dict-items]
 
-val = [(k7, b_dict[k7]) for k7 in b_dict]  # [consider-using-dict-items]
+# Should emit warning when dict access done in ``if`` portion of comprehension
 val = any(True for k8 in b_dict if b_dict[k8])  # [consider-using-dict-items]
-val = {k9: b_dict[k9] for k9 in b_dict}  # [consider-using-dict-items]
+
+# Should NOT emit warning whey key used to access a different dict
 val = [(k7, b_dict[k7]) for k7 in Foo.c_dict]
 val = any(True for k8 in Foo.c_dict if b_dict[k8])
+
+# Should NOT emit warning, essentially same check as above
+c_dict={}
 val = [(k7, c_dict[k7]) for k7 in Foo.c_dict]
 val = any(True for k8 in Foo.c_dict if c_dict[k8])
-val = [(k7, Foo.c_dict[k7]) for k7 in Foo.c_dict] # [consider-using-dict-items]
-val = any(True for k8 in Foo.c_dict if Foo.c_dict[k8]) # [consider-using-dict-items]
diff --git a/tests/functional/c/consider/consider_using_dict_items.txt b/tests/functional/c/consider/consider_using_dict_items.txt
index d79fed6886..d159b1c2fc 100644
--- a/tests/functional/c/consider/consider_using_dict_items.txt
+++ b/tests/functional/c/consider/consider_using_dict_items.txt
@@ -1,13 +1,13 @@
 consider-using-dict-items:7:4:bad:Consider iterating with .items()
 consider-using-dict-items:10:4:bad:Consider iterating with .items()
 consider-using-dict-items:22:4:another_bad:Consider iterating with .items()
-consider-using-dict-items:37:0::Consider iterating with .items()
-consider-iterating-dictionary:40:10::Consider iterating the dictionary directly instead of calling .keys()
-consider-using-dict-items:40:0::Consider iterating with .items()
-consider-using-dict-items:46:0::Consider iterating with .items()
-consider-using-dict-items:49:0::Consider iterating with .items()
-consider-using-dict-items:55:0::Consider iterating with .items()
+consider-using-dict-items:38:0::Consider iterating with .items()
+consider-using-dict-items:42:0::Consider iterating with .items()
+consider-iterating-dictionary:45:10::Consider iterating the dictionary directly instead of calling .keys()
+consider-using-dict-items:45:0::Consider iterating with .items()
+consider-using-dict-items:52:0::Consider iterating with .items()
 consider-using-dict-items:56:0::Consider iterating with .items()
 consider-using-dict-items:57:0::Consider iterating with .items()
-consider-using-dict-items:62:0::Consider iterating with .items()
-consider-using-dict-items:63:0::Consider iterating with .items()
+consider-using-dict-items:60:0::Consider iterating with .items()
+consider-using-dict-items:61:0::Consider iterating with .items()
+consider-using-dict-items:64:0::Consider iterating with .items()

From c30df89ebbf119b0ac336732bf9539235602a50f Mon Sep 17 00:00:00 2001
From: Pang Yu Shao <p.yushao2@gmail.com>
Date: Tue, 11 May 2021 10:13:20 +0800
Subject: [PATCH 05/13] Refactored utility function

---
 .../refactoring/recommendation_checker.py     | 32 ++-----------------
 pylint/checkers/utils.py                      | 30 +++++++++++++++++
 2 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py
index 01c7f36445..1ba229b30c 100644
--- a/pylint/checkers/refactoring/recommendation_checker.py
+++ b/pylint/checkers/refactoring/recommendation_checker.py
@@ -1,7 +1,7 @@
 # Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
 # For details: https://github.com/PyCQA/pylint/blob/master/LICENSE
 
-from typing import Optional, Union, cast
+from typing import cast
 
 import astroid
 
@@ -9,32 +9,6 @@
 from pylint.checkers import utils
 
 
-def _check_if_dict_keys_used(
-    node: Union[astroid.For, astroid.Comprehension]
-) -> Optional[str]:
-    if not isinstance(node.iter, astroid.Call):
-        # Is it a dictionary?
-        if not isinstance(node.iter, (astroid.Name, astroid.Attribute)):
-            return None
-        inferred = utils.safe_infer(node.iter)
-        if not isinstance(inferred, (astroid.Dict, astroid.DictComp)):
-            return None
-        iterating_object_name = node.iter.as_string()
-
-    else:
-        # Is it a proper keys call?
-        if (
-            isinstance(node.iter.func, astroid.Name)
-            or node.iter.func.attrname != "keys"
-        ):
-            return None
-        inferred = utils.safe_infer(node.iter.func)
-        if not isinstance(inferred, (astroid.BoundMethod, astroid.Dict)):
-            return None
-        iterating_object_name = node.iter.as_string().partition(".")[0]
-    return iterating_object_name
-
-
 class RecommendationChecker(checkers.BaseChecker):
 
     __implements__ = (interfaces.IAstroidChecker,)
@@ -166,7 +140,7 @@ def _check_consider_using_dict_items(self, node: astroid.For) -> None:
         # that the object which is iterated is used as a subscript in the
         # body of the for.
 
-        iterating_object_name = _check_if_dict_keys_used(node)
+        iterating_object_name = utils.get_iterating_dictionary_name(node)
         if iterating_object_name is None:
             return
 
@@ -210,7 +184,7 @@ def _check_consider_using_dict_items(self, node: astroid.For) -> None:
 
     @utils.check_messages("consider-using-dict-items")
     def visit_comprehension(self, node: astroid.Comprehension) -> None:
-        iterating_object_name = _check_if_dict_keys_used(node)
+        iterating_object_name = utils.get_iterating_dictionary_name(node)
         if iterating_object_name is None:
             return
 
diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py
index 6c11eeb9d9..8aae906433 100644
--- a/pylint/checkers/utils.py
+++ b/pylint/checkers/utils.py
@@ -1492,3 +1492,33 @@ def is_assign_name_annotated_with(node: astroid.AssignName, typing_name: str) ->
     ):
         return True
     return False
+
+
+def get_iterating_dictionary_name(
+    node: Union[astroid.For, astroid.Comprehension]
+) -> Optional[str]:
+    """Get the name of the dictionary which keys are being iterated over on
+    a ``astroid.For`` or ``astroid.Comprehension`` node.
+
+    If the iterating object is not either the keys method of a dictionary
+    or a dictionary itself, this returns None
+    """
+    if isinstance(node.iter, astroid.Call):
+        # Is it a proper keys call?
+        if (
+            isinstance(node.iter.func, astroid.Name)
+            or node.iter.func.attrname != "keys"
+        ):
+            return None
+        inferred = safe_infer(node.iter.func)
+        if not isinstance(inferred, (astroid.BoundMethod, astroid.Dict)):
+            return None
+        return node.iter.as_string().partition(".")[0]
+
+    # Is it a dictionary?
+    if not isinstance(node.iter, (astroid.Name, astroid.Attribute)):
+        return None
+    inferred = safe_infer(node.iter)
+    if not isinstance(inferred, (astroid.Dict, astroid.DictComp)):
+        return None
+    return node.iter.as_string()

From 08ba8502e5a4bf8913b69200f45d3c6492fa31ea Mon Sep 17 00:00:00 2001
From: Pang Yu Shao <p.yushao2@gmail.com>
Date: Tue, 11 May 2021 10:27:17 +0800
Subject: [PATCH 06/13] Added logic to check for augassign statements

---
 .../checkers/refactoring/recommendation_checker.py |  8 +++++---
 .../c/consider/consider_using_dict_items.py        |  3 +++
 .../c/consider/consider_using_dict_items.txt       | 14 +++++++-------
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py
index 1ba229b30c..bc70680790 100644
--- a/pylint/checkers/refactoring/recommendation_checker.py
+++ b/pylint/checkers/refactoring/recommendation_checker.py
@@ -135,7 +135,7 @@ def _check_consider_using_enumerate(self, node: astroid.For) -> None:
                 return
 
     def _check_consider_using_dict_items(self, node: astroid.For) -> None:
-        """Emit a convention when dict key used to index dict."""
+        """Add message when accessing dict values by index lookup."""
         # Verify that we have a .keys() call and
         # that the object which is iterated is used as a subscript in the
         # body of the for.
@@ -173,8 +173,10 @@ def _check_consider_using_dict_items(self, node: astroid.For) -> None:
                     # defined and compare that to the for loop's line number
                     continue
                 if (
-                    isinstance(subscript.parent, astroid.Assign)
+                    isinstance(subscript.parent, (astroid.Assign))
                     and subscript in subscript.parent.targets
+                    or isinstance(subscript.parent, (astroid.AugAssign))
+                    and subscript == subscript.parent.target
                 ):
                     # Ignore this subscript if it is the target of an assignment
                     continue
@@ -190,7 +192,7 @@ def visit_comprehension(self, node: astroid.Comprehension) -> None:
 
         children = list(node.parent.get_children())
         if node.ifs:
-            children += node.ifs
+            children.extend(node.ifs)
         for child in children:
             for subscript in child.nodes_of_class(astroid.Subscript):
                 subscript = cast(astroid.Subscript, subscript)
diff --git a/tests/functional/c/consider/consider_using_dict_items.py b/tests/functional/c/consider/consider_using_dict_items.py
index 0a51f2d788..65e11a4c2d 100644
--- a/tests/functional/c/consider/consider_using_dict_items.py
+++ b/tests/functional/c/consider/consider_using_dict_items.py
@@ -34,6 +34,9 @@ def another_good():
 for k2 in b_dict:  # Should not emit warning, key access necessary
     b_dict[k2] = 2
 
+for k2 in b_dict:  # Should not emit warning, key access necessary (augassign)
+    b_dict[k2] += 2
+
 # Warning should be emitted in this case
 for k6 in b_dict:  # [consider-using-dict-items]
     val = b_dict[k6]
diff --git a/tests/functional/c/consider/consider_using_dict_items.txt b/tests/functional/c/consider/consider_using_dict_items.txt
index d159b1c2fc..fedd8a8ecd 100644
--- a/tests/functional/c/consider/consider_using_dict_items.txt
+++ b/tests/functional/c/consider/consider_using_dict_items.txt
@@ -1,13 +1,13 @@
 consider-using-dict-items:7:4:bad:Consider iterating with .items()
 consider-using-dict-items:10:4:bad:Consider iterating with .items()
 consider-using-dict-items:22:4:another_bad:Consider iterating with .items()
-consider-using-dict-items:38:0::Consider iterating with .items()
-consider-using-dict-items:42:0::Consider iterating with .items()
-consider-iterating-dictionary:45:10::Consider iterating the dictionary directly instead of calling .keys()
+consider-using-dict-items:41:0::Consider iterating with .items()
 consider-using-dict-items:45:0::Consider iterating with .items()
-consider-using-dict-items:52:0::Consider iterating with .items()
-consider-using-dict-items:56:0::Consider iterating with .items()
-consider-using-dict-items:57:0::Consider iterating with .items()
+consider-iterating-dictionary:48:10::Consider iterating the dictionary directly instead of calling .keys()
+consider-using-dict-items:48:0::Consider iterating with .items()
+consider-using-dict-items:55:0::Consider iterating with .items()
+consider-using-dict-items:59:0::Consider iterating with .items()
 consider-using-dict-items:60:0::Consider iterating with .items()
-consider-using-dict-items:61:0::Consider iterating with .items()
+consider-using-dict-items:63:0::Consider iterating with .items()
 consider-using-dict-items:64:0::Consider iterating with .items()
+consider-using-dict-items:67:0::Consider iterating with .items()

From 31bbd174c21f81978805975ae99815d89c588f91 Mon Sep 17 00:00:00 2001
From: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date: Tue, 11 May 2021 12:57:38 +0200
Subject: [PATCH 07/13] Minor style changes

---
 .../refactoring/recommendation_checker.py     |  2 --
 pylint/checkers/utils.py                      |  6 ++---
 .../c/consider/consider_using_dict_items.py   | 19 ++++++++--------
 .../c/consider/consider_using_dict_items.txt  | 22 +++++++++----------
 4 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py
index bc70680790..2be19e4f5e 100644
--- a/pylint/checkers/refactoring/recommendation_checker.py
+++ b/pylint/checkers/refactoring/recommendation_checker.py
@@ -1,6 +1,5 @@
 # Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
 # For details: https://github.com/PyCQA/pylint/blob/master/LICENSE
-
 from typing import cast
 
 import astroid
@@ -163,7 +162,6 @@ def _check_consider_using_dict_items(self, node: astroid.For) -> None:
                     or value.name != node.target.name
                     or iterating_object_name != subscript.value.as_string()
                 ):
-
                     continue
                 last_definition_lineno = value.lookup(value.name)[1][-1].lineno
                 if last_definition_lineno > node.lineno:
diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py
index 8aae906433..64fa300ae8 100644
--- a/pylint/checkers/utils.py
+++ b/pylint/checkers/utils.py
@@ -1498,13 +1498,13 @@ def get_iterating_dictionary_name(
     node: Union[astroid.For, astroid.Comprehension]
 ) -> Optional[str]:
     """Get the name of the dictionary which keys are being iterated over on
-    a ``astroid.For`` or ``astroid.Comprehension`` node.
+    a `astroid.For` or `astroid.Comprehension` node.
 
     If the iterating object is not either the keys method of a dictionary
-    or a dictionary itself, this returns None
+    or a dictionary itself, this returns None.
     """
+    # Is it a proper keys call?
     if isinstance(node.iter, astroid.Call):
-        # Is it a proper keys call?
         if (
             isinstance(node.iter.func, astroid.Name)
             or node.iter.func.attrname != "keys"
diff --git a/tests/functional/c/consider/consider_using_dict_items.py b/tests/functional/c/consider/consider_using_dict_items.py
index 65e11a4c2d..db3626e10a 100644
--- a/tests/functional/c/consider/consider_using_dict_items.py
+++ b/tests/functional/c/consider/consider_using_dict_items.py
@@ -1,25 +1,24 @@
 """Emit a message for iteration through dict keys and subscripting dict with key."""
-
 # pylint: disable=missing-docstring,unsubscriptable-object,too-few-public-methods
 
 def bad():
-    a_dict = {1:1, 2:2, 3:3}
-    for k in a_dict: # [consider-using-dict-items]
+    a_dict = {1: 1, 2: 2, 3: 3}
+    for k in a_dict:  # [consider-using-dict-items]
         print(a_dict[k])
     another_dict = dict()
-    for k in another_dict: # [consider-using-dict-items]
+    for k in another_dict:  # [consider-using-dict-items]
         print(another_dict[k])
 
 
 def good():
-    a_dict = {1:1, 2:2, 3:3}
+    a_dict = {1: 1, 2: 2, 3: 3}
     for k in a_dict:
         print(k)
 
 out_of_scope_dict = dict()
 
 def another_bad():
-    for k in out_of_scope_dict: # [consider-using-dict-items]
+    for k in out_of_scope_dict:  # [consider-using-dict-items]
         print(out_of_scope_dict[k])
 
 def another_good():
@@ -34,7 +33,7 @@ def another_good():
 for k2 in b_dict:  # Should not emit warning, key access necessary
     b_dict[k2] = 2
 
-for k2 in b_dict:  # Should not emit warning, key access necessary (augassign)
+for k2 in b_dict:  # Should not emit warning, key access necessary (AugAssign)
     b_dict[k2] += 2
 
 # Warning should be emitted in this case
@@ -60,8 +59,8 @@ class Foo:
 val = [(k7, b_dict[k7]) for k7 in b_dict]  # [consider-using-dict-items]
 
 # Should emit warning even when using dict attribute of a class within comprehension
-val = [(k7, Foo.c_dict[k7]) for k7 in Foo.c_dict] # [consider-using-dict-items]
-val = any(True for k8 in Foo.c_dict if Foo.c_dict[k8]) # [consider-using-dict-items]
+val = [(k7, Foo.c_dict[k7]) for k7 in Foo.c_dict]  # [consider-using-dict-items]
+val = any(True for k8 in Foo.c_dict if Foo.c_dict[k8])  # [consider-using-dict-items]
 
 # Should emit warning when dict access done in ``if`` portion of comprehension
 val = any(True for k8 in b_dict if b_dict[k8])  # [consider-using-dict-items]
@@ -71,6 +70,6 @@ class Foo:
 val = any(True for k8 in Foo.c_dict if b_dict[k8])
 
 # Should NOT emit warning, essentially same check as above
-c_dict={}
+c_dict = {}
 val = [(k7, c_dict[k7]) for k7 in Foo.c_dict]
 val = any(True for k8 in Foo.c_dict if c_dict[k8])
diff --git a/tests/functional/c/consider/consider_using_dict_items.txt b/tests/functional/c/consider/consider_using_dict_items.txt
index fedd8a8ecd..d5fd1f821d 100644
--- a/tests/functional/c/consider/consider_using_dict_items.txt
+++ b/tests/functional/c/consider/consider_using_dict_items.txt
@@ -1,13 +1,13 @@
-consider-using-dict-items:7:4:bad:Consider iterating with .items()
-consider-using-dict-items:10:4:bad:Consider iterating with .items()
-consider-using-dict-items:22:4:another_bad:Consider iterating with .items()
-consider-using-dict-items:41:0::Consider iterating with .items()
-consider-using-dict-items:45:0::Consider iterating with .items()
-consider-iterating-dictionary:48:10::Consider iterating the dictionary directly instead of calling .keys()
-consider-using-dict-items:48:0::Consider iterating with .items()
-consider-using-dict-items:55:0::Consider iterating with .items()
+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:58:0::Consider iterating with .items()
 consider-using-dict-items:59:0::Consider iterating with .items()
-consider-using-dict-items:60:0::Consider iterating with .items()
+consider-using-dict-items:62:0::Consider iterating with .items()
 consider-using-dict-items:63:0::Consider iterating with .items()
-consider-using-dict-items:64:0::Consider iterating with .items()
-consider-using-dict-items:67:0::Consider iterating with .items()
+consider-using-dict-items:66:0::Consider iterating with .items()

From e8161a5148d16f7cdda8b97568b36bfe65cab57e Mon Sep 17 00:00:00 2001
From: Pang Yu Shao <p.yushao2@gmail.com>
Date: Tue, 11 May 2021 19:53:02 +0800
Subject: [PATCH 08/13] Added more test cases to check for accessing other
 dicts

---
 .../functional/c/consider/consider_using_dict_items.py | 10 +++++++++-
 .../c/consider/consider_using_dict_items.txt           | 10 +++++-----
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/tests/functional/c/consider/consider_using_dict_items.py b/tests/functional/c/consider/consider_using_dict_items.py
index db3626e10a..f45d5eca41 100644
--- a/tests/functional/c/consider/consider_using_dict_items.py
+++ b/tests/functional/c/consider/consider_using_dict_items.py
@@ -54,6 +54,15 @@ class Foo:
 for k5 in Foo.c_dict:  # [consider-using-dict-items]
     val = Foo.c_dict[k5]
 
+c_dict = {}
+
+# Should NOT emit warning whey key used to access a different dict
+for k5 in Foo.c_dict:  # This is fine
+    val = b_dict[k5]
+
+for k5 in Foo.c_dict:  # This is fine
+    val = c_dict[k5]
+
 # Should emit warning within a list/dict comprehension
 val = {k9: b_dict[k9] for k9 in b_dict}  # [consider-using-dict-items]
 val = [(k7, b_dict[k7]) for k7 in b_dict]  # [consider-using-dict-items]
@@ -70,6 +79,5 @@ class Foo:
 val = any(True for k8 in Foo.c_dict if b_dict[k8])
 
 # Should NOT emit warning, essentially same check as above
-c_dict = {}
 val = [(k7, c_dict[k7]) for k7 in Foo.c_dict]
 val = any(True for k8 in Foo.c_dict if c_dict[k8])
diff --git a/tests/functional/c/consider/consider_using_dict_items.txt b/tests/functional/c/consider/consider_using_dict_items.txt
index d5fd1f821d..00451fb4d4 100644
--- a/tests/functional/c/consider/consider_using_dict_items.txt
+++ b/tests/functional/c/consider/consider_using_dict_items.txt
@@ -6,8 +6,8 @@ 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:58:0::Consider iterating with .items()
-consider-using-dict-items:59:0::Consider iterating with .items()
-consider-using-dict-items:62:0::Consider iterating with .items()
-consider-using-dict-items:63:0::Consider iterating with .items()
-consider-using-dict-items:66: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()

From f54b22d7a4db26c449d8c446d28f5c2723e77dd4 Mon Sep 17 00:00:00 2001
From: Pang Yu Shao <p.yushao2@gmail.com>
Date: Tue, 11 May 2021 19:59:55 +0800
Subject: [PATCH 09/13] Cleaned up logic based on PR review

---
 pylint/checkers/refactoring/recommendation_checker.py |  4 ----
 pylint/checkers/utils.py                              | 11 +++++------
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py
index 2be19e4f5e..920842890e 100644
--- a/pylint/checkers/refactoring/recommendation_checker.py
+++ b/pylint/checkers/refactoring/recommendation_checker.py
@@ -155,8 +155,6 @@ def _check_consider_using_dict_items(self, node: astroid.For) -> None:
                     continue
 
                 value = subscript.slice
-                if isinstance(value, astroid.Index):
-                    value = value.value
                 if (
                     not isinstance(value, astroid.Name)
                     or value.name != node.target.name
@@ -198,8 +196,6 @@ def visit_comprehension(self, node: astroid.Comprehension) -> None:
                     continue
 
                 value = subscript.slice
-                if isinstance(value, astroid.Index):
-                    value = value.value
                 if (
                     not isinstance(value, astroid.Name)
                     or value.name != node.target.name
diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py
index 64fa300ae8..acaff104ac 100644
--- a/pylint/checkers/utils.py
+++ b/pylint/checkers/utils.py
@@ -1504,12 +1504,11 @@ def get_iterating_dictionary_name(
     or a dictionary itself, this returns None.
     """
     # Is it a proper keys call?
-    if isinstance(node.iter, astroid.Call):
-        if (
-            isinstance(node.iter.func, astroid.Name)
-            or node.iter.func.attrname != "keys"
-        ):
-            return None
+    if (
+        isinstance(node.iter, astroid.Call)
+        and isinstance(node.iter.func, astroid.Attribute)
+        and node.iter.func.attrname == "keys"
+    ):
         inferred = safe_infer(node.iter.func)
         if not isinstance(inferred, (astroid.BoundMethod, astroid.Dict)):
             return None

From b1fb77b8bec724a0a3b6b03996753b256ca4d660 Mon Sep 17 00:00:00 2001
From: Pang Yu Shao <p.yushao2@gmail.com>
Date: Tue, 11 May 2021 21:58:18 +0800
Subject: [PATCH 10/13] Added test case for .keys() within comprehension

---
 tests/functional/c/consider/consider_using_dict_items.py  | 5 ++++-
 tests/functional/c/consider/consider_using_dict_items.txt | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/functional/c/consider/consider_using_dict_items.py b/tests/functional/c/consider/consider_using_dict_items.py
index f45d5eca41..d0af4fe0ac 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=missing-docstring,unsubscriptable-object,too-few-public-methods
+# pylint: disable=line-too-long,missing-docstring,unsubscriptable-object,too-few-public-methods
 
 def bad():
     a_dict = {1: 1, 2: 2, 3: 3}
@@ -81,3 +81,6 @@ class Foo:
 # Should NOT emit warning, essentially same check as above
 val = [(k7, c_dict[k7]) for k7 in Foo.c_dict]
 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]
diff --git a/tests/functional/c/consider/consider_using_dict_items.txt b/tests/functional/c/consider/consider_using_dict_items.txt
index 00451fb4d4..8ead82351b 100644
--- a/tests/functional/c/consider/consider_using_dict_items.txt
+++ b/tests/functional/c/consider/consider_using_dict_items.txt
@@ -11,3 +11,5 @@ 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()

From c79d112a96fa00c19e34024c713491df00999648 Mon Sep 17 00:00:00 2001
From: Pang Yu Shao <p.yushao2@gmail.com>
Date: Tue, 11 May 2021 22:08:12 +0800
Subject: [PATCH 11/13] Revised codes based on review

---
 .../refactoring/recommendation_checker.py     |  7 +++---
 pylint/checkers/utils.py                      | 24 +++++++++----------
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py
index 920842890e..fba4ab8b11 100644
--- a/pylint/checkers/refactoring/recommendation_checker.py
+++ b/pylint/checkers/refactoring/recommendation_checker.py
@@ -153,8 +153,9 @@ def _check_consider_using_dict_items(self, node: astroid.For) -> None:
 
                 if not isinstance(subscript.value, (astroid.Name, astroid.Attribute)):
                     continue
-
                 value = subscript.slice
+                if isinstance(value, astroid.Index):
+                    value = value.value
                 if (
                     not isinstance(value, astroid.Name)
                     or value.name != node.target.name
@@ -185,7 +186,6 @@ def visit_comprehension(self, node: astroid.Comprehension) -> None:
         iterating_object_name = utils.get_iterating_dictionary_name(node)
         if iterating_object_name is None:
             return
-
         children = list(node.parent.get_children())
         if node.ifs:
             children.extend(node.ifs)
@@ -194,8 +194,9 @@ def visit_comprehension(self, node: astroid.Comprehension) -> None:
                 subscript = cast(astroid.Subscript, subscript)
                 if not isinstance(subscript.value, (astroid.Name, astroid.Attribute)):
                     continue
-
                 value = subscript.slice
+                if isinstance(value, astroid.Index):
+                    value = value.value
                 if (
                     not isinstance(value, astroid.Name)
                     or value.name != node.target.name
diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py
index acaff104ac..470a1e68b9 100644
--- a/pylint/checkers/utils.py
+++ b/pylint/checkers/utils.py
@@ -1504,20 +1504,18 @@ def get_iterating_dictionary_name(
     or a dictionary itself, this returns None.
     """
     # Is it a proper keys call?
-    if (
+    is_iterating_dict_keys = (
         isinstance(node.iter, astroid.Call)
         and isinstance(node.iter.func, astroid.Attribute)
         and node.iter.func.attrname == "keys"
-    ):
-        inferred = safe_infer(node.iter.func)
-        if not isinstance(inferred, (astroid.BoundMethod, astroid.Dict)):
-            return None
-        return node.iter.as_string().partition(".")[0]
+        and isinstance(safe_infer(node.iter.func), astroid.BoundMethod)
+    )
+    # Is it a proper dictionary?
+    is_iterating_dict = isinstance(
+        node.iter, (astroid.Name, astroid.Attribute)
+    ) and isinstance(safe_infer(node.iter), astroid.Dict)
 
-    # Is it a dictionary?
-    if not isinstance(node.iter, (astroid.Name, astroid.Attribute)):
-        return None
-    inferred = safe_infer(node.iter)
-    if not isinstance(inferred, (astroid.Dict, astroid.DictComp)):
-        return None
-    return node.iter.as_string()
+    if is_iterating_dict or is_iterating_dict_keys:
+        return node.iter.as_string().partition(".keys")[0]
+
+    return None

From 797b050525a8fe4ed615b15505733da6e781a199 Mon Sep 17 00:00:00 2001
From: Pang Yu Shao <p.yushao2@gmail.com>
Date: Tue, 11 May 2021 23:33:53 +0800
Subject: [PATCH 12/13] Improved readability

---
 .../refactoring/recommendation_checker.py     |  4 ++++
 pylint/checkers/utils.py                      | 21 +++++++++++--------
 .../c/consider/consider_using_dict_items.py   |  2 +-
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py
index fba4ab8b11..2ff58b8fe8 100644
--- a/pylint/checkers/refactoring/recommendation_checker.py
+++ b/pylint/checkers/refactoring/recommendation_checker.py
@@ -153,6 +153,7 @@ def _check_consider_using_dict_items(self, node: astroid.For) -> None:
 
                 if not isinstance(subscript.value, (astroid.Name, astroid.Attribute)):
                     continue
+
                 value = subscript.slice
                 if isinstance(value, astroid.Index):
                     value = value.value
@@ -186,14 +187,17 @@ def visit_comprehension(self, node: astroid.Comprehension) -> None:
         iterating_object_name = utils.get_iterating_dictionary_name(node)
         if iterating_object_name is None:
             return
+
         children = list(node.parent.get_children())
         if node.ifs:
             children.extend(node.ifs)
         for child in children:
             for subscript in child.nodes_of_class(astroid.Subscript):
                 subscript = cast(astroid.Subscript, subscript)
+
                 if not isinstance(subscript.value, (astroid.Name, astroid.Attribute)):
                     continue
+
                 value = subscript.slice
                 if isinstance(value, astroid.Index):
                     value = value.value
diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py
index 470a1e68b9..8cf97d8074 100644
--- a/pylint/checkers/utils.py
+++ b/pylint/checkers/utils.py
@@ -1504,18 +1504,21 @@ def get_iterating_dictionary_name(
     or a dictionary itself, this returns None.
     """
     # Is it a proper keys call?
-    is_iterating_dict_keys = (
+    if (
         isinstance(node.iter, astroid.Call)
         and isinstance(node.iter.func, astroid.Attribute)
         and node.iter.func.attrname == "keys"
-        and isinstance(safe_infer(node.iter.func), astroid.BoundMethod)
-    )
-    # Is it a proper dictionary?
-    is_iterating_dict = isinstance(
-        node.iter, (astroid.Name, astroid.Attribute)
-    ) and isinstance(safe_infer(node.iter), astroid.Dict)
+    ):
+        inferred = safe_infer(node.iter.func)
+        if not isinstance(inferred, astroid.BoundMethod):
+            return None
+        return node.iter.as_string().rpartition(".keys")[0]
 
-    if is_iterating_dict or is_iterating_dict_keys:
-        return node.iter.as_string().partition(".keys")[0]
+    # Is it a dictionary?
+    if isinstance(node.iter, (astroid.Name, astroid.Attribute)):
+        inferred = safe_infer(node.iter)
+        if not isinstance(inferred, astroid.Dict):
+            return None
+        return node.iter.as_string()
 
     return None
diff --git a/tests/functional/c/consider/consider_using_dict_items.py b/tests/functional/c/consider/consider_using_dict_items.py
index d0af4fe0ac..1d6c86741b 100644
--- a/tests/functional/c/consider/consider_using_dict_items.py
+++ b/tests/functional/c/consider/consider_using_dict_items.py
@@ -83,4 +83,4 @@ class Foo:
 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]
+val = any(True for k8 in Foo.c_dict.keys() if Foo.c_dict[k8])  # [consider-iterating-dictionary,consider-using-dict-items]

From d9ccab139233faac37361026bd8dbc1316523430 Mon Sep 17 00:00:00 2001
From: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date: Tue, 11 May 2021 21:46:18 +0200
Subject: [PATCH 13/13] Last small style changes

---
 pylint/checkers/refactoring/recommendation_checker.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py
index 2ff58b8fe8..6efc4190c4 100644
--- a/pylint/checkers/refactoring/recommendation_checker.py
+++ b/pylint/checkers/refactoring/recommendation_checker.py
@@ -171,9 +171,9 @@ def _check_consider_using_dict_items(self, node: astroid.For) -> None:
                     # defined and compare that to the for loop's line number
                     continue
                 if (
-                    isinstance(subscript.parent, (astroid.Assign))
+                    isinstance(subscript.parent, astroid.Assign)
                     and subscript in subscript.parent.targets
-                    or isinstance(subscript.parent, (astroid.AugAssign))
+                    or isinstance(subscript.parent, astroid.AugAssign)
                     and subscript == subscript.parent.target
                 ):
                     # Ignore this subscript if it is the target of an assignment
@@ -207,5 +207,6 @@ def visit_comprehension(self, node: astroid.Comprehension) -> None:
                     or iterating_object_name != subscript.value.as_string()
                 ):
                     continue
+
                 self.add_message("consider-using-dict-items", node=node)
                 return