Add consider-using-augmented-assign checker (#7514)

Co-authored-by: clavedeluna <danalitovsky+git@gmail.com>
Co-authored-by: Mark Byrne <31762852+mbyrnepr2@users.noreply.github.com>

doc/data/messages/c/consider-using-augmented-assign/bad.py

@@ -0,0 +1,2 @@
+x = 1
+x = x + 1  # [consider-using-augmented-assign]

doc/data/messages/c/consider-using-augmented-assign/good.py

@@ -0,0 +1,2 @@
+x = 1
+x += 1

doc/whatsnew/fragments/3391.new_check

@@ -0,0 +1,4 @@
+Added ``consider-using-augmented-assign`` which flags ``x = x + 1`` to simplify to
+``x += 1``.
+
+Closes #3391

pylint/checkers/refactoring/recommendation_checker.py

@@ -9,6 +9,7 @@
 
 from pylint import checkers
 from pylint.checkers import utils
+from pylint.interfaces import INFERENCE
 
 
 class RecommendationChecker(checkers.BaseChecker):
@@ -60,6 +61,12 @@ class RecommendationChecker(checkers.BaseChecker):
             "which could potentially be a f-string. The use of f-strings is preferred. "
             "Requires Python 3.6 and ``py-version >= 3.6``.",
         ),
+        "C0210": (
+            "Use '%s' to do an augmented assign directly",
+            "consider-using-augmented-assign",
+            "Emitted when an assignment is referring to the object that it is assigning "
+            "to. This can be changed to be an augmented assign.",
+        ),
     }
 
     def open(self) -> None:
@@ -417,3 +424,16 @@ def _detect_replacable_format_call(self, node: nodes.Const) -> None:
                 line=node.lineno,
                 col_offset=node.col_offset,
             )
+
+    @utils.only_required_for_messages("consider-using-augmented-assign")
+    def visit_assign(self, node: nodes.Assign) -> None:
+        is_aug, op = utils.is_augmented_assign(node)
+        if is_aug:
+            self.add_message(
+                "consider-using-augmented-assign",
+                args=f"{op}=",
+                node=node,
+                line=node.lineno,
+                col_offset=node.col_offset,
+                confidence=INFERENCE,
+            )

pylint/checkers/utils.py

@@ -1963,3 +1963,68 @@ def is_hashable(node: nodes.NodeNG) -> bool:
         return False
     except astroid.InferenceError:
         return True
+
+
+def get_full_name_of_attribute(node: nodes.Attribute | nodes.AssignAttr) -> str:
+    """Return the full name of an attribute and the classes it belongs to.
+
+    For example: "Class1.Class2.attr"
+    """
+    parent = node.parent
+    ret = node.attrname or ""
+    while isinstance(parent, (nodes.Attribute, nodes.Name)):
+        if isinstance(parent, nodes.Attribute):
+            ret = f"{parent.attrname}.{ret}"
+        else:
+            ret = f"{parent.name}.{ret}"
+        parent = parent.parent
+    return ret
+
+
+def _is_target_name_in_binop_side(
+    target: nodes.AssignName | nodes.AssignAttr, side: nodes.NodeNG | None
+) -> bool:
+    """Determine whether the target name-like node is referenced in the side node."""
+    if isinstance(side, nodes.Name):
+        if isinstance(target, nodes.AssignName):
+            return target.name == side.name  # type: ignore[no-any-return]
+        return False
+    if isinstance(side, nodes.Attribute) and isinstance(target, nodes.AssignAttr):
+        return get_full_name_of_attribute(target) == get_full_name_of_attribute(side)
+    return False
+
+
+def is_augmented_assign(node: nodes.Assign) -> tuple[bool, str]:
+    """Determine if the node is assigning itself (with modifications) to itself.
+
+    For example: x = 1 + x
+    """
+    if not isinstance(node.value, nodes.BinOp):
+        return False, ""
+
+    binop = node.value
+    target = node.targets[0]
+
+    if not isinstance(target, (nodes.AssignName, nodes.AssignAttr)):
+        return False, ""
+
+    # We don't want to catch x = "1" + x or x = "%s" % x
+    if isinstance(binop.left, nodes.Const) and isinstance(
+        binop.left.value, (str, bytes)
+    ):
+        return False, ""
+
+    # This could probably be improved but for now we disregard all assignments from calls
+    if isinstance(binop.left, nodes.Call) or isinstance(binop.right, nodes.Call):
+        return False, ""
+
+    if _is_target_name_in_binop_side(target, binop.left):
+        return True, binop.op
+    if _is_target_name_in_binop_side(target, binop.right):
+        inferred_left = safe_infer(binop.left)
+        if isinstance(inferred_left, nodes.Const) and isinstance(
+            inferred_left.value, int
+        ):
+            return True, binop.op
+        return False, ""
+    return False, ""

