Skip to content

Commit b1e354b

Browse files
Ruchir28ntBre
andauthored
[ruff] Avoid false positive on ClassVar reassignment (RUF012) (#21478)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary <!-- What's the purpose of the change? What does it do, and why? --> Fixes #21389 Avoid RUF012 false positives when reassigning a ClassVar ## Test Plan <!-- How was it tested? --> Added the new reassignment scenario to `crates/ruff_linter/resources/test/fixtures/ruff/RUF012.py`. --------- Co-authored-by: Brent Westbrook <[email protected]>
1 parent e4a32ba commit b1e354b

File tree

2 files changed

+23
-3
lines changed

2 files changed

+23
-3
lines changed

crates/ruff_linter/resources/test/fixtures/ruff/RUF012.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,3 +132,9 @@ class AWithQuotes:
132132
final_variable: 'Final[list[int]]' = []
133133
class_variable_without_subscript: 'ClassVar' = []
134134
final_variable_without_subscript: 'Final' = []
135+
136+
137+
# Reassignment of a ClassVar should not trigger RUF012
138+
class P:
139+
class_variable: ClassVar[list] = [10, 20, 30, 40, 50]
140+
class_variable = [*class_variable[0::1], *class_variable[2::3]]

crates/ruff_linter/src/rules/ruff/rules/mutable_class_default.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
use ruff_python_ast::{self as ast, Stmt};
1+
use rustc_hash::FxHashSet;
22

33
use ruff_macros::{ViolationMetadata, derive_message_formats};
4+
use ruff_python_ast::{self as ast, Stmt};
45
use ruff_python_semantic::analyze::typing::{is_immutable_annotation, is_mutable_expr};
56
use ruff_text_size::Ranged;
67

@@ -96,6 +97,9 @@ impl Violation for MutableClassDefault {
9697

9798
/// RUF012
9899
pub(crate) fn mutable_class_default(checker: &Checker, class_def: &ast::StmtClassDef) {
100+
// Collect any `ClassVar`s we find in case they get reassigned later.
101+
let mut class_var_targets = FxHashSet::default();
102+
99103
for statement in &class_def.body {
100104
match statement {
101105
Stmt::AnnAssign(ast::StmtAnnAssign {
@@ -104,6 +108,12 @@ pub(crate) fn mutable_class_default(checker: &Checker, class_def: &ast::StmtClas
104108
value: Some(value),
105109
..
106110
}) => {
111+
if let ast::Expr::Name(ast::ExprName { id, .. }) = target.as_ref() {
112+
if is_class_var_annotation(annotation, checker.semantic()) {
113+
class_var_targets.insert(id);
114+
}
115+
}
116+
107117
if !is_special_attribute(target)
108118
&& is_mutable_expr(value, checker.semantic())
109119
&& !is_class_var_annotation(annotation, checker.semantic())
@@ -123,8 +133,12 @@ pub(crate) fn mutable_class_default(checker: &Checker, class_def: &ast::StmtClas
123133
}
124134
}
125135
Stmt::Assign(ast::StmtAssign { value, targets, .. }) => {
126-
if !targets.iter().all(is_special_attribute)
127-
&& is_mutable_expr(value, checker.semantic())
136+
if !targets.iter().all(|target| {
137+
is_special_attribute(target)
138+
|| target
139+
.as_name_expr()
140+
.is_some_and(|name| class_var_targets.contains(&name.id))
141+
}) && is_mutable_expr(value, checker.semantic())
128142
{
129143
// Avoid, e.g., Pydantic and msgspec models, which end up copying defaults on instance creation.
130144
if has_default_copy_semantics(class_def, checker.semantic()) {

0 commit comments

Comments
 (0)