diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py index bc706807903..2be19e4f5e9 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 8aae906433c..64fa300ae8f 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 65e11a4c2db..db3626e10a6 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 fedd8a8ecd3..d5fd1f821d6 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()