From 86970f027c1b391556632dc99865ad7df9350a28 Mon Sep 17 00:00:00 2001 From: "Yu Shao, Pang" Date: Wed, 30 Jun 2021 22:02:34 +0800 Subject: fix false positive of `consider-using-dict-items` --- ChangeLog | 5 ++++ .../checkers/refactoring/recommendation_checker.py | 2 +- .../c/consider/consider_using_dict_items.py | 21 ++++++++++++++- .../c/consider/consider_using_dict_items.txt | 31 +++++++++++----------- 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/ChangeLog b/ChangeLog index ef043efa1..3482e61fe 100644 --- a/ChangeLog +++ b/ChangeLog @@ -27,6 +27,11 @@ Release date: TBA Closes #4644 +* Fix false-positive of ``unnecessary-dict-index-lookup`` and ``consider-using-dict-items`` + for reassigned dict index lookups + + Closes #4630 + What's New in Pylint 2.9.1? =========================== diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py index 6ea387126..d7d48e031 100644 --- a/pylint/checkers/refactoring/recommendation_checker.py +++ b/pylint/checkers/refactoring/recommendation_checker.py @@ -233,7 +233,7 @@ class RecommendationChecker(checkers.BaseChecker): and subscript == subscript.parent.target ): # Ignore this subscript if it is the target of an assignment - continue + return # Early termination as dict index lookup is necessary 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 index 1d6c86741..b8d230ca5 100644 --- a/tests/functional/c/consider/consider_using_dict_items.py +++ b/tests/functional/c/consider/consider_using_dict_items.py @@ -1,5 +1,5 @@ """Emit a message for iteration through dict keys and subscripting dict with key.""" -# pylint: disable=line-too-long,missing-docstring,unsubscriptable-object,too-few-public-methods +# pylint: disable=line-too-long,missing-docstring,unsubscriptable-object,too-few-public-methods,redefined-outer-name def bad(): a_dict = {1: 1, 2: 2, 3: 3} @@ -84,3 +84,22 @@ val = any(True for k8 in Foo.c_dict if c_dict[k8]) # Should emit warning, using .keys() of Foo.c_dict val = any(True for k8 in Foo.c_dict.keys() if Foo.c_dict[k8]) # [consider-iterating-dictionary,consider-using-dict-items] + +# Test false positive described in #4630 +# (https://github.com/PyCQA/pylint/issues/4630) + +d = {'key': 'value'} + +for k in d: # this is fine, with the reassignment of d[k], d[k] is necessary + d[k] += '123' + if '1' in d[k]: # index lookup necessary here, do not emit error + print('found 1') + +for k in d: # if this gets rewritten to d.items(), we are back to the above problem + d[k] = d[k] + 1 + if '1' in d[k]: # index lookup necessary here, do not emit error + print('found 1') + +for k in d: # [consider-using-dict-items] + if '1' in d[k]: # index lookup necessary here, do not emit error + print('found 1') diff --git a/tests/functional/c/consider/consider_using_dict_items.txt b/tests/functional/c/consider/consider_using_dict_items.txt index 8ead82351..9951d6848 100644 --- a/tests/functional/c/consider/consider_using_dict_items.txt +++ b/tests/functional/c/consider/consider_using_dict_items.txt @@ -1,15 +1,16 @@ -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:67:0::Consider iterating with .items() -consider-using-dict-items:68:0::Consider iterating with .items() -consider-using-dict-items:71:0::Consider iterating with .items() -consider-using-dict-items:72:0::Consider iterating with .items() -consider-using-dict-items:75:0::Consider iterating with .items() -consider-iterating-dictionary:86:25::Consider iterating the dictionary directly instead of calling .keys() -consider-using-dict-items:86:0::Consider iterating with .items() +consider-using-dict-items:6:4:bad:Consider iterating with .items():HIGH +consider-using-dict-items:9:4:bad:Consider iterating with .items():HIGH +consider-using-dict-items:21:4:another_bad:Consider iterating with .items():HIGH +consider-using-dict-items:40:0::Consider iterating with .items():HIGH +consider-using-dict-items:44:0::Consider iterating with .items():HIGH +consider-iterating-dictionary:47:10::Consider iterating the dictionary directly instead of calling .keys():HIGH +consider-using-dict-items:47:0::Consider iterating with .items():HIGH +consider-using-dict-items:54:0::Consider iterating with .items():HIGH +consider-using-dict-items:67:0::Consider iterating with .items():HIGH +consider-using-dict-items:68:0::Consider iterating with .items():HIGH +consider-using-dict-items:71:0::Consider iterating with .items():HIGH +consider-using-dict-items:72:0::Consider iterating with .items():HIGH +consider-using-dict-items:75:0::Consider iterating with .items():HIGH +consider-iterating-dictionary:86:25::Consider iterating the dictionary directly instead of calling .keys():HIGH +consider-using-dict-items:86:0::Consider iterating with .items():HIGH +consider-using-dict-items:103:0::Consider iterating with .items():HIGH -- cgit v1.2.1