Skip to content

Commit

Permalink
feat(rules): Add rule to check for mutations of loop iterator
Browse files Browse the repository at this point in the history
  • Loading branch information
mimre25 committed Jan 12, 2024
1 parent b4c661b commit fd4e612
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 0 deletions.
59 changes: 59 additions & 0 deletions bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ def _flatten_excepthandler(node):
):
expr_list.extend(expr.value.elts)
continue

yield expr


Expand Down Expand Up @@ -495,6 +496,7 @@ def visit_For(self, node):
self.check_for_b020(node)
self.check_for_b023(node)
self.check_for_b031(node)
self.check_for_b999(node)
self.generic_visit(node)

def visit_AsyncFor(self, node):
Expand Down Expand Up @@ -1544,6 +1546,18 @@ def check(num_args, param_name):
elif node.func.attr == "split":
check(2, "maxsplit")

def check_for_b999(self, node: ast.For):
if isinstance(node.iter, ast.Name):
name = _to_name_str(node.iter)
elif isinstance(node.iter, ast.Attribute):
name = _to_name_str(node.iter)
else:
return
checker = B999Checker(name)
checker.visit(node.body)
for mutation in checker.mutations:
self.errors.append(B999(mutation.lineno, mutation.col_offset))


def compose_call_path(node):
if isinstance(node, ast.Attribute):
Expand All @@ -1555,6 +1569,49 @@ def compose_call_path(node):
yield node.id


class B999Checker(ast.NodeVisitor):
def __init__(self, name: str):
self.name = name
self.mutations = []

def visit_Delete(self, node: ast.Delete):
for target in node.targets:
if isinstance(target, ast.Subscript):
name = _to_name_str(target.value)
elif isinstance(target, (ast.Attribute, ast.Name)):
name = _to_name_str(target)
else:
name = "" # fallback
self.generic_visit(target)

if name == self.name:
self.mutations.append(node)

def visit_Call(self, node: ast.Call):
if isinstance(node.func, ast.Attribute):
name = _to_name_str(node.func.value)
function_object = name

if function_object == self.name:
self.mutations.append(node)

self.generic_visit(node)

def visit_Name(self, node: ast.Name):
if isinstance(node.ctx, ast.Del):
self.mutations.append(node)
self.generic_visit(node)

def visit(self, node):
"""Like super-visit but supports iteration over lists."""
if not isinstance(node, list):
return super().visit(node)

for elem in node:
super().visit(elem)
return node


@attr.s
class NameFinder(ast.NodeVisitor):
"""Finds a name within a tree of nodes.
Expand Down Expand Up @@ -2045,6 +2102,8 @@ def visit_Lambda(self, node):
" statement."
)
)

B950 = Error(message="B950 line too long ({} > {} characters)")

B999 = Error(message="B999 editing loop iterable can cause unintended bugs!")
disabled_by_default = ["B901", "B902", "B903", "B904", "B905", "B906", "B908", "B950"]
46 changes: 46 additions & 0 deletions tests/b999.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
"""
Should emit:
B999 - on lines 11, 25, 26, 40, 46
"""


some_list = [1, 2, 3]
for elem in some_list:
print(elem)
if elem % 2 == 0:
some_list.remove(elem) # should error

some_list = [1, 2, 3]
some_other_list = [1, 2, 3]
for elem in some_list:
print(elem)
if elem % 2 == 0:
some_other_list.remove(elem) # should not error


some_list = [1, 2, 3]
for elem in some_list:
print(elem)
if elem % 2 == 0:
del some_list[2] # should error
del some_list


class A:
some_list: list

def __init__(self, ls):
self.some_list = list(ls)


a = A((1, 2, 3))
for elem in a.some_list:
print(elem)
if elem % 2 == 0:
a.some_list.remove(elem) # should error

a = A((1, 2, 3))
for elem in a.some_list:
print(elem)
if elem % 2 == 0:
del a.some_list[2] # should error
16 changes: 16 additions & 0 deletions tests/test_bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
B907,
B908,
B950,
B999,
BugBearChecker,
BugBearVisitor,
)
Expand Down Expand Up @@ -952,6 +953,21 @@ def test_selfclean_test_bugbear(self):
self.assertEqual(proc.stdout, b"")
self.assertEqual(proc.stderr, b"")

def test_b999(self):
filename = Path(__file__).absolute().parent / "b999.py"
mock_options = Namespace(select=[], extend_select=["B999"])
bbc = BugBearChecker(filename=str(filename), options=mock_options)
errors = list(bbc.run())
print(errors)
expected = [
B999(11, 8),
B999(25, 8),
B999(26, 8),
B999(40, 8),
B999(46, 8),
]
self.assertEqual(errors, self.errors(*expected))


class TestFuzz(unittest.TestCase):
# TODO: enable this test on py312 once hypothesmith supports py312
Expand Down

0 comments on commit fd4e612

Please sign in to comment.