Skip to content

Commit

Permalink
Fix false negative for consider-using-augmented-assign on dicts/lists
Browse files Browse the repository at this point in the history
Assignments on dictionaries or lists 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 22, 2023
1 parent a6d9a02 commit a1eadda
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 3 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 dicts and lists as well.

Closes #8959.
22 changes: 21 additions & 1 deletion pylint/checkers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1998,6 +1998,23 @@ 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 InferredTypeError:
return False

return left.name == right.name # type: ignore[no-any-return]


def _is_target_name_in_binop_side(
target: nodes.AssignName | nodes.AssignAttr, side: nodes.NodeNG | None
) -> bool:
Expand All @@ -2008,6 +2025,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 @@ -2022,7 +2042,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 @@ -2,7 +2,7 @@

# pylint: disable=invalid-name,too-few-public-methods,import-error,consider-using-f-string,missing-docstring

from unknown import Unknown
from unknown import Unknown, elixir, random_object

x = 1

Expand Down Expand Up @@ -120,6 +120,22 @@ 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"

# Applies to lists as well
test_list = [1, 2]
test_list[0] = test_list[0] - 1 # [consider-using-augmented-assign]
test_list[1] = test_list[0] + 10

# Can't infer, don't mark message
random_object[elixir] = random_object[elixir] + 1

# 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,7 @@ 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:134:0:134:31::Use '-=' to do an augmented assign directly:INFERENCE
consider-using-augmented-assign:150:8:150:27:A.test:Use '+=' to do an augmented assign directly:INFERENCE

0 comments on commit a1eadda

Please sign in to comment.