diff --git a/README.rst b/README.rst index 4bb4317..52820f4 100644 --- a/README.rst +++ b/README.rst @@ -188,6 +188,8 @@ second usage. Save the result to a list if the result is needed multiple times. **B033**: Sets should not contain duplicate items. Duplicate items will be replaced with a single item at runtime. +**B034**: Calls to `re.sub`, `re.subn` or `re.split` should pass `flags` or `count`/`maxsplit` as keyword arguments. It is commonly assumed that `flags` is the third positional parameter, forgetting about `count`/`maxsplit`, since many other `re` module functions are of the form `f(pattern, string, flags)`. + Opinionated warnings ~~~~~~~~~~~~~~~~~~~~ @@ -335,6 +337,7 @@ Unreleased * Fix a crash and several test failures on Python 3.12, all relating to the B907 check. * Declare support for Python 3.12. +* Add B034: re.sub/subn/split must pass flags/count/maxsplit as keyword arguments. 23.6.5 ~~~~~~ diff --git a/bugbear.py b/bugbear.py index 64d1e70..216c69f 100644 --- a/bugbear.py +++ b/bugbear.py @@ -423,8 +423,9 @@ def visit_Call(self, node): self.check_for_b026(node) - self.check_for_b905(node) self.check_for_b028(node) + self.check_for_b034(node) + self.check_for_b905(node) self.generic_visit(node) def visit_Module(self, node): @@ -1366,6 +1367,27 @@ def check_for_b033(self, node): else: seen.add(elt.value) + def check_for_b034(self, node: ast.Call): + if not isinstance(node.func, ast.Attribute): + return + if not isinstance(node.func.value, ast.Name) or node.func.value.id != "re": + return + + def check(num_args, param_name): + if len(node.args) > num_args: + self.errors.append( + B034( + node.args[num_args].lineno, + node.args[num_args].col_offset, + vars=(node.func.attr, param_name), + ) + ) + + if node.func.attr in ("sub", "subn"): + check(3, "count") + elif node.func.attr == "split": + check(2, "maxsplit") + def compose_call_path(node): if isinstance(node, ast.Attribute): @@ -1770,6 +1792,13 @@ def visit_Lambda(self, node): ) ) +B034 = Error( + message=( + "B034 {} should pass `{}` and `flags` as keyword arguments to avoid confusion" + " due to unintuitive argument positions." + ) +) + # Warnings disabled by default. B901 = Error( message=( diff --git a/tests/b034.py b/tests/b034.py new file mode 100644 index 0000000..8c20009 --- /dev/null +++ b/tests/b034.py @@ -0,0 +1,30 @@ +import re +from re import sub + +# error +re.sub("a", "b", "aaa", re.IGNORECASE) +re.sub("a", "b", "aaa", 5) +re.sub("a", "b", "aaa", 5, re.IGNORECASE) +re.subn("a", "b", "aaa", re.IGNORECASE) +re.subn("a", "b", "aaa", 5) +re.subn("a", "b", "aaa", 5, re.IGNORECASE) +re.split(" ", "a a a a", re.I) +re.split(" ", "a a a a", 2) +re.split(" ", "a a a a", 2, re.I) + +# okay +re.sub("a", "b", "aaa") +re.sub("a", "b", "aaa", flags=re.IGNORECASE) +re.sub("a", "b", "aaa", count=5) +re.sub("a", "b", "aaa", count=5, flags=re.IGNORECASE) +re.subn("a", "b", "aaa") +re.subn("a", "b", "aaa", flags=re.IGNORECASE) +re.subn("a", "b", "aaa", count=5) +re.subn("a", "b", "aaa", count=5, flags=re.IGNORECASE) +re.split(" ", "a a a a", flags=re.I) +re.split(" ", "a a a a", maxsplit=2) +re.split(" ", "a a a a", maxsplit=2, flags=re.I) + + +# not covered +sub("a", "b", "aaa", re.IGNORECASE) diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index e83769c..5524d04 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -42,6 +42,7 @@ B031, B032, B033, + B034, B901, B902, B903, @@ -502,6 +503,23 @@ def test_b033(self): ) self.assertEqual(errors, expected) + def test_b034(self): + filename = Path(__file__).absolute().parent / "b034.py" + bbc = BugBearChecker(filename=str(filename)) + errors = list(bbc.run()) + expected = self.errors( + B034(5, 24, vars=("sub", "count")), + B034(6, 24, vars=("sub", "count")), + B034(7, 24, vars=("sub", "count")), + B034(8, 25, vars=("subn", "count")), + B034(9, 25, vars=("subn", "count")), + B034(10, 25, vars=("subn", "count")), + B034(11, 25, vars=("split", "maxsplit")), + B034(12, 25, vars=("split", "maxsplit")), + B034(13, 25, vars=("split", "maxsplit")), + ) + self.assertEqual(errors, expected) + def test_b908(self): filename = Path(__file__).absolute().parent / "b908.py" bbc = BugBearChecker(filename=str(filename))