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.

Sign up for GitHub

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

Jump to bottom

[flake8-bugbear] Fix mutable-contextvar-default (B039) to resolve annotated function calls properly #14532

Merged
merged 5 commits into from
Nov 24, 2024
Merged

[flake8-bugbear] Fix mutable-contextvar-default (B039) to resolve annotated function calls properly #14532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
782044b
Add test cases
harupy Nov 22, 2024
ff6355f
Apply map_subscript
harupy Nov 22, 2024
ed61306
Fix
harupy Nov 22, 2024
2bbcc3d
Update snapshots
harupy Nov 22, 2024
f890196
RRevert snapshot_kind
harupy Nov 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B039.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
ContextVar("cv", default=MappingProxyType({}))
ContextVar("cv", default=re.compile("foo"))
ContextVar("cv", default=float(1))
ContextVar("cv", default=frozenset[str]())
ContextVar[frozenset[str]]("cv", default=frozenset[str]())

# Bad
ContextVar("cv", default=[])
Expand All @@ -25,6 +27,8 @@
ContextVar("cv", default={char for char in "foo"})
ContextVar("cv", default={char: idx for idx, char in enumerate("foo")})
ContextVar("cv", default=collections.deque())
ContextVar("cv", default=set[str]())
ContextVar[set[str]]("cv", default=set[str]())

def bar() -> list[int]:
return [1, 2, 3]
Expand Down
3 changes: 2 additions & 1 deletion crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_contextvar_default.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::map_subscript;
use ruff_python_ast::name::QualifiedName;
use ruff_python_ast::{self as ast, Expr};
use ruff_python_semantic::analyze::typing::{is_immutable_func, is_mutable_expr, is_mutable_func};
Expand Down Expand Up @@ -96,7 +97,7 @@ pub(crate) fn mutable_contextvar_default(checker: &mut Checker, call: &ast::Expr
&& !is_immutable_func(func, checker.semantic(), &extend_immutable_calls)))
&& checker
.semantic()
.resolve_qualified_name(&call.func)
.resolve_qualified_name(map_subscript(&call.func))
Copy link
Contributor Author

@harupy harupy Nov 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Unrelated to this PR, but shoud we check if call.func is contextvars.Contextvar first (and exit if not) before checking if arguments has a keyword default to avoid unnecessary work (immutability check and extending the immutable expressions)?

.is_some_and(|qualified_name| {
matches!(qualified_name.segments(), ["contextvars", "ContextVar"])
})
Expand Down
152 changes: 87 additions & 65 deletions ...les/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B039_B039.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,127 +2,149 @@
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
snapshot_kind: text
---
B039.py:19:26: B039 Do not use mutable data structures for `ContextVar` defaults
B039.py:21:26: B039 Do not use mutable data structures for `ContextVar` defaults
|
18 | # Bad
19 | ContextVar("cv", default=[])
20 | # Bad
21 | ContextVar("cv", default=[])
| ^^ B039
20 | ContextVar("cv", default={})
21 | ContextVar("cv", default=list())
22 | ContextVar("cv", default={})
23 | ContextVar("cv", default=list())
|
= help: Replace with `None`; initialize with `.set()``

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

B039.py:21:26: B039 Do not use mutable data structures for `ContextVar` defaults
B039.py:23:26: B039 Do not use mutable data structures for `ContextVar` defaults
|
19 | ContextVar("cv", default=[])
20 | ContextVar("cv", default={})
21 | ContextVar("cv", default=list())
21 | ContextVar("cv", default=[])
22 | ContextVar("cv", default={})
23 | ContextVar("cv", default=list())
| ^^^^^^ B039
22 | ContextVar("cv", default=set())
23 | ContextVar("cv", default=dict())
24 | ContextVar("cv", default=set())
25 | ContextVar("cv", default=dict())
|
= help: Replace with `None`; initialize with `.set()``

