Skip to content

Commit a5659dd

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 fd80514 commit a5659dd

File tree

4 files changed

+44
-2
lines changed

4 files changed

+44
-2
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: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2041,6 +2041,26 @@ 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+
try:
2059+
return left.name == right.name # type: ignore[no-any-return]
2060+
except AttributeError:
2061+
return False
2062+
2063+
20442064
def _is_target_name_in_binop_side(
20452065
target: nodes.AssignName | nodes.AssignAttr, side: nodes.NodeNG | None
20462066
) -> bool:
@@ -2051,6 +2071,9 @@ def _is_target_name_in_binop_side(
20512071
return False
20522072
if isinstance(side, nodes.Attribute) and isinstance(target, nodes.AssignAttr):
20532073
return target.as_string() == side.as_string() # type: ignore[no-any-return]
2074+
if isinstance(side, nodes.Subscript) and isinstance(target, nodes.Subscript):
2075+
return subscript_chain_is_equal(side, target)
2076+
20542077
return False
20552078

20562079

@@ -2065,7 +2088,7 @@ def is_augmented_assign(node: nodes.Assign) -> tuple[bool, str]:
20652088
binop = node.value
20662089
target = node.targets[0]
20672090

2068-
if not isinstance(target, (nodes.AssignName, nodes.AssignAttr)):
2091+
if not isinstance(target, (nodes.AssignName, nodes.AssignAttr, nodes.Subscript)):
20692092
return False, ""
20702093

20712094
# 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: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,19 @@ 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
123136

124137
# https://github.com/pylint-dev/pylint/issues/8086
125138
# consider-using-augmented-assign should only be flagged

tests/functional/ext/code_style/cs_consider_using_augmented_assign.txt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,7 @@ 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:147:8:147:27:A.test:Use '+=' to do an augmented assign directly:INFERENCE

0 commit comments

Comments
 (0)