Skip to content

Commit 6362880

Browse files
InSyncWithFooMichaReiser
InSyncWithFoo
and
MichaReiser
authored
[ruff] Dataclass enums (RUF049) (#15299)
Co-authored-by: Micha Reiser <[email protected]>
1 parent 832c0fa commit 6362880

File tree

13 files changed

+226
-14
lines changed

13 files changed

+226
-14
lines changed

‎crates/ruff_linter/resources/test/fixtures/ruff/RUF049.py‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
from dataclasses import dataclass
2+
from enum import Enum, Flag, IntEnum, IntFlag, StrEnum, ReprEnum
3+
4+
import attr
5+
import attrs
6+
7+
8+
## Errors
9+
10+
@dataclass
11+
class E(Enum): ...
12+
13+
14+
@dataclass # Foobar
15+
class E(Flag): ...
16+
17+
18+
@dataclass()
19+
class E(IntEnum): ...
20+
21+
22+
@dataclass() # Foobar
23+
class E(IntFlag): ...
24+
25+
26+
@dataclass(
27+
frozen=True
28+
)
29+
class E(StrEnum): ...
30+
31+
32+
@dataclass( # Foobar
33+
frozen=True
34+
)
35+
class E(ReprEnum): ...
36+
37+
38+
@dataclass(
39+
frozen=True
40+
) # Foobar
41+
class E(Enum): ...
42+
43+
44+
## No errors
45+
46+
@attrs.define
47+
class E(Enum): ...
48+
49+
50+
@attrs.frozen
51+
class E(Enum): ...
52+
53+
54+
@attrs.mutable
55+
class E(Enum): ...
56+
57+
58+
@attr.s
59+
class E(Enum): ...

‎crates/ruff_linter/src/checkers/ast/analyze/statement.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
556556
if checker.enabled(Rule::SubclassBuiltin) {
557557
refurb::rules::subclass_builtin(checker, class_def);
558558
}
559+
if checker.enabled(Rule::DataclassEnum) {
560+
ruff::rules::dataclass_enum(checker, class_def);
561+
}
559562
}
560563
Stmt::Import(ast::StmtImport { names, range: _ }) => {
561564
if checker.enabled(Rule::MultipleImportsOnOneLine) {

‎crates/ruff_linter/src/codes.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -991,6 +991,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
991991
(Ruff, "043") => (RuleGroup::Preview, rules::ruff::rules::PytestRaisesAmbiguousPattern),
992992
(Ruff, "046") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryCastToInt),
993993
(Ruff, "048") => (RuleGroup::Preview, rules::ruff::rules::MapIntVersionParsing),
994+
(Ruff, "049") => (RuleGroup::Preview, rules::ruff::rules::DataclassEnum),
994995
(Ruff, "051") => (RuleGroup::Preview, rules::ruff::rules::IfKeyInDictDel),
995996
(Ruff, "052") => (RuleGroup::Preview, rules::ruff::rules::UsedDummyVariable),
996997
(Ruff, "055") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryRegularExpression),

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,7 @@ mod tests {
421421
#[test_case(Rule::UnnecessaryCastToInt, Path::new("RUF046.py"))]
422422
#[test_case(Rule::PytestRaisesAmbiguousPattern, Path::new("RUF043.py"))]
423423
#[test_case(Rule::UnnecessaryRound, Path::new("RUF057.py"))]
424+
#[test_case(Rule::DataclassEnum, Path::new("RUF049.py"))]
424425
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
425426
let snapshot = format!(
426427
"preview__{}_{}",

‎crates/ruff_linter/src/rules/ruff/rules/dataclass_enum.rs‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
use ruff_diagnostics::{Diagnostic, Violation};
2+
use ruff_macros::{derive_message_formats, ViolationMetadata};
3+
use ruff_python_ast::StmtClassDef;
4+
use ruff_python_semantic::analyze::class::is_enumeration;
5+
6+
use crate::checkers::ast::Checker;
7+
use crate::rules::ruff::rules::helpers::{dataclass_kind, DataclassKind};
8+
9+
/// ## What it does
10+
/// Checks for enum classes which are also decorated with `@dataclass`.
11+
///
12+
/// ## Why is this bad?
13+
/// Decorating an enum with `@dataclass()` does not cause any errors at runtime,
14+
/// but may cause erroneous results:
15+
///
16+
/// ```python
17+
/// @dataclass
18+
/// class E(Enum):
19+
/// A = 1
20+
/// B = 2
21+
///
22+
/// print(E.A == E.B) # True
23+
/// ```
24+
///
25+
/// ## Example
26+
///
27+
/// ```python
28+
/// from dataclasses import dataclass
29+
/// from enum import Enum
30+
///
31+
///
32+
/// @dataclass
33+
/// class E(Enum): ...
34+
/// ```
35+
///
36+
/// Use instead:
37+
///
38+
/// ```python
39+
/// from enum import Enum
40+
///
41+
///
42+
/// class E(Enum): ...
43+
/// ```
44+
///
45+
/// ## References
46+
/// - [Python documentation: Enum HOWTO &sect; Dataclass support](https://docs.python.org/3/howto/enum.html#dataclass-support)
47+
#[derive(ViolationMetadata)]
48+
pub(crate) struct DataclassEnum;
49+
50+
impl Violation for DataclassEnum {
51+
#[derive_message_formats]
52+
fn message(&self) -> String {
53+
"An enum class should not be decorated with `@dataclass`".to_string()
54+
}
55+
56+
fn fix_title(&self) -> Option<String> {
57+
Some("Remove either `@dataclass` or `Enum`".to_string())
58+
}
59+
}
60+
61+
/// RUF049
62+
pub(crate) fn dataclass_enum(checker: &mut Checker, class_def: &StmtClassDef) {
63+
let semantic = checker.semantic();
64+
65+
let Some((DataclassKind::Stdlib, decorator)) = dataclass_kind(class_def, semantic) else {
66+
return;
67+
};
68+
69+
if !is_enumeration(class_def, semantic) {
70+
return;
71+
}
72+
73+
let diagnostic = Diagnostic::new(DataclassEnum, decorator.range);
74+
75+
checker.diagnostics.push(diagnostic);
76+
}

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ pub(crate) fn function_call_in_dataclass_default(
7777
) {
7878
let semantic = checker.semantic();
7979

80-
let Some(dataclass_kind) = dataclass_kind(class_def, semantic) else {
80+
let Some((dataclass_kind, _)) = dataclass_kind(class_def, semantic) else {
8181
return;
8282
};
8383

@@ -88,7 +88,7 @@ pub(crate) fn function_call_in_dataclass_default(
8888
let attrs_auto_attribs = match dataclass_kind {
8989
DataclassKind::Stdlib => None,
9090

91-
DataclassKind::Attrs(attrs_auto_attribs) => match attrs_auto_attribs {
91+
DataclassKind::Attrs(auto_attribs) => match auto_attribs {
9292
AttrsAutoAttribs::Unknown => return,
9393

9494
AttrsAutoAttribs::None => {
@@ -99,12 +99,13 @@ pub(crate) fn function_call_in_dataclass_default(
9999
}
100100
}
101101

102-
_ => Some(attrs_auto_attribs),
102+
_ => Some(auto_attribs),
103103
},
104104
};
105+
105106
let dataclass_kind = match attrs_auto_attribs {
106107
None => DataclassKind::Stdlib,
107-
Some(attrs_auto_attribs) => DataclassKind::Attrs(attrs_auto_attribs),
108+
Some(auto_attribs) => DataclassKind::Attrs(auto_attribs),
108109
};
109110

110111
let extend_immutable_calls: Vec<QualifiedName> = checker

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,10 @@ impl DataclassKind {
113113

114114
/// Return the kind of dataclass this class definition is (stdlib or `attrs`),
115115
/// or `None` if the class is not a dataclass.
116-
pub(super) fn dataclass_kind(
117-
class_def: &ast::StmtClassDef,
116+
pub(super) fn dataclass_kind<'a>(
117+
class_def: &'a ast::StmtClassDef,
118118
semantic: &SemanticModel,
119-
) -> Option<DataclassKind> {
119+
) -> Option<(DataclassKind, &'a ast::Decorator)> {
120120
if !(semantic.seen_module(Modules::DATACLASSES) || semantic.seen_module(Modules::ATTRS)) {
121121
return None;
122122
}
@@ -141,11 +141,11 @@ pub(super) fn dataclass_kind(
141141
AttrsAutoAttribs::None
142142
};
143143

144-
return Some(DataclassKind::Attrs(auto_attribs));
144+
return Some((DataclassKind::Attrs(auto_attribs), decorator));
145145
};
146146

147147
let Some(auto_attribs) = arguments.find_keyword("auto_attribs") else {
148-
return Some(DataclassKind::Attrs(AttrsAutoAttribs::None));
148+
return Some((DataclassKind::Attrs(AttrsAutoAttribs::None), decorator));
149149
};
150150

151151
let auto_attribs = match Truthiness::from_expr(&auto_attribs.value, |id| {
@@ -163,9 +163,9 @@ pub(super) fn dataclass_kind(
163163
Truthiness::Unknown => AttrsAutoAttribs::Unknown,
164164
};
165165

166-
return Some(DataclassKind::Attrs(auto_attribs));
166+
return Some((DataclassKind::Attrs(auto_attribs), decorator));
167167
}
168-
["dataclasses", "dataclass"] => return Some(DataclassKind::Stdlib),
168+
["dataclasses", "dataclass"] => return Some((DataclassKind::Stdlib, decorator)),
169169
_ => continue,
170170
}
171171
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ pub(crate) use assert_with_print_message::*;
33
pub(crate) use assignment_in_assert::*;
44
pub(crate) use asyncio_dangling_task::*;
55
pub(crate) use collection_literal_concatenation::*;
6+
pub(crate) use dataclass_enum::*;
67
pub(crate) use decimal_from_float_literal::*;
78
pub(crate) use default_factory_kwarg::*;
89
pub(crate) use explicit_f_string_type_conversion::*;
@@ -54,6 +55,7 @@ mod assignment_in_assert;
5455
mod asyncio_dangling_task;
5556
mod collection_literal_concatenation;
5657
mod confusables;
58+
mod dataclass_enum;
5759
mod decimal_from_float_literal;
5860
mod default_factory_kwarg;
5961
mod explicit_f_string_type_conversion;

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ pub(crate) fn mutable_class_default(checker: &mut Checker, class_def: &ast::Stmt
6666
&& !is_final_annotation(annotation, checker.semantic())
6767
&& !is_immutable_annotation(annotation, checker.semantic(), &[])
6868
{
69-
if let Some(dataclass_kind) = dataclass_kind(class_def, checker.semantic()) {
69+
if let Some((dataclass_kind, _)) = dataclass_kind(class_def, checker.semantic())
70+
{
7071
if dataclass_kind.is_stdlib() || checker.settings.preview.is_enabled() {
7172
continue;
7273
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ impl Violation for MutableDataclassDefault {
6868
pub(crate) fn mutable_dataclass_default(checker: &mut Checker, class_def: &ast::StmtClassDef) {
6969
let semantic = checker.semantic();
7070

71-
let Some(dataclass_kind) = dataclass_kind(class_def, semantic) else {
71+
let Some((dataclass_kind, _)) = dataclass_kind(class_def, semantic) else {
7272
return;
7373
};
7474

0 commit comments

Comments
 (0)