B039.py:22:26: B039 Do not use mutable data structures for `ContextVar` defaults
B039.py:24:26: B039 Do not use mutable data structures for `ContextVar` defaults
|
20 | ContextVar("cv", default={})
21 | ContextVar("cv", default=list())
22 | ContextVar("cv", default=set())
22 | ContextVar("cv", default={})
23 | ContextVar("cv", default=list())
24 | ContextVar("cv", default=set())
| ^^^^^ B039
23 | ContextVar("cv", default=dict())
24 | ContextVar("cv", default=[char for char in "foo"])
25 | ContextVar("cv", default=dict())
26 | ContextVar("cv", default=[char for char in "foo"])
|
= help: Replace with `None`; initialize with `.set()``

B039.py:23:26: B039 Do not use mutable data structures for `ContextVar` defaults
B039.py:25:26: B039 Do not use mutable data structures for `ContextVar` defaults
|
21 | ContextVar("cv", default=list())
22 | ContextVar("cv", default=set())
23 | ContextVar("cv", default=dict())
23 | ContextVar("cv", default=list())
24 | ContextVar("cv", default=set())
25 | ContextVar("cv", default=dict())
| ^^^^^^ B039
24 | ContextVar("cv", default=[char for char in "foo"])
25 | ContextVar("cv", default={char for char in "foo"})
26 | ContextVar("cv", default=[char for char in "foo"])
27 | ContextVar("cv", default={char for char in "foo"})
|
= help: Replace with `None`; initialize with `.set()``

B039.py:24:26: B039 Do not use mutable data structures for `ContextVar` defaults
B039.py:26:26: B039 Do not use mutable data structures for `ContextVar` defaults
|
22 | ContextVar("cv", default=set())
23 | ContextVar("cv", default=dict())
24 | ContextVar("cv", default=[char for char in "foo"])
24 | ContextVar("cv", default=set())
25 | ContextVar("cv", default=dict())
26 | ContextVar("cv", default=[char for char in "foo"])
| ^^^^^^^^^^^^^^^^^^^^^^^^ B039
25 | ContextVar("cv", default={char for char in "foo"})
26 | ContextVar("cv", default={char: idx for idx, char in enumerate("foo")})
27 | ContextVar("cv", default={char for char in "foo"})
28 | ContextVar("cv", default={char: idx for idx, char in enumerate("foo")})
|
= help: Replace with `None`; initialize with `.set()``

B039.py:25:26: B039 Do not use mutable data structures for `ContextVar` defaults
B039.py:27:26: B039 Do not use mutable data structures for `ContextVar` defaults
|
23 | ContextVar("cv", default=dict())
24 | ContextVar("cv", default=[char for char in "foo"])
25 | ContextVar("cv", default={char for char in "foo"})
25 | ContextVar("cv", default=dict())
26 | ContextVar("cv", default=[char for char in "foo"])
27 | ContextVar("cv", default={char for char in "foo"})
| ^^^^^^^^^^^^^^^^^^^^^^^^ B039
26 | ContextVar("cv", default={char: idx for idx, char in enumerate("foo")})
27 | ContextVar("cv", default=collections.deque())
28 | ContextVar("cv", default={char: idx for idx, char in enumerate("foo")})
29 | ContextVar("cv", default=collections.deque())
|
= help: Replace with `None`; initialize with `.set()``

B039.py:26:26: B039 Do not use mutable data structures for `ContextVar` defaults
B039.py:28:26: B039 Do not use mutable data structures for `ContextVar` defaults
|
24 | ContextVar("cv", default=[char for char in "foo"])
25 | ContextVar("cv", default={char for char in "foo"})
26 | ContextVar("cv", default={char: idx for idx, char in enumerate("foo")})
26 | ContextVar("cv", default=[char for char in "foo"])
27 | ContextVar("cv", default={char for char in "foo"})
28 | ContextVar("cv", default={char: idx for idx, char in enumerate("foo")})
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B039
27 | ContextVar("cv", default=collections.deque())
29 | ContextVar("cv", default=collections.deque())
30 | ContextVar("cv", default=set[str]())
|
= help: Replace with `None`; initialize with `.set()``

B039.py:27:26: B039 Do not use mutable data structures for `ContextVar` defaults
B039.py:29:26: B039 Do not use mutable data structures for `ContextVar` defaults
|
25 | ContextVar("cv", default={char for char in "foo"})
26 | ContextVar("cv", default={char: idx for idx, char in enumerate("foo")})
27 | ContextVar("cv", default=collections.deque())
27 | ContextVar("cv", default={char for char in "foo"})
28 | ContextVar("cv", default={char: idx for idx, char in enumerate("foo")})
29 | ContextVar("cv", default=collections.deque())
| ^^^^^^^^^^^^^^^^^^^ B039
28 |
29 | def bar() -> list[int]:
30 | ContextVar("cv", default=set[str]())
31 | ContextVar[set[str]]("cv", default=set[str]())
|
= help: Replace with `None`; initialize with `.set()``

