Skip to content

Commit

Permalink
fix(consider-interating-dictionary): fix false negatives
Browse files Browse the repository at this point in the history
  • Loading branch information
alt-yushao2 committed Nov 17, 2021
1 parent b91cc8d commit 155ce7c
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 13 deletions.
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ Release date: TBA
..
Put new features here and also in 'doc/whatsnew/2.12.rst'

* Fix false negative ``consider-iterating-dictionary`` during membership checks

Closes #5323

* Fix ``install graphiz`` message which isn't needed for puml output format.

* Fix ``simplify-boolean-expression`` when condition can be inferred as False.
Expand Down
4 changes: 4 additions & 0 deletions doc/whatsnew/2.12.rst
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ Extensions
Other Changes
=============

* Fix false negative ``consider-iterating-dictionary`` during membership checks

Closes #5323

* Fix ``install graphiz`` message which isn't needed for puml output format.

* ``pylint`` no longer crashes when checking assignment expressions within if-statements
Expand Down
28 changes: 15 additions & 13 deletions pylint/checkers/refactoring/recommendation_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,23 @@ def _check_consider_iterating_dictionary(self, node: nodes.Call) -> None:
return
if node.func.attrname != "keys":
return
if (
isinstance(node.parent, (nodes.For, nodes.Comprehension))
or isinstance(node.parent, nodes.Compare)
and any(
op
for op, comparator in node.parent.ops
if op == "in" and comparator is node
)
inferred = utils.safe_infer(node.func)
if not isinstance(inferred, astroid.BoundMethod) or not isinstance(
inferred.bound, nodes.Dict
):
inferred = utils.safe_infer(node.func)
if not isinstance(inferred, astroid.BoundMethod) or not isinstance(
inferred.bound, nodes.Dict
):
return
return

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

while not isinstance(node.parent, (nodes.Compare, nodes.Module)):
node = node.parent

if isinstance(node.parent, nodes.Compare) and any(
op
for op, comparator in node.parent.ops
if op in ["in", "not in"] and comparator is node
):
self.add_message("consider-iterating-dictionary", node=node)

def _check_use_maxsplit_arg(self, node: nodes.Call) -> None:
Expand Down
12 changes: 12 additions & 0 deletions tests/functional/c/consider/consider_iterating_dictionary.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,15 @@ def keys(self):
VAR = 1 in {}
VAR = 1 in dict()
VAR = [1, 2] == {}.keys() in {False}

# Additional membership checks
# Issue #5323
metadata = {}
if "a" not in list(metadata.keys()): # [consider-iterating-dictionary]
print(1)
if "a" not in metadata.keys(): # [consider-iterating-dictionary]
print(1)
if "a" in list(metadata.keys()): # [consider-iterating-dictionary]
print(1)
if "a" in metadata.keys(): # [consider-iterating-dictionary]
print(1)
4 changes: 4 additions & 0 deletions tests/functional/c/consider/consider_iterating_dictionary.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,7 @@ consider-iterating-dictionary:40:60::Consider iterating the dictionary directly
consider-iterating-dictionary:43:8::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:45:8::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:65:11::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:73:14::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:75:14::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:77:10::Consider iterating the dictionary directly instead of calling .keys():HIGH
consider-iterating-dictionary:79:10::Consider iterating the dictionary directly instead of calling .keys():HIGH

0 comments on commit 155ce7c

Please sign in to comment.