tests/functional/c/consider/consider_using_augmented_assign.py

@@ -0,0 +1,77 @@
+"""Tests for consider-using-augmented-assign."""
+
+# pylint: disable=invalid-name,too-few-public-methods,import-error,consider-using-f-string
+
+from unknown import Unknown
+
+x = 1
+x = x + 1  # [consider-using-augmented-assign]
+x = 1 + x  # [consider-using-augmented-assign]
+x, y = 1 + x, 2 + x
+# We don't warn on intricate expressions as we lack knowledge
+# of simplifying such expressions which is necessary to see
+# if they can become augmented
+x = 1 + x - 2
+x = 1 + x + 2
+
+# For anything other than a float or an int we only want to warn on
+# assignments where the 'itself' is on the left side of the assignment
+my_list = [2, 3, 4]
+my_list = [1] + my_list
+
+
+class MyClass:
+    """Simple base class."""
+
+    def __init__(self) -> None:
+        self.x = 1
+        self.x = self.x + 1  # [consider-using-augmented-assign]
+        self.x = 1 + self.x  # [consider-using-augmented-assign]
+
+        x = 1  # [redefined-outer-name]
+        self.x = x
+
+
+instance = MyClass()
+
+x = instance.x + 1
+
+my_str = ""
+my_str = my_str + "foo"  # [consider-using-augmented-assign]
+my_str = "foo" + my_str
+
+my_bytes = b""
+my_bytes = my_bytes + b"foo"  # [consider-using-augmented-assign]
+my_bytes = b"foo" + my_bytes
+
+
+def return_str() -> str:
+    """Return a string."""
+    return ""
+
+
+# Currently we disregard all calls
+my_str = return_str() + my_str
+
+my_str = my_str % return_str()
+my_str = my_str % 1  # [consider-using-augmented-assign]
+my_str = my_str % (1, 2)  # [consider-using-augmented-assign]
+my_str = "%s" % my_str
+my_str = return_str() % my_str
+my_str = Unknown % my_str
+my_str = my_str % Unknown  # [consider-using-augmented-assign]
+
+x = x - 3  # [consider-using-augmented-assign]
+x = x * 3  # [consider-using-augmented-assign]
+x = x / 3  # [consider-using-augmented-assign]
+x = x // 3  # [consider-using-augmented-assign]
+x = x << 3  # [consider-using-augmented-assign]
+x = x >> 3  # [consider-using-augmented-assign]
+x = x % 3  # [consider-using-augmented-assign]
+x = x**3  # [consider-using-augmented-assign]
+x = x ^ 3  # [consider-using-augmented-assign]
+x = x & 3  # [consider-using-augmented-assign]
+x = x > 3
+x = x < 3
+x = x >= 3
+x = x <= 3

tests/functional/c/consider/consider_using_augmented_assign.txt

@@ -0,0 +1,20 @@
+consider-using-augmented-assign:8:0:8:9::Use '+=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:9:0:9:9::Use '+=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:28:8:28:27:MyClass.__init__:Use '+=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:29:8:29:27:MyClass.__init__:Use '+=' to do an augmented assign directly:INFERENCE
+redefined-outer-name:31:8:31:9:MyClass.__init__:Redefining name 'x' from outer scope (line 7):UNDEFINED
+consider-using-augmented-assign:40:0:40:23::Use '+=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:44:0:44:28::Use '+=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:57:0:57:19::Use '%=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:58:0:58:24::Use '%=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:62:0:62:25::Use '%=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:64:0:64:9::Use '-=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:65:0:65:9::Use '*=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:66:0:66:9::Use '/=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:67:0:67:10::Use '//=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:68:0:68:10::Use '<<=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:69:0:69:10::Use '>>=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:70:0:70:9::Use '%=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:71:0:71:8::Use '**=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:72:0:72:9::Use '^=' to do an augmented assign directly:INFERENCE
+consider-using-augmented-assign:73:0:73:9::Use '&=' to do an augmented assign directly:INFERENCE

