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

B039 considers frozenset to be mutable #14525

Closed
layday opened this issue Nov 22, 2024 · 8 comments · Fixed by #14532
Closed

B039 considers frozenset to be mutable #14525

layday opened this issue Nov 22, 2024 · 8 comments · Fixed by #14532
Assignees
Labels
bug Something isn't working

Comments

@layday
Copy link

layday commented Nov 22, 2024

$ cat foo.py 
from contextvars import ContextVar

foo = ContextVar('foo', default=frozenset[str]())
$ ruff check --select B039 foo.py
foo.py:3:33: B039 Do not use mutable data structures for `ContextVar` defaults
  |
1 | from contextvars import ContextVar
2 | 
3 | foo = ContextVar('foo', default=frozenset[str]())
  |                                 ^^^^^^^^^^^^^^^^ B039
  |
  = help: Replace with `None`; initialize with `.set()``

Found 1 error.
$ ruff --version                 
ruff 0.8.0
@layday
Copy link
Author

layday commented Nov 22, 2024

Also appears to be the case for tuple, and only if they are parametrised, i.e. frozenset() is accepted but not frozenset[type]().

@layday
Copy link
Author

layday commented Nov 22, 2024

Conversely, the error is silenced if the context var is an annotated mutable type:

$ cat foo.py
from contextvars import ContextVar

foo = ContextVar[set[str]]('foo', default=set())  # No error
bar = ContextVar('bar', default=set())
$ ruff check --select B039 foo.py
foo.py:4:33: B039 Do not use mutable data structures for `ContextVar` defaults
  |
3 | foo = ContextVar[set[str]]('foo', default=set())  # No error
4 | bar = ContextVar('bar', default=set())
  |                                 ^^^^^ B039
  |
  = help: Replace with `None`; initialize with `.set()``

Found 1 error.

@layday
Copy link
Author

layday commented Nov 22, 2024

To recap:

from contextvars import ContextVar


# All of these should produce an error but foo3 and foo4 don't.

foo1 = ContextVar('foo1', default=set())
foo2 = ContextVar('foo2', default=set[object]())
foo3 = ContextVar[set[object]]('foo3', default=set())
foo4 = ContextVar[set[object]]('foo4', default=set[object]())
foo5: ContextVar[set[object]] = ContextVar('foo5', default=set())
foo6: ContextVar[set[object]] = ContextVar('foo6', default=set[object]())


# None of these should produce an error but bar2 and bar6 do.

bar1 = ContextVar('bar1', default=frozenset())
bar2 = ContextVar('bar2', default=frozenset[object]())
bar3 = ContextVar[frozenset[object]]('bar3', default=frozenset())
bar4 = ContextVar[frozenset[object]]('bar4', default=frozenset[object]())
bar5: ContextVar[frozenset[object]] = ContextVar('bar5', default=frozenset())
bar6: ContextVar[frozenset[object]] = ContextVar('bar6', default=frozenset[object]())

@AlexWaygood AlexWaygood added the bug Something isn't working label Nov 22, 2024
@harupy
Copy link
Contributor

harupy commented Nov 22, 2024

@AlexWaygood can I work on this?

@AlexWaygood
Copy link
Member

@harupy sure!

@AlexWaygood
Copy link
Member

@harupy this function will probably be helpful in fixing some of the issues here:

/// Given an [`Expr`] that can be a [`ExprSubscript`][ast::ExprSubscript] or not
/// (like an annotation that may be generic or not), return the underlying expr.
pub fn map_subscript(expr: &Expr) -> &Expr {
if let Expr::Subscript(ast::ExprSubscript { value, .. }) = expr {
// Ex) `Iterable[T]` => return `Iterable`
value
} else {
// Ex) `Iterable` => return `Iterable`
expr
}
}

@harupy
Copy link
Contributor

harupy commented Nov 22, 2024

@AlexWaygood Thanks!

@harupy
Copy link
Contributor

harupy commented Nov 22, 2024

Filed #14532

charliermarsh pushed a commit that referenced this issue Nov 24, 2024
… annotated function calls properly (#14532)

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

Fix #14525

## Test Plan

<!-- How was it tested? -->

New test cases

---------

Signed-off-by: harupy <hkawamura0130@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants