mirror of https://github.com/astral-sh/ruff
[`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 <brentrwestbrook@gmail.com>
This commit is contained in:
parent
e4a32ba644
commit
b1e354bd99
|
|
@ -132,3 +132,9 @@ class AWithQuotes:
|
||||||
final_variable: 'Final[list[int]]' = []
|
final_variable: 'Final[list[int]]' = []
|
||||||
class_variable_without_subscript: 'ClassVar' = []
|
class_variable_without_subscript: 'ClassVar' = []
|
||||||
final_variable_without_subscript: 'Final' = []
|
final_variable_without_subscript: 'Final' = []
|
||||||
|
|
||||||
|
|
||||||
|
# Reassignment of a ClassVar should not trigger RUF012
|
||||||
|
class P:
|
||||||
|
class_variable: ClassVar[list] = [10, 20, 30, 40, 50]
|
||||||
|
class_variable = [*class_variable[0::1], *class_variable[2::3]]
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
use ruff_python_ast::{self as ast, Stmt};
|
use rustc_hash::FxHashSet;
|
||||||
|
|
||||||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
||||||
|
use ruff_python_ast::{self as ast, Stmt};
|
||||||
use ruff_python_semantic::analyze::typing::{is_immutable_annotation, is_mutable_expr};
|
use ruff_python_semantic::analyze::typing::{is_immutable_annotation, is_mutable_expr};
|
||||||
use ruff_text_size::Ranged;
|
use ruff_text_size::Ranged;
|
||||||
|
|
||||||
|
|
@ -96,6 +97,9 @@ impl Violation for MutableClassDefault {
|
||||||
|
|
||||||
/// RUF012
|
/// RUF012
|
||||||
pub(crate) fn mutable_class_default(checker: &Checker, class_def: &ast::StmtClassDef) {
|
pub(crate) fn mutable_class_default(checker: &Checker, class_def: &ast::StmtClassDef) {
|
||||||
|
// Collect any `ClassVar`s we find in case they get reassigned later.
|
||||||
|
let mut class_var_targets = FxHashSet::default();
|
||||||
|
|
||||||
for statement in &class_def.body {
|
for statement in &class_def.body {
|
||||||
match statement {
|
match statement {
|
||||||
Stmt::AnnAssign(ast::StmtAnnAssign {
|
Stmt::AnnAssign(ast::StmtAnnAssign {
|
||||||
|
|
@ -104,6 +108,12 @@ pub(crate) fn mutable_class_default(checker: &Checker, class_def: &ast::StmtClas
|
||||||
value: Some(value),
|
value: Some(value),
|
||||||
..
|
..
|
||||||
}) => {
|
}) => {
|
||||||
|
if let ast::Expr::Name(ast::ExprName { id, .. }) = target.as_ref() {
|
||||||
|
if is_class_var_annotation(annotation, checker.semantic()) {
|
||||||
|
class_var_targets.insert(id);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if !is_special_attribute(target)
|
if !is_special_attribute(target)
|
||||||
&& is_mutable_expr(value, checker.semantic())
|
&& is_mutable_expr(value, checker.semantic())
|
||||||
&& !is_class_var_annotation(annotation, checker.semantic())
|
&& !is_class_var_annotation(annotation, checker.semantic())
|
||||||
|
|
@ -123,8 +133,12 @@ pub(crate) fn mutable_class_default(checker: &Checker, class_def: &ast::StmtClas
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Stmt::Assign(ast::StmtAssign { value, targets, .. }) => {
|
Stmt::Assign(ast::StmtAssign { value, targets, .. }) => {
|
||||||
if !targets.iter().all(is_special_attribute)
|
if !targets.iter().all(|target| {
|
||||||
&& is_mutable_expr(value, checker.semantic())
|
is_special_attribute(target)
|
||||||
|
|| target
|
||||||
|
.as_name_expr()
|
||||||
|
.is_some_and(|name| class_var_targets.contains(&name.id))
|
||||||
|
}) && is_mutable_expr(value, checker.semantic())
|
||||||
{
|
{
|
||||||
// Avoid, e.g., Pydantic and msgspec models, which end up copying defaults on instance creation.
|
// Avoid, e.g., Pydantic and msgspec models, which end up copying defaults on instance creation.
|
||||||
if has_default_copy_semantics(class_def, checker.semantic()) {
|
if has_default_copy_semantics(class_def, checker.semantic()) {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue