Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ruff] Dataclass enums (RUF049) #15299

Merged
merged 4 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF049.py
Original file line number Diff line number Diff line change
@@ -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): ...
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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__{}_{}",
Expand Down
76 changes: 76 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/dataclass_enum.rs
Original file line number Diff line number Diff line change
@@ -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 &sect; 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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand All @@ -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 => {
Expand All @@ -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
Expand Down
14 changes: 7 additions & 7 deletions crates/ruff_linter/src/rules/ruff/rules/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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| {
Expand All @@ -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,
}
}
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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`
1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading