Expand RUF008 to all classes, but to a new code (RUF012) (#4390)

AFAIK, there is no reason to limit RUF008 to just dataclasses -- mutable
defaults have the same problems for regular classes.

Partially addresses #4053
and broken out from #4096.

---------

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

crates/ruff/resources/test/fixtures/ruff/RUF012.py

@@ -0,0 +1,22 @@
+import typing
+from typing import ClassVar, Sequence
+
+KNOWINGLY_MUTABLE_DEFAULT = []
+
+
+class A:
+    mutable_default: list[int] = []
+    immutable_annotation: typing.Sequence[int] = []
+    without_annotation = []
+    ignored_via_comment: list[int] = []  # noqa: RUF012
+    correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
+    class_variable: typing.ClassVar[list[int]] = []
+
+
+class B:
+    mutable_default: list[int] = []
+    immutable_annotation: Sequence[int] = []
+    without_annotation = []
+    ignored_via_comment: list[int] = []  # noqa: RUF012
+    correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
+    class_variable: ClassVar[list[int]] = []

crates/ruff/src/checkers/ast/mod.rs

@@ -794,13 +794,16 @@ where
                 if self.any_enabled(&[
                     Rule::MutableDataclassDefault,
                     Rule::FunctionCallInDataclassDefaultArgument,
-                ]) && ruff::rules::is_dataclass(&self.semantic_model, decorator_list)
-                {
-                    if self.enabled(Rule::MutableDataclassDefault) {
-                        ruff::rules::mutable_dataclass_default(self, body);
+                    Rule::MutableClassDefault,
+                ]) {
+                    let is_dataclass =
+                        ruff::rules::is_dataclass(&self.semantic_model, decorator_list);
+                    if self.any_enabled(&[Rule::MutableDataclassDefault, Rule::MutableClassDefault])
+                    {
+                        ruff::rules::mutable_class_default(self, body, is_dataclass);
                     }
 
-                    if self.enabled(Rule::FunctionCallInDataclassDefaultArgument) {
+                    if is_dataclass && self.enabled(Rule::FunctionCallInDataclassDefaultArgument) {
                         ruff::rules::function_call_in_dataclass_defaults(self, body);
                     }
                 }

crates/ruff/src/codes.rs

@@ -752,6 +752,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "009") => (RuleGroup::Unspecified, rules::ruff::rules::FunctionCallInDataclassDefaultArgument),
         (Ruff, "010") => (RuleGroup::Unspecified, rules::ruff::rules::ExplicitFStringTypeConversion),
         (Ruff, "011") => (RuleGroup::Unspecified, rules::ruff::rules::StaticKeyDictComprehension),
+        (Ruff, "012") => (RuleGroup::Unspecified, rules::ruff::rules::MutableClassDefault),
         (Ruff, "100") => (RuleGroup::Unspecified, rules::ruff::rules::UnusedNOQA),
         (Ruff, "200") => (RuleGroup::Unspecified, rules::ruff::rules::InvalidPyprojectToml),
 

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

@@ -170,7 +170,18 @@ mod tests {
 
     #[test_case(Rule::MutableDataclassDefault, Path::new("RUF008.py"))]
     #[test_case(Rule::FunctionCallInDataclassDefaultArgument, Path::new("RUF009.py"))]
-    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("RUF012.py"))]
+    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(),

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

@@ -3,7 +3,7 @@ pub(crate) use asyncio_dangling_task::*;
 pub(crate) use collection_literal_concatenation::*;
 pub(crate) use explicit_f_string_type_conversion::*;
 pub(crate) use invalid_pyproject_toml::InvalidPyprojectToml;
-pub(crate) use mutable_defaults_in_dataclass_fields::*;
+pub(crate) use mutable_defaults_in_class_fields::*;
 pub(crate) use pairwise_over_zipped::*;
 pub(crate) use unused_noqa::*;
 
@@ -15,7 +15,7 @@ mod collection_literal_concatenation;
 mod confusables;
 mod explicit_f_string_type_conversion;
 mod invalid_pyproject_toml;
-mod mutable_defaults_in_dataclass_fields;
+mod mutable_defaults_in_class_fields;
 mod pairwise_over_zipped;
 mod static_key_dict_comprehension;
 mod unused_noqa;

crates/ruff/src/rules/ruff/rules/mutable_defaults_in_dataclass_fields.rs → crates/ruff/src/rules/ruff/rules/mutable_defaults_in_class_fields.rs

@@ -12,16 +12,18 @@ use ruff_python_semantic::{
 use crate::checkers::ast::Checker;
 
 /// ## What it does
-/// Checks for mutable default values in dataclasses without the use of
-/// `dataclasses.field`.
+/// Checks for mutable default values in dataclasses.
 ///
 /// ## Why is this bad?
 /// Mutable default values share state across all instances of the dataclass,
 /// while not being obvious. This can lead to bugs when the attributes are
 /// changed in one instance, as those changes will unexpectedly affect all
 /// other instances.
 ///
-/// ## Examples:
+/// Instead of sharing mutable defaults, use the `field(default_factory=...)`
+/// pattern.
+///
+/// ## Examples
 /// ```python
 /// from dataclasses import dataclass
 ///
@@ -40,26 +42,49 @@ use crate::checkers::ast::Checker;
 /// class A:
 ///     mutable_default: list[int] = field(default_factory=list)
 /// ```
+#[violation]
+pub struct MutableDataclassDefault;
+
+impl Violation for MutableDataclassDefault {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Do not use mutable default values for dataclass attributes")
+    }
+}
+
+/// ## What it does
+/// Checks for mutable default values in class attributes.
+///
+/// ## Why is this bad?
+/// Mutable default values share state across all instances of the class,
+/// while not being obvious. This can lead to bugs when the attributes are
+/// changed in one instance, as those changes will unexpectedly affect all
+/// other instances.
+///
+/// When mutable value are intended, they should be annotated with
+/// `typing.ClassVar`.
 ///
