Skip to content

Commit d8b1afb

Browse files
[ruff] Also report problems for attrs dataclasses in preview mode (RUF008, RUF009) (#14327)
Co-authored-by: Alex Waygood <[email protected]>
1 parent 9a3001b commit d8b1afb

12 files changed

+344
-51
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import typing
2+
from typing import ClassVar, Sequence
3+
4+
import attr
5+
from attr import s
6+
from attrs import define, frozen
7+
8+
KNOWINGLY_MUTABLE_DEFAULT = []
9+
10+
11+
@define
12+
class A:
13+
mutable_default: list[int] = []
14+
immutable_annotation: typing.Sequence[int] = []
15+
without_annotation = []
16+
correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
17+
perfectly_fine: list[int] = field(default_factory=list)
18+
class_variable: typing.ClassVar[list[int]] = []
19+
20+
21+
@frozen
22+
class B:
23+
mutable_default: list[int] = []
24+
immutable_annotation: Sequence[int] = []
25+
without_annotation = []
26+
correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
27+
perfectly_fine: list[int] = field(default_factory=list)
28+
class_variable: ClassVar[list[int]] = []
29+
30+
31+
@attr.s
32+
class C:
33+
mutable_default: list[int] = []
34+
immutable_annotation: Sequence[int] = []
35+
without_annotation = []
36+
correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
37+
perfectly_fine: list[int] = field(default_factory=list)
38+
class_variable: ClassVar[list[int]] = []
39+
40+
@s
41+
class D:
42+
mutable_default: list[int] = []
43+
immutable_annotation: Sequence[int] = []
44+
without_annotation = []
45+
correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
46+
perfectly_fine: list[int] = field(default_factory=list)
47+
class_variable: ClassVar[list[int]] = []
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import datetime
2+
import re
3+
import typing
4+
from collections import OrderedDict
5+
from fractions import Fraction
6+
from pathlib import Path
7+
from typing import ClassVar, NamedTuple
8+
9+
import attr
10+
from attrs import Factory, define, field, frozen
11+
12+
13+
def default_function() -> list[int]:
14+
return []
15+
16+
17+
class ImmutableType(NamedTuple):
18+
something: int = 8
19+
20+
21+
@attr.s
22+
class A:
23+
hidden_mutable_default: list[int] = default_function()
24+
class_variable: typing.ClassVar[list[int]] = default_function()
25+
another_class_var: ClassVar[list[int]] = default_function()
26+
27+
fine_path: Path = Path()
28+
fine_date: datetime.date = datetime.date(2042, 1, 1)
29+
fine_timedelta: datetime.timedelta = datetime.timedelta(hours=7)
30+
fine_tuple: tuple[int] = tuple([1])
31+
fine_regex: re.Pattern = re.compile(r".*")
32+
fine_float: float = float("-inf")
33+
fine_int: int = int(12)
34+
fine_complex: complex = complex(1, 2)
35+
fine_str: str = str("foo")
36+
fine_bool: bool = bool("foo")
37+
fine_fraction: Fraction = Fraction(1, 2)
38+
39+
40+
DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES = ImmutableType(40)
41+
DEFAULT_A_FOR_ALL_DATACLASSES = A([1, 2, 3])
42+
43+
44+
@define
45+
class B:
46+
hidden_mutable_default: list[int] = default_function()
47+
another_dataclass: A = A()
48+
not_optimal: ImmutableType = ImmutableType(20)
49+
good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES
50+
okay_variant: A = DEFAULT_A_FOR_ALL_DATACLASSES
51+
52+
fine_dataclass_function: list[int] = field(default_factory=list)
53+
attrs_factory: dict[str, str] = Factory(OrderedDict)
54+
55+
56+
class IntConversionDescriptor:
57+
def __init__(self, *, default):
58+
self._default = default
59+
60+
def __set_name__(self, owner, name):
61+
self._name = "_" + name
62+
63+
def __get__(self, obj, type):
64+
if obj is None:
65+
return self._default
66+
67+
return getattr(obj, self._name, self._default)
68+
69+
def __set__(self, obj, value):
70+
setattr(obj, self._name, int(value))
71+
72+
73+
@frozen
74+
class InventoryItem:
75+
quantity_on_hand: IntConversionDescriptor = IntConversionDescriptor(default=100)

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

+5
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,11 @@ mod tests {
388388

389389
#[test_case(Rule::ZipInsteadOfPairwise, Path::new("RUF007.py"))]
390390
#[test_case(Rule::UnsafeMarkupUse, Path::new("RUF035.py"))]
391+
#[test_case(
392+
Rule::FunctionCallInDataclassDefaultArgument,
393+
Path::new("RUF009_attrs.py")
394+
)]
395+
#[test_case(Rule::MutableDataclassDefault, Path::new("RUF008_attrs.py"))]
391396
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
392397
let snapshot = format!(
393398
"preview__{}_{}",

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

+28-17
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use ruff_text_size::Ranged;
88

99
use crate::checkers::ast::Checker;
1010
use crate::rules::ruff::rules::helpers::{
11-
is_class_var_annotation, is_dataclass, is_dataclass_field, is_descriptor_class,
11+
dataclass_kind, is_class_var_annotation, is_dataclass_field, is_descriptor_class,
1212
};
1313

1414
/// ## What it does
@@ -74,7 +74,13 @@ pub(crate) fn function_call_in_dataclass_default(
7474
checker: &mut Checker,
7575
class_def: &ast::StmtClassDef,
7676
) {
77-
if !is_dataclass(class_def, checker.semantic()) {
77+
let semantic = checker.semantic();
78+
79+
let Some(dataclass_kind) = dataclass_kind(class_def, semantic) else {
80+
return;
81+
};
82+
83+
if dataclass_kind.is_attrs() && checker.settings.preview.is_disabled() {
7884
return;
7985
}
8086

@@ -87,26 +93,31 @@ pub(crate) fn function_call_in_dataclass_default(
8793
.collect();
8894

8995
for statement in &class_def.body {
90-
if let Stmt::AnnAssign(ast::StmtAnnAssign {
96+
let Stmt::AnnAssign(ast::StmtAnnAssign {
9197
annotation,
9298
value: Some(expr),
9399
..
94100
}) = statement
101+
else {
102+
continue;
103+
};
104+
let Expr::Call(ast::ExprCall { func, .. }) = &**expr else {
105+
continue;
106+
};
107+
108+
if is_class_var_annotation(annotation, checker.semantic())
109+
|| is_immutable_func(func, checker.semantic(), &extend_immutable_calls)
110+
|| is_dataclass_field(func, checker.semantic(), dataclass_kind)
111+
|| is_descriptor_class(func, checker.semantic())
95112
{
96-
if let Expr::Call(ast::ExprCall { func, .. }) = expr.as_ref() {
97-
if !is_class_var_annotation(annotation, checker.semantic())
98-
&& !is_immutable_func(func, checker.semantic(), &extend_immutable_calls)
99-
&& !is_dataclass_field(func, checker.semantic())
100-
&& !is_descriptor_class(func, checker.semantic())
101-
{
102-
checker.diagnostics.push(Diagnostic::new(
103-
FunctionCallInDataclassDefaultArgument {
104-
name: UnqualifiedName::from_expr(func).map(|name| name.to_string()),
105-
},
106-
expr.range(),
107-
));
108-
}
109-
}
113+
continue;
110114
}
115+
116+
let kind = FunctionCallInDataclassDefaultArgument {
117+
name: UnqualifiedName::from_expr(func).map(|name| name.to_string()),
118+
};
119+
let diagnostic = Diagnostic::new(kind, expr.range());
120+
121+
checker.diagnostics.push(diagnostic);
111122
}
112123
}

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

+72-17
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,42 @@ pub(super) fn is_special_attribute(value: &Expr) -> bool {
1818
}
1919
}
2020

21-
/// Returns `true` if the given [`Expr`] is a `dataclasses.field` call.
22-
pub(super) fn is_dataclass_field(func: &Expr, semantic: &SemanticModel) -> bool {
23-
if !semantic.seen_module(Modules::DATACLASSES) {
24-
return false;
25-
}
26-
21+
/// Returns `true` if the given [`Expr`] is a stdlib `dataclasses.field` call.
22+
fn is_stdlib_dataclass_field(func: &Expr, semantic: &SemanticModel) -> bool {
2723
semantic
2824
.resolve_qualified_name(func)
2925
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["dataclasses", "field"]))
3026
}
3127

28+
/// Returns `true` if the given [`Expr`] is a call to `attr.ib()` or `attrs.field()`.
29+
fn is_attrs_field(func: &Expr, semantic: &SemanticModel) -> bool {
30+
semantic
31+
.resolve_qualified_name(func)
32+
.is_some_and(|qualified_name| {
33+
matches!(
34+
qualified_name.segments(),
35+
["attrs", "field" | "Factory"] | ["attr", "ib"]
36+
)
37+
})
38+
}
39+
40+
/// Return `true` if `func` represents a `field()` call corresponding to the `dataclass_kind` variant passed in.
41+
///
42+
/// I.e., if `DataclassKind::Attrs` is passed in,
43+
/// return `true` if `func` represents a call to `attr.ib()` or `attrs.field()`;
44+
/// if `DataclassKind::Stdlib` is passed in,
45+
/// return `true` if `func` represents a call to `dataclasse.field()`.
46+
pub(super) fn is_dataclass_field(
47+
func: &Expr,
48+
semantic: &SemanticModel,
49+
dataclass_kind: DataclassKind,
50+
) -> bool {
51+
match dataclass_kind {
52+
DataclassKind::Attrs => is_attrs_field(func, semantic),
53+
DataclassKind::Stdlib => is_stdlib_dataclass_field(func, semantic),
54+
}
55+
}
56+
3257
/// Returns `true` if the given [`Expr`] is a `typing.ClassVar` annotation.
3358
pub(super) fn is_class_var_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool {
3459
if !semantic.seen_typing() {
@@ -51,19 +76,49 @@ pub(super) fn is_final_annotation(annotation: &Expr, semantic: &SemanticModel) -
5176
semantic.match_typing_expr(map_subscript(annotation), "Final")
5277
}
5378

54-
/// Returns `true` if the given class is a dataclass.
55-
pub(super) fn is_dataclass(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool {
56-
if !semantic.seen_module(Modules::DATACLASSES) {
57-
return false;
79+
/// Enumeration of various kinds of dataclasses recognised by Ruff
80+
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
81+
pub(super) enum DataclassKind {
82+
/// dataclasses created by the stdlib `dataclasses` module
83+
Stdlib,
84+
/// dataclasses created by the third-party `attrs` library
85+
Attrs,
86+
}
87+
88+
impl DataclassKind {
89+
pub(super) const fn is_stdlib(self) -> bool {
90+
matches!(self, DataclassKind::Stdlib)
5891
}
5992

60-
class_def.decorator_list.iter().any(|decorator| {
61-
semantic
62-
.resolve_qualified_name(map_callable(&decorator.expression))
63-
.is_some_and(|qualified_name| {
64-
matches!(qualified_name.segments(), ["dataclasses", "dataclass"])
65-
})
66-
})
93+
pub(super) const fn is_attrs(self) -> bool {
94+
matches!(self, DataclassKind::Attrs)
95+
}
96+
}
97+
98+
/// Return the kind of dataclass this class definition is (stdlib or `attrs`), or `None` if the class is not a dataclass.
99+
pub(super) fn dataclass_kind(
100+
class_def: &ast::StmtClassDef,
101+
semantic: &SemanticModel,
102+
) -> Option<DataclassKind> {
103+
if !(semantic.seen_module(Modules::DATACLASSES) || semantic.seen_module(Modules::ATTRS)) {
104+
return None;
105+
}
106+
107+
for decorator in &class_def.decorator_list {
108+
let Some(qualified_name) =
109+
semantic.resolve_qualified_name(map_callable(&decorator.expression))
110+
else {
111+
continue;
112+
};
113+
114+
match qualified_name.segments() {
115+
["attrs", "define" | "frozen"] | ["attr", "s"] => return Some(DataclassKind::Attrs),
116+
["dataclasses", "dataclass"] => return Some(DataclassKind::Stdlib),
117+
_ => continue,
118+
}
119+
}
120+
121+
None
67122
}
68123

69124
/// Returns `true` if the given class has "default copy" semantics.

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

+7-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use ruff_text_size::Ranged;
77

88
use crate::checkers::ast::Checker;
99
use crate::rules::ruff::rules::helpers::{
10-
has_default_copy_semantics, is_class_var_annotation, is_dataclass, is_final_annotation,
10+
dataclass_kind, has_default_copy_semantics, is_class_var_annotation, is_final_annotation,
1111
is_special_attribute,
1212
};
1313

@@ -65,8 +65,13 @@ pub(crate) fn mutable_class_default(checker: &mut Checker, class_def: &ast::Stmt
6565
&& !is_class_var_annotation(annotation, checker.semantic())
6666
&& !is_final_annotation(annotation, checker.semantic())
6767
&& !is_immutable_annotation(annotation, checker.semantic(), &[])
68-
&& !is_dataclass(class_def, checker.semantic())
6968
{
69+
if let Some(dataclass_kind) = dataclass_kind(class_def, checker.semantic()) {
70+
if dataclass_kind.is_stdlib() || checker.settings.preview.is_enabled() {
71+
continue;
72+
}
73+
}
74+
7075
// Avoid, e.g., Pydantic and msgspec models, which end up copying defaults on instance creation.
7176
if has_default_copy_semantics(class_def, checker.semantic()) {
7277
return;

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

+19-11
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use ruff_python_semantic::analyze::typing::{is_immutable_annotation, is_mutable_
66
use ruff_text_size::Ranged;
77

88
use crate::checkers::ast::Checker;
9-
use crate::rules::ruff::rules::helpers::{is_class_var_annotation, is_dataclass};
9+
use crate::rules::ruff::rules::helpers::{dataclass_kind, is_class_var_annotation};
1010

1111
/// ## What it does
1212
/// Checks for mutable default values in dataclass attributes.
@@ -66,25 +66,33 @@ impl Violation for MutableDataclassDefault {
6666

6767
/// RUF008
6868
pub(crate) fn mutable_dataclass_default(checker: &mut Checker, class_def: &ast::StmtClassDef) {
69-
if !is_dataclass(class_def, checker.semantic()) {
69+
let semantic = checker.semantic();
70+
71+
let Some(dataclass_kind) = dataclass_kind(class_def, semantic) else {
72+
return;
73+
};
74+
75+
if dataclass_kind.is_attrs() && checker.settings.preview.is_disabled() {
7076
return;
7177
}
7278

7379
for statement in &class_def.body {
74-
if let Stmt::AnnAssign(ast::StmtAnnAssign {
80+
let Stmt::AnnAssign(ast::StmtAnnAssign {
7581
annotation,
7682
value: Some(value),
7783
..
7884
}) = statement
85+
else {
86+
continue;
87+
};
88+
89+
if is_mutable_expr(value, checker.semantic())
90+
&& !is_class_var_annotation(annotation, checker.semantic())
91+
&& !is_immutable_annotation(annotation, checker.semantic(), &[])
7992
{
80-
if is_mutable_expr(value, checker.semantic())
81-
&& !is_class_var_annotation(annotation, checker.semantic())
82-
&& !is_immutable_annotation(annotation, checker.semantic(), &[])
83-
{
84-
checker
85-
.diagnostics
86-
.push(Diagnostic::new(MutableDataclassDefault, value.range()));
87-
}
93+
let diagnostic = Diagnostic::new(MutableDataclassDefault, value.range());
94+
95+
checker.diagnostics.push(diagnostic);
8896
}
8997
}
9098
}

0 commit comments

Comments
 (0)