Skip to content

Commit

Permalink
Allow __slots__ assignments in mutable-class-default (#5314)
Browse files Browse the repository at this point in the history
Closes #5309.
  • Loading branch information
charliermarsh authored Jun 22, 2023
1 parent 1c2be54 commit 5f88ff8
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 54 deletions.
18 changes: 3 additions & 15 deletions crates/ruff/resources/test/fixtures/ruff/RUF012.py
Original file line number Diff line number Diff line change
@@ -1,23 +1,14 @@
import typing
from typing import ClassVar, Sequence, Final

KNOWINGLY_MUTABLE_DEFAULT = []


class A:
mutable_default: list[int] = []
immutable_annotation: typing.Sequence[int] = []
without_annotation = []
correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
class_variable: typing.ClassVar[list[int]] = []
final_variable: typing.Final[list[int]] = []
__slots__ = {
"mutable_default": "A mutable default value",
}


class B:
mutable_default: list[int] = []
immutable_annotation: Sequence[int] = []
without_annotation = []
correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
class_variable: ClassVar[list[int]] = []
final_variable: Final[list[int]] = []

Expand All @@ -30,7 +21,6 @@ class C:
mutable_default: list[int] = []
immutable_annotation: Sequence[int] = []
without_annotation = []
correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
perfectly_fine: list[int] = field(default_factory=list)
class_variable: ClassVar[list[int]] = []
final_variable: Final[list[int]] = []
Expand All @@ -43,7 +33,5 @@ class D(BaseModel):
mutable_default: list[int] = []
immutable_annotation: Sequence[int] = []
without_annotation = []
correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
perfectly_fine: list[int] = field(default_factory=list)
class_variable: ClassVar[list[int]] = []
final_variable: Final[list[int]] = []
16 changes: 16 additions & 0 deletions crates/ruff/src/rules/ruff/rules/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,22 @@ use rustpython_parser::ast::{self, Expr};
use ruff_python_ast::helpers::map_callable;
use ruff_python_semantic::SemanticModel;

/// Return `true` if the given [`Expr`] is a special class attribute, like `__slots__`.
///
/// While `__slots__` is typically defined via a tuple, Python accepts any iterable and, in
/// particular, allows the use of a dictionary to define the attribute names (as keys) and
/// docstrings (as values).
pub(super) fn is_special_attribute(value: &Expr) -> bool {
if let Expr::Name(ast::ExprName { id, .. }) = value {
matches!(
id.as_str(),
"__slots__" | "__dict__" | "__weakref__" | "__annotations__"
)
} else {
false
}
}

/// Returns `true` if the given [`Expr`] is a `dataclasses.field` call.
pub(super) fn is_dataclass_field(func: &Expr, semantic: &SemanticModel) -> bool {
semantic.resolve_call_path(func).map_or(false, |call_path| {
Expand Down
11 changes: 8 additions & 3 deletions crates/ruff/src/rules/ruff/rules/mutable_class_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use ruff_python_semantic::analyze::typing::{is_immutable_annotation, is_mutable_
use crate::checkers::ast::Checker;
use crate::rules::ruff::rules::helpers::{
is_class_var_annotation, is_dataclass, is_final_annotation, is_pydantic_model,
is_special_attribute,
};

/// ## What it does
Expand Down Expand Up @@ -51,10 +52,12 @@ pub(crate) fn mutable_class_default(checker: &mut Checker, class_def: &ast::Stmt
match statement {
Stmt::AnnAssign(ast::StmtAnnAssign {
annotation,
target,
value: Some(value),
..
}) => {
if is_mutable_expr(value, checker.semantic())
if !is_special_attribute(target)
&& is_mutable_expr(value, checker.semantic())
&& !is_class_var_annotation(annotation, checker.semantic())
&& !is_final_annotation(annotation, checker.semantic())
&& !is_immutable_annotation(annotation, checker.semantic())
Expand All @@ -70,8 +73,10 @@ pub(crate) fn mutable_class_default(checker: &mut Checker, class_def: &ast::Stmt
.push(Diagnostic::new(MutableClassDefault, value.range()));
}
}
Stmt::Assign(ast::StmtAssign { value, .. }) => {
if is_mutable_expr(value, checker.semantic()) {
Stmt::Assign(ast::StmtAssign { value, targets, .. }) => {
if !targets.iter().all(is_special_attribute)
&& is_mutable_expr(value, checker.semantic())
{
// Avoid Pydantic models, which end up copying defaults on instance creation.
if is_pydantic_model(class_def, checker.semantic()) {
return;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,52 +1,34 @@
---
source: crates/ruff/src/rules/ruff/mod.rs
---
RUF012.py:8:34: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
RUF012.py:9:34: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
|
7 | class A:
8 | mutable_default: list[int] = []
7 | }
8 |
9 | mutable_default: list[int] = []
| ^^ RUF012
9 | immutable_annotation: typing.Sequence[int] = []
10 | without_annotation = []
10 | immutable_annotation: Sequence[int] = []
11 | without_annotation = []
|

RUF012.py:10:26: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
RUF012.py:11:26: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
|
8 | mutable_default: list[int] = []
9 | immutable_annotation: typing.Sequence[int] = []
10 | without_annotation = []
9 | mutable_default: list[int] = []
10 | immutable_annotation: Sequence[int] = []
11 | without_annotation = []
| ^^ RUF012
11 | correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
12 | class_variable: typing.ClassVar[list[int]] = []
12 | class_variable: ClassVar[list[int]] = []
13 | final_variable: Final[list[int]] = []
|

RUF012.py:17:34: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
RUF012.py:23:26: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
|
16 | class B:
17 | mutable_default: list[int] = []
| ^^ RUF012
18 | immutable_annotation: Sequence[int] = []
19 | without_annotation = []
|

RUF012.py:19:26: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
|
17 | mutable_default: list[int] = []
18 | immutable_annotation: Sequence[int] = []
19 | without_annotation = []
| ^^ RUF012
20 | correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
21 | class_variable: ClassVar[list[int]] = []
|

RUF012.py:32:26: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
|
30 | mutable_default: list[int] = []
31 | immutable_annotation: Sequence[int] = []
32 | without_annotation = []
21 | mutable_default: list[int] = []
22 | immutable_annotation: Sequence[int] = []
23 | without_annotation = []
| ^^ RUF012
33 | correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
34 | perfectly_fine: list[int] = field(default_factory=list)
24 | perfectly_fine: list[int] = field(default_factory=list)
25 | class_variable: ClassVar[list[int]] = []
|


0 comments on commit 5f88ff8

Please sign in to comment.