diff --git a/crates/ruff/resources/test/fixtures/ruff/RUF010.py b/crates/ruff/resources/test/fixtures/ruff/RUF010.py new file mode 100644 index 0000000000000..430129a45bac9 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/ruff/RUF010.py @@ -0,0 +1,20 @@ +import typing +from typing import Sequence + +KNOWINGLY_MUTABLE_DEFAULT = [] + + +class A: + mutable_default: list[int] = [] + immutable_annotation: typing.Sequence[int] = [] + without_annotation = [] + ignored_via_comment: list[int] = [] # noqa: RUF010 + correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT + + +class B: + mutable_default: list[int] = [] + immutable_annotation: Sequence[int] = [] + without_annotation = [] + ignored_via_comment: list[int] = [] # noqa: RUF010 + correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT diff --git a/crates/ruff/resources/test/fixtures/ruff/RUF011.py b/crates/ruff/resources/test/fixtures/ruff/RUF011.py new file mode 100644 index 0000000000000..d6454b174c66d --- /dev/null +++ b/crates/ruff/resources/test/fixtures/ruff/RUF011.py @@ -0,0 +1,30 @@ +import datetime +import typing +from typing import NamedTuple +from dataclasses import field + +def default_function() -> list[int]: + return [] + + +class ImmutableType(NamedTuple): + something: int = 8 + + +class A: + hidden_mutable_default: list[int] = default_function() + class_variable: typing.ClassVar[list[int]] = default_function() + fine_date: datetime.date = datetime.date(2042, 1, 1) + + +DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES = ImmutableType(40) +DEFAULT_A_FOR_ALL_DATACLASSES = A([1, 2, 3]) + + +class B: + hidden_mutable_default: list[int] = default_function() + another_class: A = A() + not_optimal: ImmutableType = ImmutableType(20) + good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES + okay_variant: A = DEFAULT_A_FOR_ALL_DATACLASSES + not_fine_field: list[int] = field(default_vactory=list) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 075cada62f581..2878bf9e214c6 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -836,19 +836,45 @@ where if self.settings.rules.any_enabled(&[ Rule::MutableDataclassDefault, + Rule::MutableClassDefault, Rule::FunctionCallInDataclassDefaultArgument, - ]) && ruff::rules::is_dataclass(self, decorator_list) - { - if self.settings.rules.enabled(Rule::MutableDataclassDefault) { - ruff::rules::mutable_dataclass_default(self, body); + Rule::FunctionCallInClassDefaultArgument, + ]) { + let is_dataclass = ruff::rules::is_dataclass(self, decorator_list); + + if is_dataclass { + if self.settings.rules.enabled(Rule::MutableDataclassDefault) { + ruff::rules::mutable_class_default(self, true, body); + } + + if self + .settings + .rules + .enabled(Rule::FunctionCallInDataclassDefaultArgument) + { + ruff::rules::function_call_in_class_defaults( + self, + body, + is_dataclass, + true, + ); + } + } + if self.settings.rules.enabled(Rule::MutableClassDefault) { + ruff::rules::mutable_class_default(self, false, body); } if self .settings .rules - .enabled(Rule::FunctionCallInDataclassDefaultArgument) + .enabled(Rule::FunctionCallInClassDefaultArgument) { - ruff::rules::function_call_in_dataclass_defaults(self, body); + ruff::rules::function_call_in_class_defaults( + self, + body, + is_dataclass, + false, + ); } } diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 54e64c1781746..2ee39245d6c35 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -726,6 +726,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Ruff, "007") => Rule::PairwiseOverZipped, (Ruff, "008") => Rule::MutableDataclassDefault, (Ruff, "009") => Rule::FunctionCallInDataclassDefaultArgument, + (Ruff, "010") => Rule::MutableClassDefault, + (Ruff, "011") => Rule::FunctionCallInClassDefaultArgument, (Ruff, "100") => Rule::UnusedNOQA, // flake8-django diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index db7b827a3904a..b04480844e93b 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -662,6 +662,8 @@ ruff_macros::register_rules!( rules::ruff::rules::PairwiseOverZipped, rules::ruff::rules::MutableDataclassDefault, rules::ruff::rules::FunctionCallInDataclassDefaultArgument, + rules::ruff::rules::MutableClassDefault, + rules::ruff::rules::FunctionCallInClassDefaultArgument, // flake8-django rules::flake8_django::rules::DjangoNullableModelStringField, rules::flake8_django::rules::DjangoLocalsInRenderFunction, diff --git a/crates/ruff/src/rules/ruff/mod.rs b/crates/ruff/src/rules/ruff/mod.rs index 0be811b724722..b3812cbae21dc 100644 --- a/crates/ruff/src/rules/ruff/mod.rs +++ b/crates/ruff/src/rules/ruff/mod.rs @@ -155,7 +155,19 @@ mod tests { #[test_case(Rule::MutableDataclassDefault, Path::new("RUF008.py"); "RUF008")] #[test_case(Rule::FunctionCallInDataclassDefaultArgument, Path::new("RUF009.py"); "RUF009")] - fn mutable_defaults(rule_code: Rule, path: &Path) -> Result<()> { + fn mutable_dataclass_defaults(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); + let diagnostics = test_path( + Path::new("ruff").join(path).as_path(), + &settings::Settings::for_rule(rule_code), + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } + + #[test_case(Rule::MutableClassDefault, Path::new("RUF010.py"); "RUF010")] + #[test_case(Rule::FunctionCallInClassDefaultArgument, Path::new("RUF011.py"); "RUF011")] + fn mutable_class_defaults(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( Path::new("ruff").join(path).as_path(), diff --git a/crates/ruff/src/rules/ruff/rules/mod.rs b/crates/ruff/src/rules/ruff/rules/mod.rs index 089bceaaf4737..a7cedb7e2a8ad 100644 --- a/crates/ruff/src/rules/ruff/rules/mod.rs +++ b/crates/ruff/src/rules/ruff/rules/mod.rs @@ -2,7 +2,7 @@ mod ambiguous_unicode_character; mod asyncio_dangling_task; mod collection_literal_concatenation; mod confusables; -mod mutable_defaults_in_dataclass_fields; +mod mutable_defaults_in_class_fields; mod pairwise_over_zipped; mod unused_noqa; @@ -14,9 +14,10 @@ pub use asyncio_dangling_task::{asyncio_dangling_task, AsyncioDanglingTask}; pub use collection_literal_concatenation::{ collection_literal_concatenation, CollectionLiteralConcatenation, }; -pub use mutable_defaults_in_dataclass_fields::{ - function_call_in_dataclass_defaults, is_dataclass, mutable_dataclass_default, - FunctionCallInDataclassDefaultArgument, MutableDataclassDefault, +pub use mutable_defaults_in_class_fields::{ + function_call_in_class_defaults, is_dataclass, mutable_class_default, + FunctionCallInClassDefaultArgument, FunctionCallInDataclassDefaultArgument, + MutableClassDefault, MutableDataclassDefault, }; pub use pairwise_over_zipped::{pairwise_over_zipped, PairwiseOverZipped}; pub use unused_noqa::{UnusedCodes, UnusedNOQA}; diff --git a/crates/ruff/src/rules/ruff/rules/mutable_defaults_in_dataclass_fields.rs b/crates/ruff/src/rules/ruff/rules/mutable_defaults_in_class_fields.rs similarity index 71% rename from crates/ruff/src/rules/ruff/rules/mutable_defaults_in_dataclass_fields.rs rename to crates/ruff/src/rules/ruff/rules/mutable_defaults_in_class_fields.rs index 628326ad6b7e8..ae3bf845f9dd7 100644 --- a/crates/ruff/src/rules/ruff/rules/mutable_defaults_in_dataclass_fields.rs +++ b/crates/ruff/src/rules/ruff/rules/mutable_defaults_in_class_fields.rs @@ -63,6 +63,17 @@ impl Violation for MutableDataclassDefault { } } +/// This rule is same as MutableDataclassDefault, but for any class. The same arguments apply. +#[violation] +pub struct MutableClassDefault; + +impl Violation for MutableClassDefault { + #[derive_message_formats] + fn message(&self) -> String { + format!("Do not use mutable default values for class attributes") + } +} + /// ## What it does /// Checks for function calls in dataclass defaults. /// @@ -148,6 +159,25 @@ impl Violation for FunctionCallInDataclassDefaultArgument { } } +/// Same as FunctionCallInDataclassDefaultArgument, but for any class. +/// Importantly, this error will be issued on calls to dataclasses.field +#[violation] +pub struct FunctionCallInClassDefaultArgument { + pub name: Option, +} + +impl Violation for FunctionCallInClassDefaultArgument { + #[derive_message_formats] + fn message(&self) -> String { + let FunctionCallInClassDefaultArgument { name } = self; + if let Some(name) = name { + format!("Do not perform function call `{name}` in non-dataclass attribute defaults") + } else { + format!("Do not perform function call in non-dataclass attribute defaults") + } + } +} + fn is_mutable_expr(expr: &Expr) -> bool { matches!( &expr.node, @@ -178,8 +208,13 @@ fn is_class_var_annotation(context: &Context, annotation: &Expr) -> bool { context.match_typing_expr(value, "ClassVar") } -/// RUF009 -pub fn function_call_in_dataclass_defaults(checker: &mut Checker, body: &[Stmt]) { +/// RUF009/RUF011 +pub fn function_call_in_class_defaults( + checker: &mut Checker, + body: &[Stmt], + is_dataclass: bool, + emit_dataclass_error: bool, +) { let extend_immutable_calls: Vec = checker .settings .flake8_bugbear @@ -199,23 +234,41 @@ pub fn function_call_in_dataclass_defaults(checker: &mut Checker, body: &[Stmt]) continue; } if let ExprKind::Call { func, .. } = &expr.node { - if !is_immutable_func(&checker.ctx, func, &extend_immutable_calls) - && !is_allowed_dataclass_function(&checker.ctx, func) + if !(is_immutable_func(&checker.ctx, func, &extend_immutable_calls) + || is_dataclass && is_allowed_dataclass_function(&checker.ctx, func)) { - checker.diagnostics.push(Diagnostic::new( - FunctionCallInDataclassDefaultArgument { - name: compose_call_path(func), - }, - expr.range(), - )); + let diagnostic: Diagnostic = if emit_dataclass_error { + Diagnostic::new( + FunctionCallInDataclassDefaultArgument { + name: compose_call_path(func), + }, + expr.range(), + ) + } else { + Diagnostic::new( + FunctionCallInClassDefaultArgument { + name: compose_call_path(func), + }, + expr.range(), + ) + }; + checker.diagnostics.push(diagnostic); } } } } } -/// RUF008 -pub fn mutable_dataclass_default(checker: &mut Checker, body: &[Stmt]) { +/// RUF008/RUF010 +pub fn mutable_class_default(checker: &mut Checker, emit_dataclass_error: bool, body: &[Stmt]) { + fn diagnostic(emit_dataclass_error: bool, value: &Expr) -> Diagnostic { + if emit_dataclass_error { + Diagnostic::new(MutableDataclassDefault, value.range()) + } else { + Diagnostic::new(MutableClassDefault, value.range()) + } + } + for statement in body { match &statement.node { StmtKind::AnnAssign { @@ -229,14 +282,14 @@ pub fn mutable_dataclass_default(checker: &mut Checker, body: &[Stmt]) { { checker .diagnostics - .push(Diagnostic::new(MutableDataclassDefault, value.range())); + .push(diagnostic(emit_dataclass_error, value)); } } StmtKind::Assign { value, .. } => { if is_mutable_expr(value) { checker .diagnostics - .push(Diagnostic::new(MutableDataclassDefault, value.range())); + .push(diagnostic(emit_dataclass_error, value)); } } _ => (), diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF010_RUF010.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF010_RUF010.py.snap new file mode 100644 index 0000000000000..f1db232d9fc2f --- /dev/null +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF010_RUF010.py.snap @@ -0,0 +1,42 @@ +--- +source: crates/ruff/src/rules/ruff/mod.rs +--- +RUF010.py:8:34: RUF010 Do not use mutable default values for class attributes + | + 8 | class A: + 9 | mutable_default: list[int] = [] + | ^^ RUF010 +10 | immutable_annotation: typing.Sequence[int] = [] +11 | without_annotation = [] + | + +RUF010.py:10:26: RUF010 Do not use mutable default values for class attributes + | +10 | mutable_default: list[int] = [] +11 | immutable_annotation: typing.Sequence[int] = [] +12 | without_annotation = [] + | ^^ RUF010 +13 | ignored_via_comment: list[int] = [] # noqa: RUF010 +14 | correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT + | + +RUF010.py:16:34: RUF010 Do not use mutable default values for class attributes + | +16 | class B: +17 | mutable_default: list[int] = [] + | ^^ RUF010 +18 | immutable_annotation: Sequence[int] = [] +19 | without_annotation = [] + | + +RUF010.py:18:26: RUF010 Do not use mutable default values for class attributes + | +18 | mutable_default: list[int] = [] +19 | immutable_annotation: Sequence[int] = [] +20 | without_annotation = [] + | ^^ RUF010 +21 | ignored_via_comment: list[int] = [] # noqa: RUF010 +22 | correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT + | + + diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF011_RUF011.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF011_RUF011.py.snap new file mode 100644 index 0000000000000..3d1fad7ed199e --- /dev/null +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF011_RUF011.py.snap @@ -0,0 +1,50 @@ +--- +source: crates/ruff/src/rules/ruff/mod.rs +--- +RUF011.py:15:41: RUF011 Do not perform function call `default_function` in non-dataclass attribute defaults + | +15 | class A: +16 | hidden_mutable_default: list[int] = default_function() + | ^^^^^^^^^^^^^^^^^^ RUF011 +17 | class_variable: typing.ClassVar[list[int]] = default_function() +18 | fine_date: datetime.date = datetime.date(2042, 1, 1) + | + +RUF011.py:25:41: RUF011 Do not perform function call `default_function` in non-dataclass attribute defaults + | +25 | class B: +26 | hidden_mutable_default: list[int] = default_function() + | ^^^^^^^^^^^^^^^^^^ RUF011 +27 | another_class: A = A() +28 | not_optimal: ImmutableType = ImmutableType(20) + | + +RUF011.py:26:24: RUF011 Do not perform function call `A` in non-dataclass attribute defaults + | +26 | class B: +27 | hidden_mutable_default: list[int] = default_function() +28 | another_class: A = A() + | ^^^ RUF011 +29 | not_optimal: ImmutableType = ImmutableType(20) +30 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES + | + +RUF011.py:27:34: RUF011 Do not perform function call `ImmutableType` in non-dataclass attribute defaults + | +27 | hidden_mutable_default: list[int] = default_function() +28 | another_class: A = A() +29 | not_optimal: ImmutableType = ImmutableType(20) + | ^^^^^^^^^^^^^^^^^ RUF011 +30 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES +31 | okay_variant: A = DEFAULT_A_FOR_ALL_DATACLASSES + | + +RUF011.py:30:33: RUF011 Do not perform function call `field` in non-dataclass attribute defaults + | +30 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES +31 | okay_variant: A = DEFAULT_A_FOR_ALL_DATACLASSES +32 | not_fine_field: list[int] = field(default_vactory=list) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF011 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index b78d6161fb6a1..94bfcca876310 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2175,6 +2175,9 @@ "RUF007", "RUF008", "RUF009", + "RUF01", + "RUF010", + "RUF011", "RUF1", "RUF10", "RUF100",