From 6f8a3d8637aace669781a65f8348fcffb23e95fa Mon Sep 17 00:00:00 2001 From: Philip Meier Date: Tue, 20 Sep 2022 08:09:30 +0200 Subject: [PATCH 1/4] B026 - Argument unpacking after keyword argument --- README.rst | 2 ++ bugbear.py | 16 ++++++++++++++++ tests/b026.py | 16 ++++++++++++++++ tests/test_bugbear.py | 14 ++++++++++++++ 4 files changed, 48 insertions(+) create mode 100644 tests/b026.py diff --git a/README.rst b/README.rst index d7981f7..051a5d2 100644 --- a/README.rst +++ b/README.rst @@ -160,6 +160,8 @@ 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**: Argument unpacking after keyword argument. + Opinionated warnings ~~~~~~~~~~~~~~~~~~~~ diff --git a/bugbear.py b/bugbear.py index 4b742b7..e244393 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,19 @@ 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): + try: + starred = next(starred for starred in call.args if isinstance(starred, ast.Starred)) + except StopIteration: + return + + if any( + (keyword.value.lineno, keyword.value.col_offset) < ( + starred.lineno, starred.col_offset) + for keyword in call.keywords + ): + self.errors.append(B026(call.lineno, call.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 +1218,7 @@ def visit_Lambda(self, node): " will be considered and all other except catches can be safely removed." ) ) +B026 = Error(message="B026 Argument unpacking after keyword argument.") # Warnings disabled by default. B901 = Error( diff --git a/tests/b026.py b/tests/b026.py new file mode 100644 index 0000000..f577cd4 --- /dev/null +++ b/tests/b026.py @@ -0,0 +1,16 @@ +""" +Should emit: +B026 - on lines 14, 15, 16 +""" + + +def foo(bar, baz, bam): + pass + + +foo("bar", "baz", bam="bam") +foo("bar", baz="baz", bam="bam") +foo(bar="bar", baz="baz", bam="bam") +foo(bam="bam", *["bar", "baz"]) +foo(baz="baz", bam="bam", *["bar"]) +foo(bar="bar", baz="baz", bam="bam", *[]) \ No newline at end of file diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index b09bb48..9b5b9e6 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,19 @@ 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(14, 0), + B026(15, 0), + B026(16, 0), + ), + ) + def test_b901(self): filename = Path(__file__).absolute().parent / "b901.py" bbc = BugBearChecker(filename=str(filename)) From a3e55944769883438ddadb2544794d429f8b5b8f Mon Sep 17 00:00:00 2001 From: Philip Meier Date: Tue, 20 Sep 2022 08:12:13 +0200 Subject: [PATCH 2/4] format --- bugbear.py | 8 ++++---- tests/b026.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bugbear.py b/bugbear.py index e244393..472c6ed 100644 --- a/bugbear.py +++ b/bugbear.py @@ -645,14 +645,14 @@ def is_abstract_decorator(expr): def check_for_b026(self, call: ast.Call): try: - starred = next(starred for starred in call.args if isinstance(starred, ast.Starred)) + starred = next(arg for arg in call.args if isinstance(arg, ast.Starred)) except StopIteration: return if any( - (keyword.value.lineno, keyword.value.col_offset) < ( - starred.lineno, starred.col_offset) - for keyword in call.keywords + (keyword.value.lineno, keyword.value.col_offset) + < (starred.lineno, starred.col_offset) + for keyword in call.keywords ): self.errors.append(B026(call.lineno, call.col_offset)) diff --git a/tests/b026.py b/tests/b026.py index f577cd4..215e459 100644 --- a/tests/b026.py +++ b/tests/b026.py @@ -13,4 +13,4 @@ def foo(bar, baz, bam): foo(bar="bar", baz="baz", bam="bam") foo(bam="bam", *["bar", "baz"]) foo(baz="baz", bam="bam", *["bar"]) -foo(bar="bar", baz="baz", bam="bam", *[]) \ No newline at end of file +foo(bar="bar", baz="baz", bam="bam", *[]) From a468ce8a64d5c9193e4bf5528647e53ea2983e5a Mon Sep 17 00:00:00 2001 From: Philip Meier Date: Wed, 21 Sep 2022 10:01:02 +0200 Subject: [PATCH 3/4] add support for multiple unpackings --- bugbear.py | 21 ++++++++++++--------- tests/b026.py | 7 ++++++- tests/test_bugbear.py | 10 +++++++--- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/bugbear.py b/bugbear.py index 472c6ed..71287f9 100644 --- a/bugbear.py +++ b/bugbear.py @@ -644,17 +644,20 @@ 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): - try: - starred = next(arg for arg in call.args if isinstance(arg, ast.Starred)) - except StopIteration: + if not call.keywords: return - if any( - (keyword.value.lineno, keyword.value.col_offset) - < (starred.lineno, starred.col_offset) - for keyword in call.keywords - ): - self.errors.append(B026(call.lineno, call.col_offset)) + 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) diff --git a/tests/b026.py b/tests/b026.py index 215e459..ada19b4 100644 --- a/tests/b026.py +++ b/tests/b026.py @@ -1,6 +1,6 @@ """ Should emit: -B026 - on lines 14, 15, 16 +B026 - on lines 16, 17, 18, 19, 20, 21 """ @@ -8,9 +8,14 @@ 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 9b5b9e6..28eb964 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -389,9 +389,13 @@ def test_b026(self): self.assertEqual( errors, self.errors( - B026(14, 0), - B026(15, 0), - B026(16, 0), + B026(16, 15), + B026(17, 15), + B026(18, 26), + B026(19, 37), + B026(20, 15), + B026(20, 25), + B026(21, 25), ), ) From 9d36545cfc8d58056cf105507f3ff38c4daad9ad Mon Sep 17 00:00:00 2001 From: Philip Meier Date: Thu, 22 Sep 2022 09:44:00 +0200 Subject: [PATCH 4/4] fix wording --- README.rst | 7 ++++++- bugbear.py | 9 ++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 051a5d2..b7d4f38 100644 --- a/README.rst +++ b/README.rst @@ -160,7 +160,12 @@ 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**: Argument unpacking after keyword argument. +**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 71287f9..cc92b92 100644 --- a/bugbear.py +++ b/bugbear.py @@ -1221,7 +1221,14 @@ def visit_Lambda(self, node): " will be considered and all other except catches can be safely removed." ) ) -B026 = Error(message="B026 Argument unpacking after keyword argument.") +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(