Skip to content

Commit

Permalink
Merge pull request #476 from jakkdl/contextvar_mutable_or_call_default
Browse files Browse the repository at this point in the history
Contextvar mutable or call default
  • Loading branch information
Zac-HD authored Jun 25, 2024
2 parents 18abb9f + 6fee565 commit 188eab8
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 16 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
9 changes: 8 additions & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
~~~~~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -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 <https://github.com/PyCQA/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``.
Expand Down Expand Up @@ -351,6 +353,11 @@ MIT
Change Log
----------

FUTURE
~~~~~~

* Add B039, ``ContextVar`` with mutable literal or function call as default.

24.4.26
~~~~~~~

Expand Down
78 changes: 65 additions & 13 deletions bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"):
Expand All @@ -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)
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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_b039_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_b039_extend_immutable_calls
)
visitor.visit(kw.value)
self.errors.extend(visitor.errors)

def check_for_b007(self, node):
targets = NameFinder()
targets.visit(node.target)
Expand Down Expand Up @@ -1780,9 +1807,20 @@ def visit(self, node):
return node


class FuntionDefDefaultsVisitor(ast.NodeVisitor):
def __init__(self, b008_extend_immutable_calls=None):
self.b008_extend_immutable_calls = b008_extend_immutable_calls or set()
class FunctionDefDefaultsVisitor(ast.NodeVisitor):
"""Used by B006, B008, and B039. B039 is essentially B006+B008 but for ContextVar."""

def __init__(
self,
error_code_calls, # B006 or B039
error_code_literals, # B008 or B039
b008_b039_extend_immutable_calls=None,
):
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:
setattr(self, f"visit_{node}", self.visit_mutable_literal_or_comprehension)
self.errors = []
Expand All @@ -1801,18 +1839,18 @@ 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

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

Expand All @@ -1824,9 +1862,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)
Expand Down Expand Up @@ -1926,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 = {
Expand Down Expand Up @@ -1956,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",
Expand Down Expand Up @@ -2166,6 +2210,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=(
Expand Down
17 changes: 17 additions & 0 deletions tests/b039.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import contextvars
import time
from contextvars import ContextVar

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
14 changes: 14 additions & 0 deletions tests/test_bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
B035,
B036,
B037,
B039,
B901,
B902,
B903,
Expand Down Expand Up @@ -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))
Expand Down
3 changes: 2 additions & 1 deletion tox.ini
Original file line number Diff line number Diff line change
@@ -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 =
Expand All @@ -10,6 +10,7 @@ python =
3.10: py310
3.11: py311,pep8_naming
3.12: py312
3.13-dev: py313

[testenv]
description = Run coverage
Expand Down

0 comments on commit 188eab8

Please sign in to comment.