From ba92c33826d766af5df6b0bf00ea4010d8a67bda Mon Sep 17 00:00:00 2001 From: "Yu Shao, Pang" Date: Wed, 30 Jun 2021 21:41:23 +0800 Subject: fix false positive of `unnecessary-dict-index-lookup` --- pylint/checkers/refactoring/refactoring_checker.py | 9 ++++- .../u/unnecessary/unnecessary_dict_index_lookup.py | 23 ++++++++++- .../unnecessary/unnecessary_dict_index_lookup.txt | 46 +++++++++++----------- 3 files changed, 52 insertions(+), 26 deletions(-) diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index e771db750..c17545ded 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -1689,7 +1689,14 @@ class RefactoringChecker(checkers.BaseTokenChecker): and subscript == subscript.parent.target ): # Ignore this subscript if it is the target of an assignment - continue + if subscript.as_string() == subscript.parent.value.as_string(): + # Fire error as d[k] += d[k] has an unnecessary index lookup + self.add_message( + "unnecessary-dict-index-lookup", + node=subscript, + args=(node.target.elts[1].as_string()), + ) + return # Early termination; after reassignment dict index lookup will be necessary # Case where .items is assigned to k,v (i.e., for k, v in d.items()) if isinstance(value, astroid.Name): diff --git a/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.py b/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.py index 901e176b3..1e56b046b 100644 --- a/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.py +++ b/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.py @@ -8,9 +8,11 @@ for k, v in a_dict.items(): print(b_dict[k]) # Should not emit warning, accessing other dictionary a_dict[k] = 123 # Should not emit warning, key access necessary a_dict[k] += 123 # Should not emit warning, key access necessary - print(a_dict[k]) # [unnecessary-dict-index-lookup] + print(a_dict[k]) # Should not emit warning, v != a_dict[k] + +for k, v in b_dict.items(): k = "another key" - print(a_dict[k]) # This is fine, key reassigned + print(b_dict[k]) # This is fine, key reassigned # Tests on comprehensions @@ -61,3 +63,20 @@ for item in d.items(): print(d[item[0]]) # [unnecessary-dict-index-lookup] item = (2, "b") print(d[item[0]]) # This is fine, no warning thrown as key has been reassigned + + +# Test false positive described in #4630 +# (https://github.com/PyCQA/pylint/issues/4630) + +d = {'key': 'value'} + +for k, _ in d.items(): + d[k] += 'VALUE' + if 'V' in d[k]: # This is fine, if d[k] is replaced with _, the semantics change + print('found V') + + +for k, _ in d.items(): + if 'V' in d[k]: # [unnecessary-dict-index-lookup] + d[k] = "value" + print(d[k]) # This is fine diff --git a/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.txt b/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.txt index dedf9e7cc..c0d5d6d16 100644 --- a/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.txt +++ b/tests/functional/u/unnecessary/unnecessary_dict_index_lookup.txt @@ -1,23 +1,23 @@ -unnecessary-dict-index-lookup:7:10::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:11:10::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:17:36::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:19:1::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:20:1::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:20:44::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:22:33::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:24:1::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:25:1::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:25:41::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:34:10::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:35:21::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:39:40::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:41:1::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:42:1::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:42:52::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:44:37::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:46:1::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:47:1::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:47:49::Unnecessary dictionary index lookup, use 'v' instead -unnecessary-dict-index-lookup:53:10::Unnecessary dictionary index lookup, use 'item[1]' instead -unnecessary-dict-index-lookup:56:1::Unnecessary dictionary index lookup, use 'item[1]' instead -unnecessary-dict-index-lookup:61:10::Unnecessary dictionary index lookup, use 'item[1]' instead +unnecessary-dict-index-lookup:7:10::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:19:36::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:21:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:22:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:22:44::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:24:33::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:26:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:27:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:27:41::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:36:10::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:37:4::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:41:40::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:43:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:44:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:44:52::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:46:37::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:48:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:49:1::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:49:49::Unnecessary dictionary index lookup, use 'v' instead:HIGH +unnecessary-dict-index-lookup:55:10::Unnecessary dictionary index lookup, use 'item[1]' instead:HIGH +unnecessary-dict-index-lookup:58:1::Unnecessary dictionary index lookup, use 'item[1]' instead:HIGH +unnecessary-dict-index-lookup:63:10::Unnecessary dictionary index lookup, use 'item[1]' instead:HIGH +unnecessary-dict-index-lookup:80:14::Unnecessary dictionary index lookup, use '_' instead:HIGH -- cgit v1.2.1