[flake8-bugbear] Implement class-as-data-structure (B903)

crates/ruff_linter/resources/test/fixtures/flake8_bugbear/class_as_data_structure.py

@@ -0,0 +1,98 @@
+from __future__ import annotations
+
+from dataclasses import dataclass
+
+
+class Point:  # B903
+    def __init__(self, x: float, y: float) -> None:
+        self.x = x
+        self.y = y
+
+
+class Rectangle:  # OK
+    def __init__(self, top_left: Point, bottom_right: Point) -> None:
+        ...
+
+    def area(self) -> float:
+        ...
+
+
+@dataclass
+class Circle:  # OK
+    center: Point
+    radius: float
+
+    def area(self) -> float:
+        ...
+
+
+class CustomException(Exception):  # OK
+    ...
+
+
+class A:   # OK
+    class B:
+        ...
+
+    def __init__(self):
+        ...
+
+class C:   # B903
+    c: int
+    def __init__(self,d:list):
+        self.d = d
+
+class D:   # B903
+    """This class has a docstring."""
+    # this next method is an init
+    def __init__(self,e:dict):
+        self.e = e
+
+# <--- begin flake8-bugbear tests below
+# (we have modified them to have type annotations,
+# since our implementation only triggers in that
+# stricter setting.)
+class NoWarningsMoreMethods:
+    def __init__(self, foo:int, bar:list):
+        self.foo = foo
+        self.bar = bar
+
+    def other_function(self): ...
+
+
+class NoWarningsClassAttributes:
+    spam = "ham"
+
+    def __init__(self, foo:int, bar:list):
+        self.foo = foo
+        self.bar = bar
+
+
+class NoWarningsComplicatedAssignment:
+    def __init__(self, foo:int, bar:list):
+        self.foo = foo
+        self.bar = bar
+        self.spam = " - ".join([foo, bar])
+
+
+class NoWarningsMoreStatements:
+    def __init__(self, foo:int, bar:list):
+        foo = " - ".join([foo, bar])
+        self.foo = foo
+        self.bar = bar
+
+
+class Warnings:
+    def __init__(self, foo:int, bar:list):
+        self.foo = foo
+        self.bar = bar
+
+
+class WarningsWithDocstring:
+    """A docstring should not be an impediment to a warning"""
+
+    def __init__(self, foo:int, bar:list):
+        self.foo = foo
+        self.bar = bar
+
+# <-- end flake8-bugbear tests

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

@@ -431,6 +431,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             if checker.enabled(Rule::EqWithoutHash) {
                 pylint::rules::object_without_hash_method(checker, class_def);
             }
+            if checker.enabled(Rule::ClassAsDataStructure) {
+                flake8_bugbear::rules::class_as_data_structure(checker, class_def);
+            }
             if checker.enabled(Rule::TooManyPublicMethods) {
                 pylint::rules::too_many_public_methods(
                     checker,

crates/ruff_linter/src/codes.rs

@@ -356,6 +356,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Bugbear, "035") => (RuleGroup::Stable, rules::flake8_bugbear::rules::StaticKeyDictComprehension),
         (Flake8Bugbear, "039") => (RuleGroup::Stable, rules::flake8_bugbear::rules::MutableContextvarDefault),
         (Flake8Bugbear, "901") => (RuleGroup::Preview, rules::flake8_bugbear::rules::ReturnInGenerator),
+        (Flake8Bugbear, "903") => (RuleGroup::Preview, rules::flake8_bugbear::rules::ClassAsDataStructure),
         (Flake8Bugbear, "904") => (RuleGroup::Stable, rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept),
         (Flake8Bugbear, "905") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ZipWithoutExplicitStrict),
         (Flake8Bugbear, "909") => (RuleGroup::Preview, rules::flake8_bugbear::rules::LoopIteratorMutation),

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

