Skip to content

Commit

Permalink
Make consider-iterating-dictionary consider memb. check
Browse files Browse the repository at this point in the history
This closes pylint-dev#4069
  • Loading branch information
DanielNoord committed Sep 13, 2021
1 parent 9ec8cb9 commit 400f8d7
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 20 deletions.
4 changes: 4 additions & 0 deletions ChangeLog
Expand Up @@ -93,6 +93,10 @@ Release date: TBA

Closes #4616

* The ``consider-iterating-dictionary`` checker now also considers membership checks

Closes #4069


What's New in Pylint 2.10.3?
============================
Expand Down
4 changes: 4 additions & 0 deletions doc/whatsnew/2.11.rst
Expand Up @@ -85,3 +85,7 @@ Other Changes

Closes #1375
Closes #330

* The ``consider-iterating-dictionary`` checker now also considers membership checks

Closes #4069
13 changes: 9 additions & 4 deletions pylint/checkers/refactoring/recommendation_checker.py
Expand Up @@ -26,7 +26,7 @@ class RecommendationChecker(checkers.BaseChecker):
"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".',
'in "for key in dictionary" or "if key in dictionary".',
),
"C0206": (
"Consider iterating with .items()",
Expand Down Expand Up @@ -75,7 +75,13 @@ 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)):
if not (
isinstance(node.parent, (nodes.For, nodes.Comprehension))
or (
isinstance(node.parent, nodes.Compare)
and isinstance(node.parent.parent, nodes.If)
)
):
return

inferred = utils.safe_infer(node.func)
Expand All @@ -84,8 +90,7 @@ def _check_consider_iterating_dictionary(self, node: nodes.Call) -> None:
):
return

if isinstance(node.parent, (nodes.For, nodes.Comprehension)):
self.add_message("consider-iterating-dictionary", node=node)
self.add_message("consider-iterating-dictionary", node=node)

def _check_use_maxsplit_arg(self, node: nodes.Call) -> None:
"""Add message when accessing first or last elements of a str.split() or str.rsplit()."""
Expand Down
20 changes: 19 additions & 1 deletion tests/functional/c/consider/consider_iterating_dictionary.py
@@ -1,4 +1,4 @@
# 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, no-member, import-error, no-self-use, line-too-long, useless-object-inheritance, unnecessary-comprehension, use-dict-literal

from unknown import Unknown

Expand Down Expand Up @@ -36,3 +36,21 @@ 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
32 changes: 17 additions & 15 deletions tests/functional/c/consider/consider_iterating_dictionary.txt
@@ -1,15 +1,17 @@
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:23:16::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:24:16::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:25:16::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:26:21::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:27:24::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:28:24::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:29::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:31:11::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:36:24::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:36:55::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:37:31::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:37:61::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:38:30::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:38:60::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:41:8::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

0 comments on commit 400f8d7

Please sign in to comment.