B039.py:30:26: B039 Do not use mutable data structures for `ContextVar` defaults
|
28 | ContextVar("cv", default={char: idx for idx, char in enumerate("foo")})
29 | ContextVar("cv", default=collections.deque())
30 | ContextVar("cv", default=set[str]())
| ^^^^^^^^^^ B039
31 | ContextVar[set[str]]("cv", default=set[str]())
|
= help: Replace with `None`; initialize with `.set()``

B039.py:32:26: B039 Do not use mutable data structures for `ContextVar` defaults
B039.py:31:36: B039 Do not use mutable data structures for `ContextVar` defaults
|
30 | return [1, 2, 3]
31 |
32 | ContextVar("cv", default=bar())
29 | ContextVar("cv", default=collections.deque())
30 | ContextVar("cv", default=set[str]())
31 | ContextVar[set[str]]("cv", default=set[str]())
| ^^^^^^^^^^ B039
32 |
33 | def bar() -> list[int]:
|
= help: Replace with `None`; initialize with `.set()``

B039.py:36:26: B039 Do not use mutable data structures for `ContextVar` defaults
|
34 | return [1, 2, 3]
35 |
36 | ContextVar("cv", default=bar())
| ^^^^^ B039
33 | ContextVar("cv", default=time.time())
37 | ContextVar("cv", default=time.time())
|
= help: Replace with `None`; initialize with `.set()``

B039.py:33:26: B039 Do not use mutable data structures for `ContextVar` defaults
B039.py:37:26: B039 Do not use mutable data structures for `ContextVar` defaults
|
32 | ContextVar("cv", default=bar())
33 | ContextVar("cv", default=time.time())
36 | ContextVar("cv", default=bar())
37 | ContextVar("cv", default=time.time())
| ^^^^^^^^^^^ B039
34 |
35 | def baz(): ...
38 |
39 | def baz(): ...
|
= help: Replace with `None`; initialize with `.set()``

B039.py:36:26: B039 Do not use mutable data structures for `ContextVar` defaults
B039.py:40:26: B039 Do not use mutable data structures for `ContextVar` defaults
|
35 | def baz(): ...
36 | ContextVar("cv", default=baz())
39 | def baz(): ...
40 | ContextVar("cv", default=baz())
| ^^^^^ B039
|
= help: Replace with `None`; initialize with `.set()``
4 changes: 2 additions & 2 deletions crates/ruff_python_semantic/src/analyze/typing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ pub fn is_immutable_func(
extend_immutable_calls: &[QualifiedName],
) -> bool {
semantic
.resolve_qualified_name(func)
.resolve_qualified_name(map_subscript(func))
.is_some_and(|qualified_name| {
is_immutable_return_type(qualified_name.segments())
|| extend_immutable_calls
Expand All @@ -306,7 +306,7 @@ pub fn is_mutable_expr(expr: &Expr, semantic: &SemanticModel) -> bool {
| Expr::ListComp(_)
| Expr::DictComp(_)
| Expr::SetComp(_) => true,
Expr::Call(ast::ExprCall { func, .. }) => is_mutable_func(func, semantic),
Expr::Call(ast::ExprCall { func, .. }) => is_mutable_func(map_subscript(func), semantic),
_ => false,
}
}
Expand Down
Loading