diff --git a/doc/data/messages/c/consider-using-augmented-assign/bad.py b/doc/data/messages/c/consider-using-augmented-assign/bad.py new file mode 100644 index 0000000000..90b8931a66 --- /dev/null +++ b/doc/data/messages/c/consider-using-augmented-assign/bad.py @@ -0,0 +1,2 @@ +x = 1 +x = x + 1 # [consider-using-augmented-assign] diff --git a/doc/data/messages/c/consider-using-augmented-assign/good.py b/doc/data/messages/c/consider-using-augmented-assign/good.py new file mode 100644 index 0000000000..3e34f6b266 --- /dev/null +++ b/doc/data/messages/c/consider-using-augmented-assign/good.py @@ -0,0 +1,2 @@ +x = 1 +x += 1 diff --git a/doc/whatsnew/fragments/3391.new_check b/doc/whatsnew/fragments/3391.new_check new file mode 100644 index 0000000000..7111a3a5ce --- /dev/null +++ b/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 diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py index f610763beb..8a3b2d2044 100644 --- a/pylint/checkers/refactoring/recommendation_checker.py +++ b/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, + ) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index fe462b258a..2a4426083c 100644 --- a/pylint/checkers/utils.py +++ b/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, "" diff --git a/tests/functional/c/consider/consider_using_augmented_assign.py b/tests/functional/c/consider/consider_using_augmented_assign.py new file mode 100644 index 0000000000..e4af956864 --- /dev/null +++ b/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 diff --git a/tests/functional/c/consider/consider_using_augmented_assign.txt b/tests/functional/c/consider/consider_using_augmented_assign.txt new file mode 100644 index 0000000000..1684953e9e --- /dev/null +++ b/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 diff --git a/tests/functional/e/e1101_9588_base_attr_aug_assign.py b/tests/functional/e/e1101_9588_base_attr_aug_assign.py index 9fe95fd4ba..131dfc2c9b 100644 --- a/tests/functional/e/e1101_9588_base_attr_aug_assign.py +++ b/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." diff --git a/tests/functional/i/inconsistent/inconsistent_returns.py b/tests/functional/i/inconsistent/inconsistent_returns.py index 08dde253e9..c1183b288e 100644 --- a/tests/functional/i/inconsistent/inconsistent_returns.py +++ b/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') diff --git a/tests/functional/u/undefined/undefined_variable_py38.py b/tests/functional/u/undefined/undefined_variable_py38.py index 61a70d4724..79a23da13d 100644 --- a/tests/functional/u/undefined/undefined_variable_py38.py +++ b/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 diff --git a/tests/functional/u/use/use_maxsplit_arg.py b/tests/functional/u/use/use_maxsplit_arg.py index d0d43c2b98..28973c0f40 100644 --- a/tests/functional/u/use/use_maxsplit_arg.py +++ b/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] diff --git a/tests/functional/u/used/used_before_assignment_conditional.py b/tests/functional/u/used/used_before_assignment_conditional.py index b5d16925eb..2691bd099e 100644 --- a/tests/functional/u/used/used_before_assignment_conditional.py +++ b/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] diff --git a/tests/functional/u/used/used_before_assignment_conditional.txt b/tests/functional/u/used/used_before_assignment_conditional.txt index 56626f0fee..9e009df288 100644 --- a/tests/functional/u/used/used_before_assignment_conditional.txt +++ b/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 diff --git a/tests/functional/u/used/used_before_assignment_nonlocal.py b/tests/functional/u/used/used_before_assignment_nonlocal.py index 18e16177d0..f48281aae9 100644 --- a/tests/functional/u/used/used_before_assignment_nonlocal.py +++ b/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(): diff --git a/tests/test_similar.py b/tests/test_similar.py index f054d7473e..5558b70e73 100644 --- a/tests/test_similar.py +++ b/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