diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B039.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B039.py new file mode 100644 index 0000000000000..7d96075f15697 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B039.py @@ -0,0 +1,36 @@ +from contextvars import ContextVar +from types import MappingProxyType +import re +import collections +import time + +# Okay +ContextVar("cv") +ContextVar("cv", default=()) +ContextVar("cv", default=(1, 2, 3)) +ContextVar("cv", default="foo") +ContextVar("cv", default=tuple()) +ContextVar("cv", default=frozenset()) +ContextVar("cv", default=MappingProxyType({})) +ContextVar("cv", default=re.compile("foo")) +ContextVar("cv", default=float(1)) + +# Bad +ContextVar("cv", default=[]) +ContextVar("cv", default={}) +ContextVar("cv", default=list()) +ContextVar("cv", default=set()) +ContextVar("cv", default=dict()) +ContextVar("cv", default=[char for char in "foo"]) +ContextVar("cv", default={char for char in "foo"}) +ContextVar("cv", default={char: idx for idx, char in enumerate("foo")}) +ContextVar("cv", default=collections.deque()) + +def bar() -> list[int]: + return [1, 2, 3] + +ContextVar("cv", default=bar()) +ContextVar("cv", default=time.time()) + +def baz(): ... +ContextVar("cv", default=baz()) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B039_extended.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B039_extended.py new file mode 100644 index 0000000000000..71d46cc7a5b0f --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B039_extended.py @@ -0,0 +1,7 @@ +from contextvars import ContextVar + +from fastapi import Query +ContextVar("cv", default=Query(None)) + +from something_else import Depends +ContextVar("cv", default=Depends()) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index a86fdc2bb1bbe..5e362d96ae2bc 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -580,6 +580,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::NoExplicitStacklevel) { flake8_bugbear::rules::no_explicit_stacklevel(checker, call); } + if checker.enabled(Rule::MutableContextvarDefault) { + flake8_bugbear::rules::mutable_contextvar_default(checker, call); + } if checker.enabled(Rule::UnnecessaryDictKwargs) { flake8_pie::rules::unnecessary_dict_kwargs(checker, call); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 4ca985e2d3b77..70bbaab4ebf94 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -345,6 +345,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Bugbear, "033") => (RuleGroup::Stable, rules::flake8_bugbear::rules::DuplicateValue), (Flake8Bugbear, "034") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ReSubPositionalArgs), (Flake8Bugbear, "035") => (RuleGroup::Stable, rules::flake8_bugbear::rules::StaticKeyDictComprehension), + (Flake8Bugbear, "039") => (RuleGroup::Preview, rules::flake8_bugbear::rules::MutableContextvarDefault), (Flake8Bugbear, "901") => (RuleGroup::Preview, rules::flake8_bugbear::rules::ReturnInGenerator), (Flake8Bugbear, "904") => (RuleGroup::Stable, rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept), (Flake8Bugbear, "905") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ZipWithoutExplicitStrict), diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs b/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs index 2fa8d5b7a67aa..1f122f15438b3 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs @@ -64,6 +64,7 @@ mod tests { #[test_case(Rule::UselessExpression, Path::new("B018.py"))] #[test_case(Rule::ReturnInGenerator, Path::new("B901.py"))] #[test_case(Rule::LoopIteratorMutation, Path::new("B909.py"))] + #[test_case(Rule::MutableContextvarDefault, Path::new("B039.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( @@ -124,4 +125,20 @@ mod tests { assert_messages!(snapshot, diagnostics); Ok(()) } + + #[test] + fn extend_mutable_contextvar_default() -> Result<()> { + let snapshot = "extend_mutable_contextvar_default".to_string(); + let diagnostics = test_path( + Path::new("flake8_bugbear/B039_extended.py"), + &LinterSettings { + flake8_bugbear: super::settings::Settings { + extend_immutable_calls: vec!["fastapi.Query".to_string()], + }, + ..LinterSettings::for_rule(Rule::MutableContextvarDefault) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } } diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs index 4f7fd0eebf42b..b4af7755dd435 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs @@ -15,6 +15,7 @@ pub(crate) use jump_statement_in_finally::*; pub(crate) use loop_iterator_mutation::*; pub(crate) use loop_variable_overrides_iterator::*; pub(crate) use mutable_argument_default::*; +pub(crate) use mutable_contextvar_default::*; pub(crate) use no_explicit_stacklevel::*; pub(crate) use raise_literal::*; pub(crate) use raise_without_from_inside_except::*; @@ -52,6 +53,7 @@ mod jump_statement_in_finally; mod loop_iterator_mutation; mod loop_variable_overrides_iterator; mod mutable_argument_default; +mod mutable_contextvar_default; mod no_explicit_stacklevel; mod raise_literal; mod raise_without_from_inside_except; diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_contextvar_default.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_contextvar_default.rs new file mode 100644 index 0000000000000..c6dc1394a14f3 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_contextvar_default.rs @@ -0,0 +1,108 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +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}; +use ruff_python_semantic::Modules; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for uses of mutable objects as `ContextVar` defaults. +/// +/// ## Why is this bad? +/// +/// The `ContextVar` default is evaluated once, when the `ContextVar` is defined. +/// +/// The same mutable object is then shared across all `.get()` method calls to +/// the `ContextVar`. If the object is modified, those modifications will persist +/// across calls, which can lead to unexpected behavior. +/// +/// Instead, prefer to use immutable data structures; or, take `None` as a +/// default, and initialize a new mutable object inside for each call using the +/// `.set()` method. +/// +/// Types outside the standard library can be marked as immutable with the +/// [`lint.flake8-bugbear.extend-immutable-calls`] configuration option. +/// +/// ## Example +/// ```python +/// from contextvars import ContextVar +/// +/// +/// cv: ContextVar[list] = ContextVar("cv", default=[]) +/// ``` +/// +/// Use instead: +/// ```python +/// from contextvars import ContextVar +/// +/// +/// cv: ContextVar[list | None] = ContextVar("cv", default=None) +/// +/// ... +/// +/// if cv.get() is None: +/// cv.set([]) +/// ``` +/// +/// ## Options +/// - `lint.flake8-bugbear.extend-immutable-calls` +/// +/// ## References +/// - [Python documentation: [`contextvars` — Context Variables](https://docs.python.org/3/library/contextvars.html) +#[violation] +pub struct MutableContextvarDefault; + +impl Violation for MutableContextvarDefault { + #[derive_message_formats] + fn message(&self) -> String { + format!("Do not use mutable data structures for `ContextVar` defaults") + } + + fn fix_title(&self) -> Option { + Some("Replace with `None`; initialize with `.set()``".to_string()) + } +} + +/// B039 +pub(crate) fn mutable_contextvar_default(checker: &mut Checker, call: &ast::ExprCall) { + if !checker.semantic().seen_module(Modules::CONTEXTVARS) { + return; + } + + let Some(default) = call + .arguments + .find_keyword("default") + .map(|keyword| &keyword.value) + else { + return; + }; + + let extend_immutable_calls: Vec = checker + .settings + .flake8_bugbear + .extend_immutable_calls + .iter() + .map(|target| QualifiedName::from_dotted_name(target)) + .collect(); + + if (is_mutable_expr(default, checker.semantic()) + || matches!( + default, + Expr::Call(ast::ExprCall { func, .. }) + if !is_mutable_func(func, checker.semantic()) + && !is_immutable_func(func, checker.semantic(), &extend_immutable_calls))) + && checker + .semantic() + .resolve_qualified_name(&call.func) + .is_some_and(|qualified_name| { + matches!(qualified_name.segments(), ["contextvars", "ContextVar"]) + }) + { + checker + .diagnostics + .push(Diagnostic::new(MutableContextvarDefault, default.range())); + } +} diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B039_B039.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B039_B039.py.snap new file mode 100644 index 0000000000000..e2bbb851aa3c8 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B039_B039.py.snap @@ -0,0 +1,127 @@ +--- +source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs +--- +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()`` + +B039.py:20:26: B039 Do not use mutable data structures for `ContextVar` defaults + | +18 | # Bad +19 | ContextVar("cv", default=[]) +20 | ContextVar("cv", default={}) + | ^^ B039 +21 | ContextVar("cv", default=list()) +22 | 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 + | +19 | ContextVar("cv", default=[]) +20 | ContextVar("cv", default={}) +21 | ContextVar("cv", default=list()) + | ^^^^^^ B039 +22 | ContextVar("cv", default=set()) +23 | 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 + | +20 | ContextVar("cv", default={}) +21 | ContextVar("cv", default=list()) +22 | ContextVar("cv", default=set()) + | ^^^^^ B039 +23 | ContextVar("cv", default=dict()) +24 | 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 + | +21 | ContextVar("cv", default=list()) +22 | ContextVar("cv", default=set()) +23 | ContextVar("cv", default=dict()) + | ^^^^^^ B039 +24 | ContextVar("cv", default=[char for char in "foo"]) +25 | 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 + | +22 | ContextVar("cv", default=set()) +23 | ContextVar("cv", default=dict()) +24 | 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")}) + | + = help: Replace with `None`; initialize with `.set()`` + +B039.py:25: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"}) + | ^^^^^^^^^^^^^^^^^^^^^^^^ B039 +26 | ContextVar("cv", default={char: idx for idx, char in enumerate("foo")}) +27 | 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 + | +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")}) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B039 +27 | ContextVar("cv", default=collections.deque()) + | + = help: Replace with `None`; initialize with `.set()`` + +B039.py:27: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()) + | ^^^^^^^^^^^^^^^^^^^ B039 +28 | +29 | def bar() -> list[int]: + | + = help: Replace with `None`; initialize with `.set()`` + +B039.py:32:26: B039 Do not use mutable data structures for `ContextVar` defaults + | +30 | return [1, 2, 3] +31 | +32 | ContextVar("cv", default=bar()) + | ^^^^^ B039 +33 | 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 + | +32 | ContextVar("cv", default=bar()) +33 | ContextVar("cv", default=time.time()) + | ^^^^^^^^^^^ B039 +34 | +35 | def baz(): ... + | + = help: Replace with `None`; initialize with `.set()`` + +B039.py:36:26: B039 Do not use mutable data structures for `ContextVar` defaults + | +35 | def baz(): ... +36 | ContextVar("cv", default=baz()) + | ^^^^^ B039 + | + = help: Replace with `None`; initialize with `.set()`` diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__extend_mutable_contextvar_default.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__extend_mutable_contextvar_default.snap new file mode 100644 index 0000000000000..d03f11124f91f --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__extend_mutable_contextvar_default.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs +--- +B039_extended.py:7:26: B039 Do not use mutable data structures for `ContextVar` defaults + | +6 | from something_else import Depends +7 | ContextVar("cv", default=Depends()) + | ^^^^^^^^^ B039 + | + = help: Replace with `None`; initialize with `.set()`` diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index e567a5e936f84..6a813c55c2ccd 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -1233,6 +1233,7 @@ impl<'a> SemanticModel<'a> { "_typeshed" => self.seen.insert(Modules::TYPESHED), "builtins" => self.seen.insert(Modules::BUILTINS), "collections" => self.seen.insert(Modules::COLLECTIONS), + "contextvars" => self.seen.insert(Modules::CONTEXTVARS), "dataclasses" => self.seen.insert(Modules::DATACLASSES), "datetime" => self.seen.insert(Modules::DATETIME), "django" => self.seen.insert(Modules::DJANGO), @@ -1820,6 +1821,7 @@ bitflags! { const TYPESHED = 1 << 16; const DATACLASSES = 1 << 17; const BUILTINS = 1 << 18; + const CONTEXTVARS = 1 << 19; } } diff --git a/ruff.schema.json b/ruff.schema.json index 4c77a7dd895ce..cfe10e3a4cbc7 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2760,6 +2760,7 @@ "B033", "B034", "B035", + "B039", "B9", "B90", "B901",