tests/functional/e/e1101_9588_base_attr_aug_assign.py

@@ -31,7 +31,7 @@ class NegativeClass(BaseClass):
     def __init__(self):
         "Ordinary assignment is OK."
         BaseClass.__init__(self)
-        self.e1101 = self.e1101 + 1
+        self.e1101 += self.e1101
 
     def countup(self):
         "No problem."

tests/functional/i/inconsistent/inconsistent_returns.py

@@ -91,13 +91,13 @@ def explicit_returns6(x, y, z):
 
 def explicit_returns7(arg):
     if arg < 0:
-        arg = 2 * arg
+        arg *= 2
         return 'below 0'
     elif arg == 0:
         print("Null arg")
         return '0'
     else:
-        arg = 3 * arg
+        arg *= 3
         return 'above 0'
 
 def bug_1772():
@@ -184,7 +184,7 @@ def explicit_implicit_returns3(arg): # [inconsistent-return-statements]
 
 def returns_missing_in_catched_exceptions(arg): # [inconsistent-return-statements]
     try:
-        arg = arg**2
+        arg **= arg
         raise ValueError('test')
     except ValueError:
         print('ValueError')

tests/functional/u/undefined/undefined_variable_py38.py

@@ -1,5 +1,5 @@
 """Tests for undefined variable with assignment expressions"""
-# pylint: disable=using-constant-test, expression-not-assigned
+# pylint: disable=using-constant-test, expression-not-assigned, consider-using-augmented-assign
 
 # Tests for annotation of variables and potentially undefinition
 

tests/functional/u/use/use_maxsplit_arg.py

@@ -1,5 +1,5 @@
 """Emit a message for accessing first/last element of string.split"""
-# pylint: disable=line-too-long,missing-docstring,unsubscriptable-object,too-few-public-methods,invalid-name,redefined-builtin
+# pylint: disable=line-too-long,missing-docstring,unsubscriptable-object,too-few-public-methods,invalid-name,redefined-builtin,consider-using-augmented-assign
 
 # Test subscripting .split()
 get_first = '1,2,3'.split(',')[0]  # [use-maxsplit-arg]

tests/functional/u/used/used_before_assignment_conditional.py

@@ -1,4 +1,7 @@
 """used-before-assignment cases involving IF conditions"""
+
+# pylint: disable=consider-using-augmented-assign
+
 if 1 + 1 == 2:
     x = x + 1  # [used-before-assignment]
 

tests/functional/u/used/used_before_assignment_conditional.txt

@@ -1,2 +1,2 @@
-used-before-assignment:3:8:3:9::Using variable 'x' before assignment:HIGH
-used-before-assignment:5:3:5:4::Using variable 'y' before assignment:HIGH
+used-before-assignment:6:8:6:9::Using variable 'x' before assignment:HIGH
+used-before-assignment:8:3:8:4::Using variable 'y' before assignment:HIGH

tests/functional/u/used/used_before_assignment_nonlocal.py

@@ -1,5 +1,5 @@
 """Check for nonlocal and used-before-assignment"""
-# pylint: disable=missing-docstring, unused-variable, too-few-public-methods
+# pylint: disable=missing-docstring, unused-variable, too-few-public-methods, consider-using-augmented-assign
 
 
 def test_ok():

tests/test_similar.py

@@ -36,7 +36,7 @@ def _runtest(self, args: list[str], code: int) -> None:
     @staticmethod
     def _run_pylint(args: list[str], out: TextIO) -> int:
         """Runs pylint with a patched output."""
-        args = args + [
+        args += [
             "--persistent=no",
             "--enable=astroid-error",
             # Enable functionality that will build another ast