Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make consider-iterating-dictionary consider memb. check #4997

Merged
merged 9 commits into from
Sep 14, 2021
Merged
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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() "
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
"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".',
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
),
"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)
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
)
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
):
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
Original file line number Diff line number Diff line change
@@ -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

Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
@@ -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
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
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