[ruff] Dataclass enums (RUF049) (#15299)

Co-authored-by: Micha Reiser <[email protected]>

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

@@ -0,0 +1,59 @@
+from dataclasses import dataclass
+from enum import Enum, Flag, IntEnum, IntFlag, StrEnum, ReprEnum
+
+import attr
+import attrs
+
+
+## Errors
+
+@dataclass
+class E(Enum): ...
+
+
+@dataclass  # Foobar
+class E(Flag): ...
+
+
+@dataclass()
+class E(IntEnum): ...
+
+
+@dataclass()  # Foobar
+class E(IntFlag): ...
+
+
+@dataclass(
+    frozen=True
+)
+class E(StrEnum): ...
+
+
+@dataclass(  # Foobar
+    frozen=True
+)
+class E(ReprEnum): ...
+
+
+@dataclass(
+    frozen=True
+)  # Foobar
+class E(Enum): ...
+
+
+## No errors
+
+@attrs.define
+class E(Enum): ...
+
+
+@attrs.frozen
+class E(Enum): ...
+
+
+@attrs.mutable
+class E(Enum): ...
+
+
+@attr.s
+class E(Enum): ...

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

@@ -556,6 +556,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             if checker.enabled(Rule::SubclassBuiltin) {
                 refurb::rules::subclass_builtin(checker, class_def);
             }
+            if checker.enabled(Rule::DataclassEnum) {
+                ruff::rules::dataclass_enum(checker, class_def);
+            }
         }
         Stmt::Import(ast::StmtImport { names, range: _ }) => {
             if checker.enabled(Rule::MultipleImportsOnOneLine) {

crates/ruff_linter/src/codes.rs

@@ -991,6 +991,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "043") => (RuleGroup::Preview, rules::ruff::rules::PytestRaisesAmbiguousPattern),
         (Ruff, "046") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryCastToInt),
         (Ruff, "048") => (RuleGroup::Preview, rules::ruff::rules::MapIntVersionParsing),
+        (Ruff, "049") => (RuleGroup::Preview, rules::ruff::rules::DataclassEnum),
         (Ruff, "051") => (RuleGroup::Preview, rules::ruff::rules::IfKeyInDictDel),
         (Ruff, "052") => (RuleGroup::Preview, rules::ruff::rules::UsedDummyVariable),
         (Ruff, "055") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryRegularExpression),

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

@@ -421,6 +421,7 @@ mod tests {
     #[test_case(Rule::UnnecessaryCastToInt, Path::new("RUF046.py"))]
     #[test_case(Rule::PytestRaisesAmbiguousPattern, Path::new("RUF043.py"))]
     #[test_case(Rule::UnnecessaryRound, Path::new("RUF057.py"))]
+    #[test_case(Rule::DataclassEnum, Path::new("RUF049.py"))]
     fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!(
             "preview__{}_{}",

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

@@ -0,0 +1,76 @@
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, ViolationMetadata};
+use ruff_python_ast::StmtClassDef;
+use ruff_python_semantic::analyze::class::is_enumeration;
+
+use crate::checkers::ast::Checker;
+use crate::rules::ruff::rules::helpers::{dataclass_kind, DataclassKind};
+
+/// ## What it does
+/// Checks for enum classes which are also decorated with `@dataclass`.
+///
+/// ## Why is this bad?
+/// Decorating an enum with `@dataclass()` does not cause any errors at runtime,
+/// but may cause erroneous results:
+///
+/// ```python
+/// @dataclass
+/// class E(Enum):
+///     A = 1
+///     B = 2
+///
+/// print(E.A == E.B)  # True
+/// ```
+///
+/// ## Example
+///
+/// ```python
+/// from dataclasses import dataclass
+/// from enum import Enum
+///
+///
+/// @dataclass
+/// class E(Enum): ...
+/// ```
+///
+/// Use instead:
+///
+/// ```python
+/// from enum import Enum
+///
+///
+/// class E(Enum): ...
+/// ```
+///
+/// ## References
+/// - [Python documentation: Enum HOWTO § Dataclass support](https://docs.python.org/3/howto/enum.html#dataclass-support)
+#[derive(ViolationMetadata)]
+pub(crate) struct DataclassEnum;
+
+impl Violation for DataclassEnum {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        "An enum class should not be decorated with `@dataclass`".to_string()
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        Some("Remove either `@dataclass` or `Enum`".to_string())
+    }
+}
+
+/// RUF049
+pub(crate) fn dataclass_enum(checker: &mut Checker, class_def: &StmtClassDef) {
+    let semantic = checker.semantic();
+
+    let Some((DataclassKind::Stdlib, decorator)) = dataclass_kind(class_def, semantic) else {
+        return;
+    };
+
+    if !is_enumeration(class_def, semantic) {
+        return;
+    }
+
+    let diagnostic = Diagnostic::new(DataclassEnum, decorator.range);
+
+    checker.diagnostics.push(diagnostic);
+}

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

