From 4f1d47f018a4f20033b93ece6360dc3b817e8c29 Mon Sep 17 00:00:00 2001 From: Pang Yu Shao Date: Thu, 6 May 2021 18:47:26 +0800 Subject: [PATCH] Implemented new check `consider-using-dict-items` (#3389) --- .../refactoring/recommendation_checker.py | 71 ++++++++++++++++++- .../c/consider/consider_using_dict_items.py | 17 +++++ .../c/consider/consider_using_dict_items.txt | 2 + 3 files changed, 89 insertions(+), 1 deletion(-) 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/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py index b1175f03e03..0bcd2cbf153 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/tests/functional/c/consider/consider_using_dict_items.py b/tests/functional/c/consider/consider_using_dict_items.py new file mode 100644 index 00000000000..9b067728341 --- /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 00000000000..46662e5ce93 --- /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()