From 5d3355b992d40dfd3f580cae1c633ac091e44b72 Mon Sep 17 00:00:00 2001 From: CrazyBolillo Date: Fri, 18 Aug 2023 13:51:06 -0600 Subject: [PATCH] Fix false negative for `consider-using-augmented-assign` on dicts Assignments on dictionaries which used the same subscripts were not generating `consider-using-augmented-assign` messages. This has been fixed. Closes #8959. --- doc/whatsnew/fragments/8959.false_negative | 3 +++ pylint/checkers/utils.py | 25 ++++++++++++++++++- .../cs_consider_using_augmented_assign.py | 9 +++++++ .../cs_consider_using_augmented_assign.txt | 4 ++- 4 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 doc/whatsnew/fragments/8959.false_negative diff --git a/doc/whatsnew/fragments/8959.false_negative b/doc/whatsnew/fragments/8959.false_negative new file mode 100644 index 00000000000..f71280688d7 --- /dev/null +++ b/doc/whatsnew/fragments/8959.false_negative @@ -0,0 +1,3 @@ +``consider-using-augmented-assign`` is now applied to dictionaries as well. + +Closes #8959. diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index edb0aa6c3b1..02462c24d44 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -2041,6 +2041,26 @@ def is_hashable(node: nodes.NodeNG) -> bool: return True +def subscript_chain_is_equal(left: nodes.Subscript, right: nodes.Subscript) -> bool: + while isinstance(left, nodes.Subscript) and isinstance(right, nodes.Subscript): + try: + if ( + get_subscript_const_value(left).value + != get_subscript_const_value(right).value + ): + return False + + left = left.value + right = right.value + except (AttributeError, InferredTypeError): + return False + + try: + return left.name == right.name # type: ignore[no-any-return] + except AttributeError: + return False + + def _is_target_name_in_binop_side( target: nodes.AssignName | nodes.AssignAttr, side: nodes.NodeNG | None ) -> bool: @@ -2051,6 +2071,9 @@ def _is_target_name_in_binop_side( return False if isinstance(side, nodes.Attribute) and isinstance(target, nodes.AssignAttr): return target.as_string() == side.as_string() # type: ignore[no-any-return] + if isinstance(side, nodes.Subscript) and isinstance(target, nodes.Subscript): + return subscript_chain_is_equal(side, target) + return False @@ -2065,7 +2088,7 @@ def is_augmented_assign(node: nodes.Assign) -> tuple[bool, str]: binop = node.value target = node.targets[0] - if not isinstance(target, (nodes.AssignName, nodes.AssignAttr)): + if not isinstance(target, (nodes.AssignName, nodes.AssignAttr, nodes.Subscript)): return False, "" # We don't want to catch x = "1" + x or x = "%s" % x diff --git a/tests/functional/ext/code_style/cs_consider_using_augmented_assign.py b/tests/functional/ext/code_style/cs_consider_using_augmented_assign.py index ab3acfe278c..b068fb89889 100644 --- a/tests/functional/ext/code_style/cs_consider_using_augmented_assign.py +++ b/tests/functional/ext/code_style/cs_consider_using_augmented_assign.py @@ -120,6 +120,15 @@ def return_str() -> str: x = x <= 3 x = 3 <= x +# Should also apply to dictionaries when subscripts are the same +my_dict = {"count": 5} +my_dict["count"] = my_dict["count"] + 2 # [consider-using-augmented-assign] +my_dict["apples"] = my_dict["count"] + 1 + +my_dict = {"msg": {"title": "Hello"}} +my_dict["msg"]["title"] = my_dict["msg"]["title"] + " world!" # [consider-using-augmented-assign] +my_dict["msg"]["body"] = my_dict["msg"]["title"] + " everyone, this should not raise messages" + # https://github.com/pylint-dev/pylint/issues/8086 # consider-using-augmented-assign should only be flagged diff --git a/tests/functional/ext/code_style/cs_consider_using_augmented_assign.txt b/tests/functional/ext/code_style/cs_consider_using_augmented_assign.txt index f820eb67bfe..329c53e7fc5 100644 --- a/tests/functional/ext/code_style/cs_consider_using_augmented_assign.txt +++ b/tests/functional/ext/code_style/cs_consider_using_augmented_assign.txt @@ -24,4 +24,6 @@ consider-using-augmented-assign:104:0:104:9::Use '&=' to do an augmented assign consider-using-augmented-assign:105:0:105:9::Use '&=' to do an augmented assign directly:INFERENCE consider-using-augmented-assign:108:0:108:9::Use '|=' to do an augmented assign directly:INFERENCE consider-using-augmented-assign:109:0:109:9::Use '|=' to do an augmented assign directly:INFERENCE -consider-using-augmented-assign:134:8:134:27:A.test:Use '+=' to do an augmented assign directly:INFERENCE +consider-using-augmented-assign:125:0:125:39::Use '+=' to do an augmented assign directly:INFERENCE +consider-using-augmented-assign:129:0:129:61::Use '+=' to do an augmented assign directly:INFERENCE +consider-using-augmented-assign:143:8:143:27:A.test:Use '+=' to do an augmented assign directly:INFERENCE