Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-bugbear] Implement mutable-contextvar-default (B039) #12113

Merged
merged 2 commits into from
Jul 1, 2024

Conversation

tjkuson
Copy link
Contributor

@tjkuson tjkuson commented Jun 30, 2024

Summary

Implement mutable-contextvar-default (B039) which was added to flake8-bugbear in PyCQA/flake8-bugbear#476.

This rule is similar to mutable-argument-default (B006) and function-call-in-default-argument (B008), except that it checks the default keyword argument to contextvars.ContextVar.

B039.py:19:26: B039 Do not use mutable data structures for ContextVar defaults
   |
18 | # Bad
19 | ContextVar("cv", default=[])
   |                          ^^ B039
20 | ContextVar("cv", default={})
21 | ContextVar("cv", default=list())
   |
   = help: Replace with `None`; initialize with `.set()` after checking for `None`

In the upstream flake8-plugin, this rule is written expressly as a corollary to B008 and shares much of its logic. Likewise, this implementation reuses the logic of the Ruff implementation of B008, namely

Expr::Call(ast::ExprCall { func, .. }) => {
if !is_mutable_func(func, self.semantic)
&& !is_immutable_func(func, self.semantic, self.extend_immutable_calls)

and

if is_mutable_expr(default, checker.semantic())

Thus, this rule deliberately replicates B006's and B008's heuristics. For example, this rule assumes that all functions are mutable unless otherwise qualified. If improvements are to be made to B039 heuristics, they should probably be made to B006 and B008 as well (whilst trying to match the upstream implementation).

This rule does not have an autofix as it is unknown where the ContextVar next used (and it might not be within the same file).

Closes #12054

Test Plan

cargo nextest run

@tjkuson tjkuson marked this pull request as ready for review June 30, 2024 21:46
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@charliermarsh charliermarsh added rule Implementing or modifying a lint rule preview Related to preview mode features labels Jul 1, 2024
@charliermarsh charliermarsh enabled auto-merge (squash) July 1, 2024 01:52
@charliermarsh charliermarsh merged commit d80a9d9 into astral-sh:main Jul 1, 2024
17 checks passed
@tjkuson tjkuson deleted the feat/B039 branch July 5, 2024 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New check: B039
2 participants