@@ -21,6 +21,7 @@ mod tests {
     #[test_case(Rule::AssertRaisesException, Path::new("B017.py"))]
     #[test_case(Rule::AssignmentToOsEnviron, Path::new("B003.py"))]
     #[test_case(Rule::CachedInstanceMethod, Path::new("B019.py"))]
+    #[test_case(Rule::ClassAsDataStructure, Path::new("class_as_data_structure.py"))]
     #[test_case(Rule::DuplicateHandlerException, Path::new("B014.py"))]
     #[test_case(Rule::DuplicateTryBlockException, Path::new("B025.py"))]
     #[test_case(Rule::DuplicateValue, Path::new("B033.py"))]

crates/ruff_linter/src/rules/flake8_bugbear/rules/class_as_data_structure.rs

@@ -0,0 +1,127 @@
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, ViolationMetadata};
+use ruff_python_ast::{self as ast};
+use ruff_python_semantic::analyze::visibility::{self, Visibility::Public};
+use ruff_text_size::Ranged;
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for classes that only have a public `__init__` method,
+/// without base classes and decorators.
+///
+/// ## Why is this bad?
+/// Classes with just an `__init__` are possibly better off
+/// being a dataclass or a namedtuple, which have less boilerplate.
+///
+/// ## Example
+/// ```python
+/// class Point:
+///     def __init__(self, x: float, y: float):
+///         self.x = x
+///         self.y = y
+/// ```
+///
+/// Use instead:
+/// ```python
+/// from dataclasses import dataclass
+///
+///
+/// @dataclass
+/// class Point:
+///     x: float
+///     y: float
+/// ```
+#[derive(ViolationMetadata)]
+pub(crate) struct ClassAsDataStructure;
+
+impl Violation for ClassAsDataStructure {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        "Class could be dataclass or namedtuple".to_string()
+    }
+}
+
+/// B903
+pub(crate) fn class_as_data_structure(checker: &mut Checker, class_def: &ast::StmtClassDef) {
+    // skip stub files
+    if checker.source_type.is_stub() {
+        return;
+    }
+
+    // allow decorated classes
+    if !class_def.decorator_list.is_empty() {
+        return;
+    }
+
+    // allow classes with base classes
+    if class_def.arguments.is_some() {
+        return;
+    }
+
+    let mut public_methods = 0;
+    let mut has_dunder_init = false;
+
+    for stmt in &class_def.body {
+        if public_methods > 1 && has_dunder_init {
+            // we're good to break here
+            break;
+        }
+        match stmt {
+            ast::Stmt::FunctionDef(func_def) => {
+                if !has_dunder_init
+                    && func_def.name.to_string() == "__init__"
+                    && func_def
+                        .parameters
+                        .iter()
+                        // skip `self`
+                        .skip(1)
+                        .all(|param| param.annotation().is_some() && !param.is_variadic())
+                    // `__init__` should not have complicated logic in it
+                    // only assignments
+                    && func_def
+                        .body
+                        .iter()
+                        .all(is_simple_assignment_to_attribute)
+                {
+                    has_dunder_init = true;
+                }
+                if matches!(visibility::method_visibility(func_def), Public) {
+                    public_methods += 1;
+                }
+            }
+            // Ignore class variables
+            ast::Stmt::Assign(_) | ast::Stmt::AnnAssign(_) |
+            // and expressions (e.g. string literals)
+            ast::Stmt::Expr(_) => {}
+            _ => {
+                // Bail for anything else - e.g. nested classes
+                // or conditional methods.
+                return;
+            }
+        }
+    }
+
+    if has_dunder_init && public_methods == 1 {
+        checker
+            .diagnostics
+            .push(Diagnostic::new(ClassAsDataStructure, class_def.range()));
+    }
+}
+
+// Checks whether a statement is a, possibly augmented,
+// assignment of a name to an attribute.
+fn is_simple_assignment_to_attribute(stmt: &ast::Stmt) -> bool {
+    match stmt {
+        ast::Stmt::Assign(ast::StmtAssign { targets, value, .. }) => {
+            let [target] = targets.as_slice() else {
+                return false;
+            };
+            target.is_attribute_expr() && value.is_name_expr()
+        }
+        ast::Stmt::AnnAssign(ast::StmtAnnAssign { target, value, .. }) => {
+            target.is_attribute_expr() && value.as_ref().is_some_and(|val| val.is_name_expr())
+        }
+        _ => false,
+    }
+}

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

