Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add consider-using-augmented-assign checker #7514

Merged
merged 11 commits into from
Sep 22, 2022
2 changes: 2 additions & 0 deletions doc/data/messages/c/consider-using-augmented-assign/bad.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = 1
x = x + 1 # [consider-using-augmented-assign]
2 changes: 2 additions & 0 deletions doc/data/messages/c/consider-using-augmented-assign/good.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = 1
x += 1
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/3391.new_check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Added ``consider-using-augmented-assign`` which flags ``x = x + 1`` to simplify to
``x += 1``.

Closes #3391
20 changes: 20 additions & 0 deletions pylint/checkers/refactoring/recommendation_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from pylint import checkers
from pylint.checkers import utils
from pylint.interfaces import INFERENCE


class RecommendationChecker(checkers.BaseChecker):
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
)
65 changes: 65 additions & 0 deletions pylint/checkers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
"""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, ""
77 changes: 77 additions & 0 deletions tests/functional/c/consider/consider_using_augmented_assign.py
Original file line number Diff line number Diff line change
@@ -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
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved
# 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
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
# assignments where the 'itself' is on the left side of the assignment
my_list = [2, 3, 4]
my_list = [1] + my_list


Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved
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]
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved

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
20 changes: 20 additions & 0 deletions tests/functional/c/consider/consider_using_augmented_assign.txt
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion tests/functional/e/e1101_9588_base_attr_aug_assign.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
6 changes: 3 additions & 3 deletions tests/functional/i/inconsistent/inconsistent_returns.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down Expand Up @@ -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')
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/u/undefined/undefined_variable_py38.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down
2 changes: 1 addition & 1 deletion tests/functional/u/use/use_maxsplit_arg.py
Original file line number Diff line number Diff line change
@@ -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]
Expand Down
3 changes: 3 additions & 0 deletions tests/functional/u/used/used_before_assignment_conditional.py
Original file line number Diff line number Diff line change
@@ -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]

Expand Down
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion tests/functional/u/used/used_before_assignment_nonlocal.py
Original file line number Diff line number Diff line change
@@ -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():
Expand Down
2 changes: 1 addition & 1 deletion tests/test_similar.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down