diff --git a/README.rst b/README.rst index d7981f7..b7d4f38 100644 --- a/README.rst +++ b/README.rst @@ -160,6 +160,13 @@ the loop, because `late-binding closures are a classic gotcha This check identifies exception types that are specified in multiple ``except`` clauses. The first specification is the only one ever considered, so all others can be removed. +**B026**: Star-arg unpacking after a keyword argument is strongly discouraged, because +it only works when the keyword parameter is declared after all parameters supplied by +the unpacked sequence, and this change of ordering can surprise and mislead readers. +There was `cpython discussion of disallowing this syntax +`_, but legacy usage and parser +limitations make it difficult. + Opinionated warnings ~~~~~~~~~~~~~~~~~~~~ diff --git a/bugbear.py b/bugbear.py index 4b742b7..cc92b92 100644 --- a/bugbear.py +++ b/bugbear.py @@ -354,6 +354,8 @@ def visit_Call(self, node): ): self.errors.append(B010(node.lineno, node.col_offset)) + self.check_for_b026(node) + self.generic_visit(node) def visit_Assign(self, node): @@ -641,6 +643,22 @@ def is_abstract_decorator(expr): self.errors.append(B024(node.lineno, node.col_offset, vars=(node.name,))) + def check_for_b026(self, call: ast.Call): + if not call.keywords: + return + + starreds = [arg for arg in call.args if isinstance(arg, ast.Starred)] + if not starreds: + return + + first_keyword = call.keywords[0].value + for starred in starreds: + if (starred.lineno, starred.col_offset) > ( + first_keyword.lineno, + first_keyword.col_offset, + ): + self.errors.append(B026(starred.lineno, starred.col_offset)) + def _get_assigned_names(self, loop_node): loop_targets = (ast.For, ast.AsyncFor, ast.comprehension) for node in children_in_scope(loop_node): @@ -1203,6 +1221,14 @@ def visit_Lambda(self, node): " will be considered and all other except catches can be safely removed." ) ) +B026 = Error( + message=( + "B026 Star-arg unpacking after a keyword argument is strongly discouraged, " + "because it only works when the keyword parameter is declared after all " + "parameters supplied by the unpacked sequence, and this change of ordering can " + "surprise and mislead readers." + ) +) # Warnings disabled by default. B901 = Error( diff --git a/tests/b026.py b/tests/b026.py new file mode 100644 index 0000000..ada19b4 --- /dev/null +++ b/tests/b026.py @@ -0,0 +1,21 @@ +""" +Should emit: +B026 - on lines 16, 17, 18, 19, 20, 21 +""" + + +def foo(bar, baz, bam): + pass + + +bar_baz = ["bar", "baz"] + +foo("bar", "baz", bam="bam") +foo("bar", baz="baz", bam="bam") +foo(bar="bar", baz="baz", bam="bam") +foo(bam="bam", *["bar", "baz"]) +foo(bam="bam", *bar_baz) +foo(baz="baz", bam="bam", *["bar"]) +foo(bar="bar", baz="baz", bam="bam", *[]) +foo(bam="bam", *["bar"], *["baz"]) +foo(*["bar"], bam="bam", *["baz"]) diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index b09bb48..28eb964 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -36,6 +36,7 @@ B023, B024, B025, + B026, B901, B902, B903, @@ -381,6 +382,23 @@ def test_b025(self): ), ) + def test_b026(self): + filename = Path(__file__).absolute().parent / "b026.py" + bbc = BugBearChecker(filename=str(filename)) + errors = list(bbc.run()) + self.assertEqual( + errors, + self.errors( + B026(16, 15), + B026(17, 15), + B026(18, 26), + B026(19, 37), + B026(20, 15), + B026(20, 25), + B026(21, 25), + ), + ) + def test_b901(self): filename = Path(__file__).absolute().parent / "b901.py" bbc = BugBearChecker(filename=str(filename))