Skip to content

Commit 06926b6

Browse files
committed
Fix false negative for consider-using-augmented-assign on dicts/lists
Assignments on dictionaries or lists which used the same subscripts were not generating `consider-using-augmented-assign` messages. This has been fixed. Closes #8959.
1 parent a0ce6e4 commit 06926b6

File tree

4 files changed

+50
-3
lines changed

4 files changed

+50
-3
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
``consider-using-augmented-assign`` is now applied to dicts and lists as well.
2+
3+
Closes #8959.

pylint/checkers/utils.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2041,6 +2041,23 @@ def is_hashable(node: nodes.NodeNG) -> bool:
20412041
return True
20422042

20432043

2044+
def subscript_chain_is_equal(left: nodes.Subscript, right: nodes.Subscript) -> bool:
2045+
while isinstance(left, nodes.Subscript) and isinstance(right, nodes.Subscript):
2046+
try:
2047+
if (
2048+
get_subscript_const_value(left).value
2049+
!= get_subscript_const_value(right).value
2050+
):
2051+
return False
2052+
2053+
left = left.value
2054+
right = right.value
2055+
except InferredTypeError:
2056+
return False
2057+
2058+
return left.as_string() == right.as_string() # type: ignore[no-any-return]
2059+
2060+
20442061
def _is_target_name_in_binop_side(
20452062
target: nodes.AssignName | nodes.AssignAttr, side: nodes.NodeNG | None
20462063
) -> bool:
@@ -2051,6 +2068,9 @@ def _is_target_name_in_binop_side(
20512068
return False
20522069
if isinstance(side, nodes.Attribute) and isinstance(target, nodes.AssignAttr):
20532070
return target.as_string() == side.as_string() # type: ignore[no-any-return]
2071+
if isinstance(side, nodes.Subscript) and isinstance(target, nodes.Subscript):
2072+
return subscript_chain_is_equal(target, side)
2073+
20542074
return False
20552075

20562076

@@ -2065,7 +2085,7 @@ def is_augmented_assign(node: nodes.Assign) -> tuple[bool, str]:
20652085
binop = node.value
20662086
target = node.targets[0]
20672087

2068-
if not isinstance(target, (nodes.AssignName, nodes.AssignAttr)):
2088+
if not isinstance(target, (nodes.AssignName, nodes.AssignAttr, nodes.Subscript)):
20692089
return False, ""
20702090

20712091
# We don't want to catch x = "1" + x or x = "%s" % x

tests/functional/ext/code_style/cs_consider_using_augmented_assign.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

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

5-
from unknown import Unknown
5+
from unknown import Unknown, elixir, random_object
66

77
x = 1
88

@@ -120,6 +120,22 @@ def return_str() -> str:
120120
x = x <= 3
121121
x = 3 <= x
122122

123+
# Should also apply to dictionaries when subscripts are the same
124+
my_dict = {"count": 5}
125+
my_dict["count"] = my_dict["count"] + 2 # [consider-using-augmented-assign]
126+
my_dict["apples"] = my_dict["count"] + 1
127+
128+
my_dict = {"msg": {"title": "Hello"}}
129+
my_dict["msg"]["title"] = my_dict["msg"]["title"] + " world!" # [consider-using-augmented-assign]
130+
my_dict["msg"]["body"] = my_dict["msg"]["title"] + " everyone, this should not raise messages"
131+
132+
# Applies to lists as well
133+
test_list = [1, 2]
134+
test_list[0] = test_list[0] - 1 # [consider-using-augmented-assign]
135+
test_list[1] = test_list[0] + 10
136+
137+
# Can't infer, don't mark message
138+
random_object[elixir] = random_object[elixir] + 1
123139

124140
# https://github.com/pylint-dev/pylint/issues/8086
125141
# consider-using-augmented-assign should only be flagged
@@ -129,7 +145,11 @@ class A:
129145
def __init__(self) -> None:
130146
self.a = 1
131147
self.b = A()
148+
self.c = [1, 2, 3]
132149

133150
def test(self) -> None:
134151
self.a = self.a + 1 # [consider-using-augmented-assign]
135152
self.b.a = self.a + 1 # Names don't match!
153+
154+
def line(self) -> None:
155+
self.c[1] = self.c[1] + 1 # [consider-using-augmented-assign]

tests/functional/ext/code_style/cs_consider_using_augmented_assign.txt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,8 @@ consider-using-augmented-assign:104:0:104:9::Use '&=' to do an augmented assign
2424
consider-using-augmented-assign:105:0:105:9::Use '&=' to do an augmented assign directly:INFERENCE
2525
consider-using-augmented-assign:108:0:108:9::Use '|=' to do an augmented assign directly:INFERENCE
2626
consider-using-augmented-assign:109:0:109:9::Use '|=' to do an augmented assign directly:INFERENCE
27-
consider-using-augmented-assign:134:8:134:27:A.test:Use '+=' to do an augmented assign directly:INFERENCE
27+
consider-using-augmented-assign:125:0:125:39::Use '+=' to do an augmented assign directly:INFERENCE
28+
consider-using-augmented-assign:129:0:129:61::Use '+=' to do an augmented assign directly:INFERENCE
29+
consider-using-augmented-assign:134:0:134:31::Use '-=' to do an augmented assign directly:INFERENCE
30+
consider-using-augmented-assign:151:8:151:27:A.test:Use '+=' to do an augmented assign directly:INFERENCE
31+
consider-using-augmented-assign:155:8:155:33:A.line:Use '+=' to do an augmented assign directly:INFERENCE

0 commit comments

Comments
 (0)