@@ -4,6 +4,7 @@ pub(crate) use assert_raises_exception::*;
 pub(crate) use assignment_to_os_environ::*;
 pub(crate) use batched_without_explicit_strict::*;
 pub(crate) use cached_instance_method::*;
+pub(crate) use class_as_data_structure::*;
 pub(crate) use duplicate_exceptions::*;
 pub(crate) use duplicate_value::*;
 pub(crate) use except_with_empty_tuple::*;
@@ -43,6 +44,7 @@ mod assert_raises_exception;
 mod assignment_to_os_environ;
 mod batched_without_explicit_strict;
 mod cached_instance_method;
+mod class_as_data_structure;
 mod duplicate_exceptions;
 mod duplicate_value;
 mod except_with_empty_tuple;

crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B903_class_as_data_structure.py.snap

@@ -0,0 +1,71 @@
+---
+source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
+---
+class_as_data_structure.py:6:1: B903 Class could be dataclass or namedtuple
+  |
+6 | / class Point:  # B903
+7 | |     def __init__(self, x: float, y: float) -> None:
+8 | |         self.x = x
+9 | |         self.y = y
+  | |__________________^ B903
+  |
+
+class_as_data_structure.py:40:1: B903 Class could be dataclass or namedtuple
+   |
+38 |           ...
+39 |   
+40 | / class C:   # B903
+41 | |     c: int
+42 | |     def __init__(self,d:list):
+43 | |         self.d = d
+   | |__________________^ B903
+44 |   
+45 |   class D:   # B903
+   |
+
+class_as_data_structure.py:45:1: B903 Class could be dataclass or namedtuple
+   |
+43 |           self.d = d
+44 |   
+45 | / class D:   # B903
+46 | |     """This class has a docstring."""
+47 | |     # this next method is an init
+48 | |     def __init__(self,e:dict):
+49 | |         self.e = e
+   | |__________________^ B903
+50 |   
+51 |   # <--- begin flake8-bugbear tests below
+   |
+
+class_as_data_structure.py:63:1: B903 Class could be dataclass or namedtuple
+   |
+63 | / class NoWarningsClassAttributes:
+64 | |     spam = "ham"
+65 | | 
+66 | |     def __init__(self, foo:int, bar:list):
+67 | |         self.foo = foo
+68 | |         self.bar = bar
+   | |______________________^ B903
+   |
+
+class_as_data_structure.py:85:1: B903 Class could be dataclass or namedtuple
+   |
+85 | / class Warnings:
+86 | |     def __init__(self, foo:int, bar:list):
+87 | |         self.foo = foo
+88 | |         self.bar = bar
+   | |______________________^ B903
+   |
+
+class_as_data_structure.py:91:1: B903 Class could be dataclass or namedtuple
+   |
+91 | / class WarningsWithDocstring:
+92 | |     """A docstring should not be an impediment to a warning"""
+93 | | 
+94 | |     def __init__(self, foo:int, bar:list):
+95 | |         self.foo = foo
+96 | |         self.bar = bar
+   | |______________________^ B903
+97 |   
+98 |   # <-- end flake8-bugbear tests
+   |

crates/ruff_workspace/src/configuration.rs

@@ -1571,7 +1571,7 @@ mod tests {
     const PREVIEW_RULES: &[Rule] = &[
         Rule::ReimplementedStarmap,
         Rule::SliceCopy,
-        Rule::TooManyPublicMethods,
+        Rule::ClassAsDataStructure,
         Rule::TooManyPublicMethods,
         Rule::UnnecessaryEnumerate,
         Rule::MathConstant,