diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF008_attrs.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF008_attrs.py new file mode 100644 index 0000000000000..016be01bced00 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF008_attrs.py @@ -0,0 +1,47 @@ +import typing +from typing import ClassVar, Sequence + +import attr +from attr import s +from attrs import define, frozen + +KNOWINGLY_MUTABLE_DEFAULT = [] + + +@define +class A: + mutable_default: list[int] = [] + immutable_annotation: typing.Sequence[int] = [] + without_annotation = [] + correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT + perfectly_fine: list[int] = field(default_factory=list) + class_variable: typing.ClassVar[list[int]] = [] + + +@frozen +class B: + 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]] = [] + + +@attr.s +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]] = [] + +@s +class D: + 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]] = [] diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF009_attrs.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF009_attrs.py new file mode 100644 index 0000000000000..7910b3932306b --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF009_attrs.py @@ -0,0 +1,75 @@ +import datetime +import re +import typing +from collections import OrderedDict +from fractions import Fraction +from pathlib import Path +from typing import ClassVar, NamedTuple + +import attr +from attrs import Factory, define, field, frozen + + +def default_function() -> list[int]: + return [] + + +class ImmutableType(NamedTuple): + something: int = 8 + + +@attr.s +class A: + hidden_mutable_default: list[int] = default_function() + class_variable: typing.ClassVar[list[int]] = default_function() + another_class_var: ClassVar[list[int]] = default_function() + + fine_path: Path = Path() + fine_date: datetime.date = datetime.date(2042, 1, 1) + fine_timedelta: datetime.timedelta = datetime.timedelta(hours=7) + fine_tuple: tuple[int] = tuple([1]) + fine_regex: re.Pattern = re.compile(r".*") + fine_float: float = float("-inf") + fine_int: int = int(12) + fine_complex: complex = complex(1, 2) + fine_str: str = str("foo") + fine_bool: bool = bool("foo") + fine_fraction: Fraction = Fraction(1, 2) + + +DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES = ImmutableType(40) +DEFAULT_A_FOR_ALL_DATACLASSES = A([1, 2, 3]) + + +@define +class B: + hidden_mutable_default: list[int] = default_function() + another_dataclass: A = A() + not_optimal: ImmutableType = ImmutableType(20) + good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES + okay_variant: A = DEFAULT_A_FOR_ALL_DATACLASSES + + fine_dataclass_function: list[int] = field(default_factory=list) + attrs_factory: dict[str, str] = Factory(OrderedDict) + + +class IntConversionDescriptor: + def __init__(self, *, default): + self._default = default + + def __set_name__(self, owner, name): + self._name = "_" + name + + def __get__(self, obj, type): + if obj is None: + return self._default + + return getattr(obj, self._name, self._default) + + def __set__(self, obj, value): + setattr(obj, self._name, int(value)) + + +@frozen +class InventoryItem: + quantity_on_hand: IntConversionDescriptor = IntConversionDescriptor(default=100) diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 19917696691b0..95971a0378f72 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -388,6 +388,11 @@ mod tests { #[test_case(Rule::ZipInsteadOfPairwise, Path::new("RUF007.py"))] #[test_case(Rule::UnsafeMarkupUse, Path::new("RUF035.py"))] + #[test_case( + Rule::FunctionCallInDataclassDefaultArgument, + Path::new("RUF009_attrs.py") + )] + #[test_case(Rule::MutableDataclassDefault, Path::new("RUF008_attrs.py"))] fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!( "preview__{}_{}", diff --git a/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs b/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs index 1251940991c43..9e40f9a1fecca 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs @@ -8,7 +8,7 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::rules::ruff::rules::helpers::{ - is_class_var_annotation, is_dataclass, is_dataclass_field, is_descriptor_class, + dataclass_kind, is_class_var_annotation, is_dataclass_field, is_descriptor_class, }; /// ## What it does @@ -74,7 +74,13 @@ pub(crate) fn function_call_in_dataclass_default( checker: &mut Checker, class_def: &ast::StmtClassDef, ) { - if !is_dataclass(class_def, checker.semantic()) { + let semantic = checker.semantic(); + + let Some(dataclass_kind) = dataclass_kind(class_def, semantic) else { + return; + }; + + if dataclass_kind.is_attrs() && checker.settings.preview.is_disabled() { return; } @@ -87,26 +93,31 @@ pub(crate) fn function_call_in_dataclass_default( .collect(); for statement in &class_def.body { - if let Stmt::AnnAssign(ast::StmtAnnAssign { + let Stmt::AnnAssign(ast::StmtAnnAssign { annotation, value: Some(expr), .. }) = statement + else { + continue; + }; + let Expr::Call(ast::ExprCall { func, .. }) = &**expr else { + continue; + }; + + if is_class_var_annotation(annotation, checker.semantic()) + || is_immutable_func(func, checker.semantic(), &extend_immutable_calls) + || is_dataclass_field(func, checker.semantic(), dataclass_kind) + || is_descriptor_class(func, checker.semantic()) { - if let Expr::Call(ast::ExprCall { func, .. }) = expr.as_ref() { - if !is_class_var_annotation(annotation, checker.semantic()) - && !is_immutable_func(func, checker.semantic(), &extend_immutable_calls) - && !is_dataclass_field(func, checker.semantic()) - && !is_descriptor_class(func, checker.semantic()) - { - checker.diagnostics.push(Diagnostic::new( - FunctionCallInDataclassDefaultArgument { - name: UnqualifiedName::from_expr(func).map(|name| name.to_string()), - }, - expr.range(), - )); - } - } + continue; } + + let kind = FunctionCallInDataclassDefaultArgument { + name: UnqualifiedName::from_expr(func).map(|name| name.to_string()), + }; + let diagnostic = Diagnostic::new(kind, expr.range()); + + checker.diagnostics.push(diagnostic); } } diff --git a/crates/ruff_linter/src/rules/ruff/rules/helpers.rs b/crates/ruff_linter/src/rules/ruff/rules/helpers.rs index 07b96af1f0fbf..3e255d60d0ecb 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/helpers.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/helpers.rs @@ -18,17 +18,42 @@ pub(super) fn is_special_attribute(value: &Expr) -> bool { } } -/// Returns `true` if the given [`Expr`] is a `dataclasses.field` call. -pub(super) fn is_dataclass_field(func: &Expr, semantic: &SemanticModel) -> bool { - if !semantic.seen_module(Modules::DATACLASSES) { - return false; - } - +/// Returns `true` if the given [`Expr`] is a stdlib `dataclasses.field` call. +fn is_stdlib_dataclass_field(func: &Expr, semantic: &SemanticModel) -> bool { semantic .resolve_qualified_name(func) .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["dataclasses", "field"])) } +/// Returns `true` if the given [`Expr`] is a call to `attr.ib()` or `attrs.field()`. +fn is_attrs_field(func: &Expr, semantic: &SemanticModel) -> bool { + semantic + .resolve_qualified_name(func) + .is_some_and(|qualified_name| { + matches!( + qualified_name.segments(), + ["attrs", "field" | "Factory"] | ["attr", "ib"] + ) + }) +} + +/// Return `true` if `func` represents a `field()` call corresponding to the `dataclass_kind` variant passed in. +/// +/// I.e., if `DataclassKind::Attrs` is passed in, +/// return `true` if `func` represents a call to `attr.ib()` or `attrs.field()`; +/// if `DataclassKind::Stdlib` is passed in, +/// return `true` if `func` represents a call to `dataclasse.field()`. +pub(super) fn is_dataclass_field( + func: &Expr, + semantic: &SemanticModel, + dataclass_kind: DataclassKind, +) -> bool { + match dataclass_kind { + DataclassKind::Attrs => is_attrs_field(func, semantic), + DataclassKind::Stdlib => is_stdlib_dataclass_field(func, semantic), + } +} + /// Returns `true` if the given [`Expr`] is a `typing.ClassVar` annotation. pub(super) fn is_class_var_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool { if !semantic.seen_typing() { @@ -51,19 +76,49 @@ pub(super) fn is_final_annotation(annotation: &Expr, semantic: &SemanticModel) - semantic.match_typing_expr(map_subscript(annotation), "Final") } -/// Returns `true` if the given class is a dataclass. -pub(super) fn is_dataclass(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool { - if !semantic.seen_module(Modules::DATACLASSES) { - return false; +/// Enumeration of various kinds of dataclasses recognised by Ruff +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub(super) enum DataclassKind { + /// dataclasses created by the stdlib `dataclasses` module + Stdlib, + /// dataclasses created by the third-party `attrs` library + Attrs, +} + +impl DataclassKind { + pub(super) const fn is_stdlib(self) -> bool { + matches!(self, DataclassKind::Stdlib) } - class_def.decorator_list.iter().any(|decorator| { - semantic - .resolve_qualified_name(map_callable(&decorator.expression)) - .is_some_and(|qualified_name| { - matches!(qualified_name.segments(), ["dataclasses", "dataclass"]) - }) - }) + pub(super) const fn is_attrs(self) -> bool { + matches!(self, DataclassKind::Attrs) + } +} + +/// 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, + semantic: &SemanticModel, +) -> Option { + if !(semantic.seen_module(Modules::DATACLASSES) || semantic.seen_module(Modules::ATTRS)) { + return None; + } + + for decorator in &class_def.decorator_list { + let Some(qualified_name) = + semantic.resolve_qualified_name(map_callable(&decorator.expression)) + else { + continue; + }; + + match qualified_name.segments() { + ["attrs", "define" | "frozen"] | ["attr", "s"] => return Some(DataclassKind::Attrs), + ["dataclasses", "dataclass"] => return Some(DataclassKind::Stdlib), + _ => continue, + } + } + + None } /// Returns `true` if the given class has "default copy" semantics. diff --git a/crates/ruff_linter/src/rules/ruff/rules/mutable_class_default.rs b/crates/ruff_linter/src/rules/ruff/rules/mutable_class_default.rs index 295e89ac48242..2debe408195af 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mutable_class_default.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mutable_class_default.rs @@ -7,7 +7,7 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::rules::ruff::rules::helpers::{ - has_default_copy_semantics, is_class_var_annotation, is_dataclass, is_final_annotation, + dataclass_kind, has_default_copy_semantics, is_class_var_annotation, is_final_annotation, is_special_attribute, }; @@ -65,8 +65,13 @@ pub(crate) fn mutable_class_default(checker: &mut Checker, class_def: &ast::Stmt && !is_class_var_annotation(annotation, checker.semantic()) && !is_final_annotation(annotation, checker.semantic()) && !is_immutable_annotation(annotation, checker.semantic(), &[]) - && !is_dataclass(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; + } + } + // Avoid, e.g., Pydantic and msgspec models, which end up copying defaults on instance creation. if has_default_copy_semantics(class_def, checker.semantic()) { return; diff --git a/crates/ruff_linter/src/rules/ruff/rules/mutable_dataclass_default.rs b/crates/ruff_linter/src/rules/ruff/rules/mutable_dataclass_default.rs index b21a61e3908d1..35645384bd07a 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mutable_dataclass_default.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mutable_dataclass_default.rs @@ -6,7 +6,7 @@ use ruff_python_semantic::analyze::typing::{is_immutable_annotation, is_mutable_ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; -use crate::rules::ruff::rules::helpers::{is_class_var_annotation, is_dataclass}; +use crate::rules::ruff::rules::helpers::{dataclass_kind, is_class_var_annotation}; /// ## What it does /// Checks for mutable default values in dataclass attributes. @@ -66,25 +66,33 @@ impl Violation for MutableDataclassDefault { /// RUF008 pub(crate) fn mutable_dataclass_default(checker: &mut Checker, class_def: &ast::StmtClassDef) { - if !is_dataclass(class_def, checker.semantic()) { + let semantic = checker.semantic(); + + let Some(dataclass_kind) = dataclass_kind(class_def, semantic) else { + return; + }; + + if dataclass_kind.is_attrs() && checker.settings.preview.is_disabled() { return; } for statement in &class_def.body { - if let Stmt::AnnAssign(ast::StmtAnnAssign { + let Stmt::AnnAssign(ast::StmtAnnAssign { annotation, value: Some(value), .. }) = statement + else { + continue; + }; + + if is_mutable_expr(value, checker.semantic()) + && !is_class_var_annotation(annotation, checker.semantic()) + && !is_immutable_annotation(annotation, checker.semantic(), &[]) { - if is_mutable_expr(value, checker.semantic()) - && !is_class_var_annotation(annotation, checker.semantic()) - && !is_immutable_annotation(annotation, checker.semantic(), &[]) - { - checker - .diagnostics - .push(Diagnostic::new(MutableDataclassDefault, value.range())); - } + let diagnostic = Diagnostic::new(MutableDataclassDefault, value.range()); + + checker.diagnostics.push(diagnostic); } } } diff --git a/crates/ruff_linter/src/rules/ruff/rules/post_init_default.rs b/crates/ruff_linter/src/rules/ruff/rules/post_init_default.rs index 75eddf84659a6..21a4c20cf7488 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/post_init_default.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/post_init_default.rs @@ -10,7 +10,7 @@ use ruff_text_size::Ranged; use crate::{checkers::ast::Checker, importer::ImportRequest}; -use super::helpers::is_dataclass; +use super::helpers::{dataclass_kind, DataclassKind}; /// ## What it does /// Checks for `__post_init__` dataclass methods with parameter defaults. @@ -91,7 +91,10 @@ pub(crate) fn post_init_default(checker: &mut Checker, function_def: &ast::StmtF let current_scope = checker.semantic().current_scope(); match current_scope.kind { ScopeKind::Class(class_def) => { - if !is_dataclass(class_def, checker.semantic()) { + if !matches!( + dataclass_kind(class_def, checker.semantic()), + Some(DataclassKind::Stdlib) + ) { return; } } diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF009_RUF009.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF009_RUF009.py.snap index c9c3e9ed8477e..3d38dd0d510b5 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF009_RUF009.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF009_RUF009.py.snap @@ -40,5 +40,3 @@ RUF009.py:45:34: RUF009 Do not perform function call `ImmutableType` in dataclas 46 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES 47 | okay_variant: A = DEFAULT_A_FOR_ALL_DATACLASSES | - - diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF008_RUF008_attrs.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF008_RUF008_attrs.py.snap new file mode 100644 index 0000000000000..23561eb18fc0b --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF008_RUF008_attrs.py.snap @@ -0,0 +1,42 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF008_attrs.py:13:34: RUF008 Do not use mutable default values for dataclass attributes + | +11 | @define +12 | class A: +13 | mutable_default: list[int] = [] + | ^^ RUF008 +14 | immutable_annotation: typing.Sequence[int] = [] +15 | without_annotation = [] + | + +RUF008_attrs.py:23:34: RUF008 Do not use mutable default values for dataclass attributes + | +21 | @frozen +22 | class B: +23 | mutable_default: list[int] = [] + | ^^ RUF008 +24 | immutable_annotation: Sequence[int] = [] +25 | without_annotation = [] + | + +RUF008_attrs.py:33:34: RUF008 Do not use mutable default values for dataclass attributes + | +31 | @attr.s +32 | class C: +33 | mutable_default: list[int] = [] + | ^^ RUF008 +34 | immutable_annotation: Sequence[int] = [] +35 | without_annotation = [] + | + +RUF008_attrs.py:42:34: RUF008 Do not use mutable default values for dataclass attributes + | +40 | @s +41 | class D: +42 | mutable_default: list[int] = [] + | ^^ RUF008 +43 | immutable_annotation: Sequence[int] = [] +44 | without_annotation = [] + | diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF009_RUF009_attrs.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF009_RUF009_attrs.py.snap new file mode 100644 index 0000000000000..2052306b4f288 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF009_RUF009_attrs.py.snap @@ -0,0 +1,42 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF009_attrs.py:23:41: RUF009 Do not perform function call `default_function` in dataclass defaults + | +21 | @attr.s +22 | class A: +23 | hidden_mutable_default: list[int] = default_function() + | ^^^^^^^^^^^^^^^^^^ RUF009 +24 | class_variable: typing.ClassVar[list[int]] = default_function() +25 | another_class_var: ClassVar[list[int]] = default_function() + | + +RUF009_attrs.py:46:41: RUF009 Do not perform function call `default_function` in dataclass defaults + | +44 | @define +45 | class B: +46 | hidden_mutable_default: list[int] = default_function() + | ^^^^^^^^^^^^^^^^^^ RUF009 +47 | another_dataclass: A = A() +48 | not_optimal: ImmutableType = ImmutableType(20) + | + +RUF009_attrs.py:47:28: RUF009 Do not perform function call `A` in dataclass defaults + | +45 | class B: +46 | hidden_mutable_default: list[int] = default_function() +47 | another_dataclass: A = A() + | ^^^ RUF009 +48 | not_optimal: ImmutableType = ImmutableType(20) +49 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES + | + +RUF009_attrs.py:48:34: RUF009 Do not perform function call `ImmutableType` in dataclass defaults + | +46 | hidden_mutable_default: list[int] = default_function() +47 | another_dataclass: A = A() +48 | not_optimal: ImmutableType = ImmutableType(20) + | ^^^^^^^^^^^^^^^^^ RUF009 +49 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES +50 | okay_variant: A = DEFAULT_A_FOR_ALL_DATACLASSES + | diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 9826be9a76327..273a670c251fe 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -1287,6 +1287,7 @@ impl<'a> SemanticModel<'a> { "trio" => self.seen.insert(Modules::TRIO), "typing" => self.seen.insert(Modules::TYPING), "typing_extensions" => self.seen.insert(Modules::TYPING_EXTENSIONS), + "attr" | "attrs" => self.seen.insert(Modules::ATTRS), _ => {} } } @@ -1862,6 +1863,7 @@ bitflags! { const COPY = 1 << 22; const MARKUPSAFE = 1 << 23; const FLASK = 1 << 24; + const ATTRS = 1 << 25; } }