@@ -77,7 +77,7 @@ pub(crate) fn function_call_in_dataclass_default(
 ) {
     let semantic = checker.semantic();
 
-    let Some(dataclass_kind) = dataclass_kind(class_def, semantic) else {
+    let Some((dataclass_kind, _)) = dataclass_kind(class_def, semantic) else {
         return;
     };
 
@@ -88,7 +88,7 @@ pub(crate) fn function_call_in_dataclass_default(
     let attrs_auto_attribs = match dataclass_kind {
         DataclassKind::Stdlib => None,
 
-        DataclassKind::Attrs(attrs_auto_attribs) => match attrs_auto_attribs {
+        DataclassKind::Attrs(auto_attribs) => match auto_attribs {
             AttrsAutoAttribs::Unknown => return,
 
             AttrsAutoAttribs::None => {
@@ -99,12 +99,13 @@ pub(crate) fn function_call_in_dataclass_default(
                 }
             }
 
-            _ => Some(attrs_auto_attribs),
+            _ => Some(auto_attribs),
         },
     };
+
     let dataclass_kind = match attrs_auto_attribs {
         None => DataclassKind::Stdlib,
-        Some(attrs_auto_attribs) => DataclassKind::Attrs(attrs_auto_attribs),
+        Some(auto_attribs) => DataclassKind::Attrs(auto_attribs),
     };
 
     let extend_immutable_calls: Vec<QualifiedName> = checker

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

@@ -113,10 +113,10 @@ impl DataclassKind {
 
 /// Return the kind of dataclass this class definition is (stdlib or `attrs`),
 /// or `None` if the class is not a dataclass.
-pub(super) fn dataclass_kind(
-    class_def: &ast::StmtClassDef,
+pub(super) fn dataclass_kind<'a>(
+    class_def: &'a ast::StmtClassDef,
     semantic: &SemanticModel,
-) -> Option<DataclassKind> {
+) -> Option<(DataclassKind, &'a ast::Decorator)> {
     if !(semantic.seen_module(Modules::DATACLASSES) || semantic.seen_module(Modules::ATTRS)) {
         return None;
     }
@@ -141,11 +141,11 @@ pub(super) fn dataclass_kind(
                         AttrsAutoAttribs::None
                     };
 
-                    return Some(DataclassKind::Attrs(auto_attribs));
+                    return Some((DataclassKind::Attrs(auto_attribs), decorator));
                 };
 
                 let Some(auto_attribs) = arguments.find_keyword("auto_attribs") else {
-                    return Some(DataclassKind::Attrs(AttrsAutoAttribs::None));
+                    return Some((DataclassKind::Attrs(AttrsAutoAttribs::None), decorator));
                 };
 
                 let auto_attribs = match Truthiness::from_expr(&auto_attribs.value, |id| {
@@ -163,9 +163,9 @@ pub(super) fn dataclass_kind(
                     Truthiness::Unknown => AttrsAutoAttribs::Unknown,
                 };
 
-                return Some(DataclassKind::Attrs(auto_attribs));
+                return Some((DataclassKind::Attrs(auto_attribs), decorator));
             }
-            ["dataclasses", "dataclass"] => return Some(DataclassKind::Stdlib),
+            ["dataclasses", "dataclass"] => return Some((DataclassKind::Stdlib, decorator)),
             _ => continue,
         }
     }

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

@@ -3,6 +3,7 @@ pub(crate) use assert_with_print_message::*;
 pub(crate) use assignment_in_assert::*;
 pub(crate) use asyncio_dangling_task::*;
 pub(crate) use collection_literal_concatenation::*;
+pub(crate) use dataclass_enum::*;
 pub(crate) use decimal_from_float_literal::*;
 pub(crate) use default_factory_kwarg::*;
 pub(crate) use explicit_f_string_type_conversion::*;
@@ -54,6 +55,7 @@ mod assignment_in_assert;
 mod asyncio_dangling_task;
 mod collection_literal_concatenation;
 mod confusables;
+mod dataclass_enum;
 mod decimal_from_float_literal;
 mod default_factory_kwarg;
 mod explicit_f_string_type_conversion;

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

@@ -66,7 +66,8 @@ pub(crate) fn mutable_class_default(checker: &mut Checker, class_def: &ast::Stmt
                     && !is_final_annotation(annotation, checker.semantic())
                     && !is_immutable_annotation(annotation, checker.semantic(), &[])
                 {
-                    if let Some(dataclass_kind) = dataclass_kind(class_def, checker.semantic()) {
+                    if let Some((dataclass_kind, _)) = dataclass_kind(class_def, checker.semantic())
+                    {
                         if dataclass_kind.is_stdlib() || checker.settings.preview.is_enabled() {
                             continue;
                         }

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

@@ -68,7 +68,7 @@ impl Violation for MutableDataclassDefault {
 pub(crate) fn mutable_dataclass_default(checker: &mut Checker, class_def: &ast::StmtClassDef) {
     let semantic = checker.semantic();
 
-    let Some(dataclass_kind) = dataclass_kind(class_def, semantic) else {
+    let Some((dataclass_kind, _)) = dataclass_kind(class_def, semantic) else {
         return;
     };
 

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

@@ -93,7 +93,7 @@ pub(crate) fn post_init_default(checker: &mut Checker, function_def: &ast::StmtF
         ScopeKind::Class(class_def) => {
             if !matches!(
                 dataclass_kind(class_def, checker.semantic()),
-                Some(DataclassKind::Stdlib)
+                Some((DataclassKind::Stdlib, _))
             ) {
                 return;
             }

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF049_RUF049.py.snap

@@ -0,0 +1,67 @@
+---
+source: crates/ruff_linter/src/rules/ruff/mod.rs
+snapshot_kind: text
+---
+RUF049.py:10:1: RUF049 An enum class should not be decorated with `@dataclass`
+   |
+ 8 | ## Errors
+ 9 | 
+10 | @dataclass
+   | ^^^^^^^^^^ RUF049
+11 | class E(Enum): ...
+   |
+   = help: Remove either `@dataclass` or `Enum`
+
+RUF049.py:14:1: RUF049 An enum class should not be decorated with `@dataclass`
+   |
+14 | @dataclass  # Foobar
+   | ^^^^^^^^^^ RUF049
+15 | class E(Flag): ...
+   |
+   = help: Remove either `@dataclass` or `Enum`
+
+RUF049.py:18:1: RUF049 An enum class should not be decorated with `@dataclass`
+   |
+18 | @dataclass()
+   | ^^^^^^^^^^^^ RUF049
+19 | class E(IntEnum): ...
+   |
+   = help: Remove either `@dataclass` or `Enum`
+
+RUF049.py:22:1: RUF049 An enum class should not be decorated with `@dataclass`
+   |
+22 | @dataclass()  # Foobar
+   | ^^^^^^^^^^^^ RUF049
+23 | class E(IntFlag): ...
+   |
+   = help: Remove either `@dataclass` or `Enum`
+
+RUF049.py:26:1: RUF049 An enum class should not be decorated with `@dataclass`
+   |
+26 | / @dataclass(
+27 | |     frozen=True
+28 | | )
+   | |_^ RUF049
+29 |   class E(StrEnum): ...
+   |
+   = help: Remove either `@dataclass` or `Enum`
+
+RUF049.py:32:1: RUF049 An enum class should not be decorated with `@dataclass`
+   |
+32 | / @dataclass(  # Foobar
+33 | |     frozen=True
+34 | | )
+   | |_^ RUF049
+35 |   class E(ReprEnum): ...
+   |
+   = help: Remove either `@dataclass` or `Enum`
+
+RUF049.py:38:1: RUF049 An enum class should not be decorated with `@dataclass`
+   |
+38 | / @dataclass(
+39 | |     frozen=True
+40 | | )  # Foobar
+   | |_^ RUF049
+41 |   class E(Enum): ...
+   |
+   = help: Remove either `@dataclass` or `Enum`