-/// Alternatively, if you _want_ shared behaviour, make it more obvious
-/// by assigning to a module-level variable:
+/// ## Examples
 /// ```python
-/// from dataclasses import dataclass
+/// class A:
+///     mutable_default: list[int] = []
+/// ```
 ///
-/// I_KNOW_THIS_IS_SHARED_STATE = [1, 2, 3, 4]
+/// Use instead:
+/// ```python
+/// from typing import ClassVar
 ///
 ///
-/// @dataclass
 /// class A:
-///     mutable_default: list[int] = I_KNOW_THIS_IS_SHARED_STATE
+///     mutable_default: ClassVar[list[int]] = []
 /// ```
 #[violation]
-pub struct MutableDataclassDefault;
+pub struct MutableClassDefault;
 
-impl Violation for MutableDataclassDefault {
+impl Violation for MutableClassDefault {
     #[derive_message_formats]
     fn message(&self) -> String {
-        format!("Do not use mutable default values for dataclass attributes")
+        format!("Do not use mutable default values for class attributes")
     }
 }
 
@@ -73,7 +98,7 @@ impl Violation for MutableDataclassDefault {
 /// ## Options
 /// - `flake8-bugbear.extend-immutable-calls`
 ///
-/// ## Examples:
+/// ## Examples
 /// ```python
 /// from dataclasses import dataclass
 ///
@@ -214,8 +239,15 @@ pub(crate) fn function_call_in_dataclass_defaults(checker: &mut Checker, body: &
     }
 }
 
-/// RUF008
-pub(crate) fn mutable_dataclass_default(checker: &mut Checker, body: &[Stmt]) {
+/// RUF008/RUF012
+pub(crate) fn mutable_class_default(checker: &mut Checker, body: &[Stmt], is_dataclass: bool) {
+    fn diagnostic(is_dataclass: bool, value: &Expr) -> Diagnostic {
+        if is_dataclass {
+            Diagnostic::new(MutableDataclassDefault, value.range())
+        } else {
+            Diagnostic::new(MutableClassDefault, value.range())
+        }
+    }
     for statement in body {
         match statement {
             Stmt::AnnAssign(ast::StmtAnnAssign {
@@ -227,16 +259,12 @@ pub(crate) fn mutable_dataclass_default(checker: &mut Checker, body: &[Stmt]) {
                     && !is_immutable_annotation(checker.semantic_model(), annotation)
                     && is_mutable_expr(value)
                 {
-                    checker
-                        .diagnostics
-                        .push(Diagnostic::new(MutableDataclassDefault, value.range()));
+                    checker.diagnostics.push(diagnostic(is_dataclass, value));
                 }
             }
             Stmt::Assign(ast::StmtAssign { value, .. }) => {
                 if is_mutable_expr(value) {
-                    checker
-                        .diagnostics
-                        .push(Diagnostic::new(MutableDataclassDefault, value.range()));
+                    checker.diagnostics.push(diagnostic(is_dataclass, value));
                 }
             }
             _ => (),

crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF012_RUF012.py.snap

@@ -0,0 +1,42 @@
+---
+source: crates/ruff/src/rules/ruff/mod.rs
+---
+RUF012.py:8:34: RUF012 Do not use mutable default values for class attributes
+   |
+ 7 | class A:
+ 8 |     mutable_default: list[int] = []
+   |                                  ^^ RUF012
+ 9 |     immutable_annotation: typing.Sequence[int] = []
+10 |     without_annotation = []
+   |
+
+RUF012.py:10:26: RUF012 Do not use mutable default values for class attributes
+   |
+ 8 |     mutable_default: list[int] = []
+ 9 |     immutable_annotation: typing.Sequence[int] = []
+10 |     without_annotation = []
+   |                          ^^ RUF012
+11 |     ignored_via_comment: list[int] = []  # noqa: RUF012
+12 |     correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
+   |
+
+RUF012.py:17:34: RUF012 Do not use mutable default values for class attributes
+   |
+16 | class B:
+17 |     mutable_default: list[int] = []
+   |                                  ^^ RUF012
+18 |     immutable_annotation: Sequence[int] = []
+19 |     without_annotation = []
+   |
+
+RUF012.py:19:26: RUF012 Do not use mutable default values for class attributes
+   |
+17 |     mutable_default: list[int] = []
+18 |     immutable_annotation: Sequence[int] = []
+19 |     without_annotation = []
+   |                          ^^ RUF012
+20 |     ignored_via_comment: list[int] = []  # noqa: RUF012
+21 |     correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
+   |
+
+