Skip to content

Commit

Permalink
Fix false negative for consider-using-augmented-assign on dicts
Browse files Browse the repository at this point in the history
Assignments on dictionaries which used the same subscripts were not
generating `consider-using-augmented-assign` messages. This has been
fixed.

Closes pylint-dev#8959.
  • Loading branch information
crazybolillo committed Aug 18, 2023
1 parent fd80514 commit 5d3355b
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 2 deletions.
3 changes: 3 additions & 0 deletions doc/whatsnew/fragments/8959.false_negative
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
``consider-using-augmented-assign`` is now applied to dictionaries as well.

Closes #8959.
25 changes: 24 additions & 1 deletion pylint/checkers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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


Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 5d3355b

Please sign in to comment.