From f72ba6c7054b75411d737b29c90977851f2481a9 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sun, 23 Jun 2024 11:45:10 +0200 Subject: [PATCH 1/4] add b039: ContextVar with mutable literal or function call as default --- README.rst | 9 ++++++- bugbear.py | 55 +++++++++++++++++++++++++++++++++++++------ tests/b039.py | 17 +++++++++++++ tests/test_bugbear.py | 14 +++++++++++ 4 files changed, 87 insertions(+), 8 deletions(-) create mode 100644 tests/b039.py diff --git a/README.rst b/README.rst index ce0cfcb..a210968 100644 --- a/README.rst +++ b/README.rst @@ -201,6 +201,8 @@ second usage. Save the result to a list if the result is needed multiple times. **B038**: **Moved to B909** - Found a mutation of a mutable loop iterable inside the loop body. Changes to the iterable of a loop such as calls to `list.remove()` or via `del` can cause unintended bugs. +**B039**: ``ContextVar`` with mutable literal or function call as default. This is only evaluated once, and all subsequent calls to `.get()` would return the same instance of the default. This uses the same logic as B006 and B008, including ignoring values in ``extend-immutable-calls``. + Opinionated warnings ~~~~~~~~~~~~~~~~~~~~ @@ -315,7 +317,7 @@ The plugin currently has the following settings: ``extend-immutable-calls``: Specify a list of additional immutable calls. This could be useful, when using other libraries that provide more immutable calls, beside those already handled by ``flake8-bugbear``. Calls to these method will no longer -raise a ``B008`` warning. +raise a ``B008`` or ``B039`` warning. ``classmethod-decorators``: Specify a list of decorators to additionally mark a method as a ``classmethod`` as used by B902. The default only checks for ``classmethod``. When an ``@obj.name`` decorator is specified it will match against either ``name`` or ``obj.name``. This functions similarly to how `pep8-naming ` handles it, but with different defaults, and they don't support specifying attributes such that a decorator will never match against a specified value ``obj.name`` even if decorated with ``@obj.name``. @@ -351,6 +353,11 @@ MIT Change Log ---------- +FUTURE +~~~~~~ + +* Add B039, ``ContextVar`` with mutable literal or function call as default. + 24.4.26 ~~~~~~~ diff --git a/bugbear.py b/bugbear.py index c2a3032..740d164 100644 --- a/bugbear.py +++ b/bugbear.py @@ -507,6 +507,7 @@ def visit_Call(self, node): self.check_for_b026(node) self.check_for_b028(node) self.check_for_b034(node) + self.check_for_b039(node) self.check_for_b905(node) self.generic_visit(node) @@ -655,10 +656,36 @@ def check_for_b005(self, node): self.errors.append(B005(node.lineno, node.col_offset)) def check_for_b006_and_b008(self, node): - visitor = FuntionDefDefaultsVisitor(self.b008_extend_immutable_calls) + visitor = FunctionDefDefaultsVisitor( + B006, B008, self.b008_extend_immutable_calls + ) visitor.visit(node.args.defaults + node.args.kw_defaults) self.errors.extend(visitor.errors) + def check_for_b039(self, node: ast.Call): + if not ( + (isinstance(node.func, ast.Name) and node.func.id == "ContextVar") + or ( + isinstance(node.func, ast.Attribute) + and node.func.attr == "ContextVar" + and isinstance(node.func.value, ast.Name) + and node.func.value.id == "contextvars" + ) + ): + return + # ContextVar only takes one kw currently, but better safe than sorry + for kw in node.keywords: + if kw.arg == "default": + break + else: + return + + visitor = FunctionDefDefaultsVisitor( + B039, B039, self.b008_extend_immutable_calls + ) + visitor.visit(kw.value) + self.errors.extend(visitor.errors) + def check_for_b007(self, node): targets = NameFinder() targets.visit(node.target) @@ -1780,9 +1807,13 @@ def visit(self, node): return node -class FuntionDefDefaultsVisitor(ast.NodeVisitor): - def __init__(self, b008_extend_immutable_calls=None): +class FunctionDefDefaultsVisitor(ast.NodeVisitor): + def __init__( + self, error_code_calls, error_code_literals, b008_extend_immutable_calls=None + ): self.b008_extend_immutable_calls = b008_extend_immutable_calls or set() + self.error_code_calls = error_code_calls + self.error_code_literals = error_code_literals for node in B006.mutable_literals + B006.mutable_comprehensions: setattr(self, f"visit_{node}", self.visit_mutable_literal_or_comprehension) self.errors = [] @@ -1801,14 +1832,14 @@ def visit_mutable_literal_or_comprehension(self, node): # # We do still search for cases of B008 within mutable structures though. if self.arg_depth == 1: - self.errors.append(B006(node.lineno, node.col_offset)) + self.errors.append(self.error_code_calls(node.lineno, node.col_offset)) # Check for nested functions. self.generic_visit(node) def visit_Call(self, node): call_path = ".".join(compose_call_path(node.func)) if call_path in B006.mutable_calls: - self.errors.append(B006(node.lineno, node.col_offset)) + self.errors.append(self.error_code_calls(node.lineno, node.col_offset)) self.generic_visit(node) return @@ -1824,9 +1855,11 @@ def visit_Call(self, node): pass else: if math.isfinite(value): - self.errors.append(B008(node.lineno, node.col_offset)) + self.errors.append( + self.error_code_literals(node.lineno, node.col_offset) + ) else: - self.errors.append(B008(node.lineno, node.col_offset)) + self.errors.append(self.error_code_literals(node.lineno, node.col_offset)) # Check for nested functions. self.generic_visit(node) @@ -2166,6 +2199,14 @@ def visit_Lambda(self, node): message="B037 Class `__init__` methods must not return or yield and any values." ) +B039 = Error( + message=( + "B039 ContextVar with mutable literal or function call as default. " + "This is only evaluated once, and all subsequent calls to `.get()` " + "will return the same instance of the default." + ) +) + # Warnings disabled by default. B901 = Error( message=( diff --git a/tests/b039.py b/tests/b039.py new file mode 100644 index 0000000..baa316e --- /dev/null +++ b/tests/b039.py @@ -0,0 +1,17 @@ +from contextvars import ContextVar +import contextvars +import time + +ContextVar("cv", default=[]) # bad +ContextVar("cv", default=list()) # bad +ContextVar("cv", default=set()) # bad +ContextVar("cv", default=time.time()) # bad (B008-like) +contextvars.ContextVar("cv", default=[]) # bad + + +# good +ContextVar("cv", default=()) +contextvars.ContextVar("cv", default=()) +ContextVar("cv", default=tuple()) + +# see tests/b006_b008.py for more comprehensive tests diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index e84e4e8..249484e 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -46,6 +46,7 @@ B035, B036, B037, + B039, B901, B902, B903, @@ -636,6 +637,19 @@ def test_b037(self) -> None: ) self.assertEqual(errors, expected) + def test_b039(self) -> None: + filename = Path(__file__).absolute().parent / "b039.py" + bbc = BugBearChecker(filename=str(filename)) + errors = list(bbc.run()) + expected = self.errors( + B039(5, 25), + B039(6, 25), + B039(7, 25), + B039(8, 25), + B039(9, 37), + ) + self.assertEqual(errors, expected) + def test_b908(self): filename = Path(__file__).absolute().parent / "b908.py" bbc = BugBearChecker(filename=str(filename)) From 446f75269a54d7be420ab69f76e4924b96057022 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sun, 23 Jun 2024 11:51:27 +0200 Subject: [PATCH 2/4] add py313 to tox+CI --- .github/workflows/ci.yml | 2 +- tox.ini | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 783f923..4c88b3c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,7 +9,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] + python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13-dev"] os: [ubuntu-latest] diff --git a/tox.ini b/tox.ini index d83f206..3997ad3 100644 --- a/tox.ini +++ b/tox.ini @@ -1,7 +1,7 @@ # The test environment and commands [tox] # default environments to run without `-e` -envlist = py38, py39, py310, py311, py312, pep8_naming +envlist = py38, py39, py310, py311, py312, py313, pep8_naming [gh-actions] python = @@ -10,6 +10,7 @@ python = 3.10: py310 3.11: py311,pep8_naming 3.12: py312 + 3.13-dev: py313 [testenv] description = Run coverage From 70a8853a84138ad94f2b9aea78f1d2709e7a7f3d Mon Sep 17 00:00:00 2001 From: jakkdl Date: Tue, 25 Jun 2024 13:01:12 +0200 Subject: [PATCH 3/4] fix isort --- tests/b039.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/b039.py b/tests/b039.py index baa316e..1ba7ce3 100644 --- a/tests/b039.py +++ b/tests/b039.py @@ -1,6 +1,6 @@ -from contextvars import ContextVar import contextvars import time +from contextvars import ContextVar ContextVar("cv", default=[]) # bad ContextVar("cv", default=list()) # bad From 6fee565cb53598eee192becd28b79932a1487e39 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Tue, 25 Jun 2024 13:07:02 +0200 Subject: [PATCH 4/4] add some comments, rename b008_extend_immutable_calls --- bugbear.py | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/bugbear.py b/bugbear.py index 740d164..d3c3e7b 100644 --- a/bugbear.py +++ b/bugbear.py @@ -63,9 +63,9 @@ def run(self): self.load_file() if self.options and hasattr(self.options, "extend_immutable_calls"): - b008_extend_immutable_calls = set(self.options.extend_immutable_calls) + b008_b039_extend_immutable_calls = set(self.options.extend_immutable_calls) else: - b008_extend_immutable_calls = set() + b008_b039_extend_immutable_calls = set() b902_classmethod_decorators: set[str] = B902_default_decorators if self.options and hasattr(self.options, "classmethod_decorators"): @@ -74,7 +74,7 @@ def run(self): visitor = self.visitor( filename=self.filename, lines=self.lines, - b008_extend_immutable_calls=b008_extend_immutable_calls, + b008_b039_extend_immutable_calls=b008_b039_extend_immutable_calls, b902_classmethod_decorators=b902_classmethod_decorators, ) visitor.visit(self.tree) @@ -360,7 +360,7 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler): class BugBearVisitor(ast.NodeVisitor): filename = attr.ib() lines = attr.ib() - b008_extend_immutable_calls = attr.ib(default=attr.Factory(set)) + b008_b039_extend_immutable_calls = attr.ib(default=attr.Factory(set)) b902_classmethod_decorators = attr.ib(default=attr.Factory(set)) node_window = attr.ib(default=attr.Factory(list)) errors = attr.ib(default=attr.Factory(list)) @@ -657,7 +657,7 @@ def check_for_b005(self, node): def check_for_b006_and_b008(self, node): visitor = FunctionDefDefaultsVisitor( - B006, B008, self.b008_extend_immutable_calls + B006, B008, self.b008_b039_extend_immutable_calls ) visitor.visit(node.args.defaults + node.args.kw_defaults) self.errors.extend(visitor.errors) @@ -681,7 +681,7 @@ def check_for_b039(self, node: ast.Call): return visitor = FunctionDefDefaultsVisitor( - B039, B039, self.b008_extend_immutable_calls + B039, B039, self.b008_b039_extend_immutable_calls ) visitor.visit(kw.value) self.errors.extend(visitor.errors) @@ -1808,10 +1808,17 @@ def visit(self, node): class FunctionDefDefaultsVisitor(ast.NodeVisitor): + """Used by B006, B008, and B039. B039 is essentially B006+B008 but for ContextVar.""" + def __init__( - self, error_code_calls, error_code_literals, b008_extend_immutable_calls=None + self, + error_code_calls, # B006 or B039 + error_code_literals, # B008 or B039 + b008_b039_extend_immutable_calls=None, ): - self.b008_extend_immutable_calls = b008_extend_immutable_calls or set() + self.b008_b039_extend_immutable_calls = ( + b008_b039_extend_immutable_calls or set() + ) self.error_code_calls = error_code_calls self.error_code_literals = error_code_literals for node in B006.mutable_literals + B006.mutable_comprehensions: @@ -1843,7 +1850,7 @@ def visit_Call(self, node): self.generic_visit(node) return - if call_path in B008.immutable_calls | self.b008_extend_immutable_calls: + if call_path in B008.immutable_calls | self.b008_b039_extend_immutable_calls: self.generic_visit(node) return @@ -1959,6 +1966,8 @@ def visit_Lambda(self, node): "between them." ) ) + +# Note: these are also used by B039 B006.mutable_literals = ("Dict", "List", "Set") B006.mutable_comprehensions = ("ListComp", "DictComp", "SetComp") B006.mutable_calls = { @@ -1989,6 +1998,8 @@ def visit_Lambda(self, node): "use that variable as a default value." ) ) + +# Note: these are also used by B039 B008.immutable_calls = { "tuple", "frozenset",