Respect __str__ definitions from super classes (astral-sh#9338)

Closes astral-sh#9242.

crates/ruff_linter/resources/test/fixtures/flake8_django/DJ008.py

@@ -165,3 +165,19 @@ def my_brand_new_property(self):
 
     def my_beautiful_method(self):
         return 2
+
+
+# Subclass with its own __str__
+class SubclassTestModel1(TestModel1):
+    def __str__(self):
+        return self.new_field
+
+
+# Subclass with inherited __str__
+class SubclassTestModel2(TestModel4):
+    pass
+
+
+# Subclass without __str__
+class SubclassTestModel3(TestModel1):
+    pass

crates/ruff_linter/src/rules/flake8_django/rules/helpers.rs

@@ -4,14 +4,14 @@ use ruff_python_semantic::{analyze, SemanticModel};
 
 /// Return `true` if a Python class appears to be a Django model, based on its base classes.
 pub(super) fn is_model(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool {
-    analyze::class::any_over_body(class_def, semantic, &|call_path| {
+    analyze::class::any_call_path(class_def, semantic, &|call_path| {
         matches!(call_path.as_slice(), ["django", "db", "models", "Model"])
     })
 }
 
 /// Return `true` if a Python class appears to be a Django model form, based on its base classes.
 pub(super) fn is_model_form(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool {
-    analyze::class::any_over_body(class_def, semantic, &|call_path| {
+    analyze::class::any_call_path(class_def, semantic, &|call_path| {
         matches!(
             call_path.as_slice(),
             ["django", "forms", "ModelForm"] | ["django", "forms", "models", "ModelForm"]

crates/ruff_linter/src/rules/flake8_django/rules/model_without_dunder_str.rs

@@ -3,7 +3,7 @@ use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::helpers::is_const_true;
 use ruff_python_ast::identifier::Identifier;
 use ruff_python_ast::{self as ast, Expr, Stmt};
-use ruff_python_semantic::SemanticModel;
+use ruff_python_semantic::{analyze, SemanticModel};
 
 use crate::checkers::ast::Checker;
 
@@ -55,20 +55,24 @@ pub(crate) fn model_without_dunder_str(checker: &mut Checker, class_def: &ast::S
     if !is_non_abstract_model(class_def, checker.semantic()) {
         return;
     }
-    if has_dunder_method(class_def) {
+
+    if has_dunder_method(class_def, checker.semantic()) {
         return;
     }
+
     checker.diagnostics.push(Diagnostic::new(
         DjangoModelWithoutDunderStr,
         class_def.identifier(),
     ));
 }
 
 /// Returns `true` if the class has `__str__` method.
-fn has_dunder_method(class_def: &ast::StmtClassDef) -> bool {
-    class_def.body.iter().any(|val| match val {
-        Stmt::FunctionDef(ast::StmtFunctionDef { name, .. }) => name == "__str__",
-        _ => false,
+fn has_dunder_method(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool {
+    analyze::class::any_super_class(class_def, semantic, &|class_def| {
+        class_def.body.iter().any(|val| match val {
+            Stmt::FunctionDef(ast::StmtFunctionDef { name, .. }) => name == "__str__",
+            _ => false,
+        })
     })
 }
 

crates/ruff_linter/src/rules/flake8_django/rules/nullable_model_string_field.rs

@@ -1,8 +1,9 @@
-use ruff_python_ast::{self as ast, Arguments, Expr, Stmt};
+use ruff_python_ast::{self as ast, Expr, Stmt};
 
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::helpers::is_const_true;
+use ruff_python_semantic::SemanticModel;
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
@@ -48,7 +49,7 @@ impl Violation for DjangoNullableModelStringField {
     #[derive_message_formats]
     fn message(&self) -> String {
         let DjangoNullableModelStringField { field_name } = self;
-        format!("Avoid using `null=True` on string-based fields such as {field_name}")
+        format!("Avoid using `null=True` on string-based fields such as `{field_name}`")
     }
 }
 
@@ -58,7 +59,7 @@ pub(crate) fn nullable_model_string_field(checker: &mut Checker, body: &[Stmt])
         let Stmt::Assign(ast::StmtAssign { value, .. }) = statement else {
             continue;
         };
-        if let Some(field_name) = is_nullable_field(checker, value) {
+        if let Some(field_name) = is_nullable_field(value, checker.semantic()) {
             checker.diagnostics.push(Diagnostic::new(
                 DjangoNullableModelStringField {
                     field_name: field_name.to_string(),
@@ -69,22 +70,12 @@ pub(crate) fn nullable_model_string_field(checker: &mut Checker, body: &[Stmt])
     }
 }
 
-fn is_nullable_field<'a>(checker: &'a Checker, value: &'a Expr) -> Option<&'a str> {
-    let Expr::Call(ast::ExprCall {
-        func,
-        arguments: Arguments { keywords, .. },
-        ..
-    }) = value
-    else {
-        return None;
-    };
-
-    let Some(valid_field_name) = helpers::get_model_field_name(func, checker.semantic()) else {
-        return None;
-    };
+fn is_nullable_field<'a>(value: &'a Expr, semantic: &'a SemanticModel) -> Option<&'a str> {
+    let call = value.as_call_expr()?;
 
+    let field_name = helpers::get_model_field_name(&call.func, semantic)?;
     if !matches!(
-        valid_field_name,
+        field_name,
         "CharField" | "TextField" | "SlugField" | "EmailField" | "FilePathField" | "URLField"
     ) {
         return None;
@@ -93,7 +84,7 @@ fn is_nullable_field<'a>(checker: &'a Checker, value: &'a Expr) -> Option<&'a st
     let mut null_key = false;
     let mut blank_key = false;
     let mut unique_key = false;
-    for keyword in keywords {
+    for keyword in &call.arguments.keywords {
         let Some(argument) = &keyword.arg else {
             continue;
         };
@@ -113,5 +104,6 @@ fn is_nullable_field<'a>(checker: &'a Checker, value: &'a Expr) -> Option<&'a st
     if !null_key {
         return None;
     }
-    Some(valid_field_name)
+
+    Some(field_name)
 }

crates/ruff_linter/src/rules/flake8_django/snapshots/ruff_linter__rules__flake8_django__tests__DJ001_DJ001.py.snap

@@ -1,7 +1,7 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_django/mod.rs
 ---
-DJ001.py:7:17: DJ001 Avoid using `null=True` on string-based fields such as CharField
+DJ001.py:7:17: DJ001 Avoid using `null=True` on string-based fields such as `CharField`
   |
 6 | class IncorrectModel(models.Model):
 7 |     charfield = models.CharField(max_length=255, null=True)
@@ -10,7 +10,7 @@ DJ001.py:7:17: DJ001 Avoid using `null=True` on string-based fields such as Char
 9 |     slugfield = models.SlugField(max_length=255, null=True)
   |
 
-DJ001.py:8:17: DJ001 Avoid using `null=True` on string-based fields such as TextField
+DJ001.py:8:17: DJ001 Avoid using `null=True` on string-based fields such as `TextField`
    |
  6 | class IncorrectModel(models.Model):
  7 |     charfield = models.CharField(max_length=255, null=True)
@@ -20,7 +20,7 @@ DJ001.py:8:17: DJ001 Avoid using `null=True` on string-based fields such as Text
 10 |     emailfield = models.EmailField(max_length=255, null=True)
    |
 
-DJ001.py:9:17: DJ001 Avoid using `null=True` on string-based fields such as SlugField
+DJ001.py:9:17: DJ001 Avoid using `null=True` on string-based fields such as `SlugField`
    |
  7 |     charfield = models.CharField(max_length=255, null=True)
  8 |     textfield = models.TextField(max_length=255, null=True)
@@ -30,7 +30,7 @@ DJ001.py:9:17: DJ001 Avoid using `null=True` on string-based fields such as Slug
 11 |     filepathfield = models.FilePathField(max_length=255, null=True)
    |
 
-DJ001.py:10:18: DJ001 Avoid using `null=True` on string-based fields such as EmailField
+DJ001.py:10:18: DJ001 Avoid using `null=True` on string-based fields such as `EmailField`
    |
  8 |     textfield = models.TextField(max_length=255, null=True)
  9 |     slugfield = models.SlugField(max_length=255, null=True)
@@ -40,7 +40,7 @@ DJ001.py:10:18: DJ001 Avoid using `null=True` on string-based fields such as Ema
 12 |     urlfield = models.URLField(max_length=255, null=True)
    |
 
-DJ001.py:11:21: DJ001 Avoid using `null=True` on string-based fields such as FilePathField
+DJ001.py:11:21: DJ001 Avoid using `null=True` on string-based fields such as `FilePathField`
    |
  9 |     slugfield = models.SlugField(max_length=255, null=True)
 10 |     emailfield = models.EmailField(max_length=255, null=True)
@@ -49,15 +49,15 @@ DJ001.py:11:21: DJ001 Avoid using `null=True` on string-based fields such as Fil
 12 |     urlfield = models.URLField(max_length=255, null=True)
    |
 
-DJ001.py:12:16: DJ001 Avoid using `null=True` on string-based fields such as URLField
+DJ001.py:12:16: DJ001 Avoid using `null=True` on string-based fields such as `URLField`
    |
 10 |     emailfield = models.EmailField(max_length=255, null=True)
 11 |     filepathfield = models.FilePathField(max_length=255, null=True)
 12 |     urlfield = models.URLField(max_length=255, null=True)
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ DJ001
    |
 
-DJ001.py:16:17: DJ001 Avoid using `null=True` on string-based fields such as CharField
+DJ001.py:16:17: DJ001 Avoid using `null=True` on string-based fields such as `CharField`
    |
 15 | class IncorrectModelWithAlias(DjangoModel):
 16 |     charfield = DjangoModel.CharField(max_length=255, null=True)
@@ -66,7 +66,7 @@ DJ001.py:16:17: DJ001 Avoid using `null=True` on string-based fields such as Cha
 18 |     slugfield = models.SlugField(max_length=255, null=True)
    |
 
-DJ001.py:17:17: DJ001 Avoid using `null=True` on string-based fields such as CharField
+DJ001.py:17:17: DJ001 Avoid using `null=True` on string-based fields such as `CharField`
    |
 15 | class IncorrectModelWithAlias(DjangoModel):
 16 |     charfield = DjangoModel.CharField(max_length=255, null=True)
@@ -76,7 +76,7 @@ DJ001.py:17:17: DJ001 Avoid using `null=True` on string-based fields such as Cha
 19 |     emailfield = models.EmailField(max_length=255, null=True)
    |
 
-DJ001.py:18:17: DJ001 Avoid using `null=True` on string-based fields such as SlugField
+DJ001.py:18:17: DJ001 Avoid using `null=True` on string-based fields such as `SlugField`
    |
 16 |     charfield = DjangoModel.CharField(max_length=255, null=True)
 17 |     textfield = SmthCharField(max_length=255, null=True)
@@ -86,7 +86,7 @@ DJ001.py:18:17: DJ001 Avoid using `null=True` on string-based fields such as Slu
 20 |     filepathfield = models.FilePathField(max_length=255, null=True)
    |
 
-DJ001.py:19:18: DJ001 Avoid using `null=True` on string-based fields such as EmailField
+DJ001.py:19:18: DJ001 Avoid using `null=True` on string-based fields such as `EmailField`
    |
 17 |     textfield = SmthCharField(max_length=255, null=True)
 18 |     slugfield = models.SlugField(max_length=255, null=True)
@@ -96,7 +96,7 @@ DJ001.py:19:18: DJ001 Avoid using `null=True` on string-based fields such as Ema
 21 |     urlfield = models.URLField(max_length=255, null=True)
    |
 
-DJ001.py:20:21: DJ001 Avoid using `null=True` on string-based fields such as FilePathField
+DJ001.py:20:21: DJ001 Avoid using `null=True` on string-based fields such as `FilePathField`
    |
 18 |     slugfield = models.SlugField(max_length=255, null=True)
 19 |     emailfield = models.EmailField(max_length=255, null=True)
@@ -105,15 +105,15 @@ DJ001.py:20:21: DJ001 Avoid using `null=True` on string-based fields such as Fil
 21 |     urlfield = models.URLField(max_length=255, null=True)
    |
 
-DJ001.py:21:16: DJ001 Avoid using `null=True` on string-based fields such as URLField
+DJ001.py:21:16: DJ001 Avoid using `null=True` on string-based fields such as `URLField`
    |
 19 |     emailfield = models.EmailField(max_length=255, null=True)
 20 |     filepathfield = models.FilePathField(max_length=255, null=True)
 21 |     urlfield = models.URLField(max_length=255, null=True)
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ DJ001
    |
 
-DJ001.py:25:17: DJ001 Avoid using `null=True` on string-based fields such as CharField
+DJ001.py:25:17: DJ001 Avoid using `null=True` on string-based fields such as `CharField`
    |
 24 | class IncorrectModelWithoutSuperclass:
 25 |     charfield = DjangoModel.CharField(max_length=255, null=True)
@@ -122,7 +122,7 @@ DJ001.py:25:17: DJ001 Avoid using `null=True` on string-based fields such as Cha
 27 |     slugfield = models.SlugField(max_length=255, null=True)
    |
 
-DJ001.py:26:17: DJ001 Avoid using `null=True` on string-based fields such as CharField
+DJ001.py:26:17: DJ001 Avoid using `null=True` on string-based fields such as `CharField`
    |
 24 | class IncorrectModelWithoutSuperclass:
 25 |     charfield = DjangoModel.CharField(max_length=255, null=True)
@@ -132,7 +132,7 @@ DJ001.py:26:17: DJ001 Avoid using `null=True` on string-based fields such as Cha
 28 |     emailfield = models.EmailField(max_length=255, null=True)
    |
 
-DJ001.py:27:17: DJ001 Avoid using `null=True` on string-based fields such as SlugField
+DJ001.py:27:17: DJ001 Avoid using `null=True` on string-based fields such as `SlugField`
    |
 25 |     charfield = DjangoModel.CharField(max_length=255, null=True)
 26 |     textfield = SmthCharField(max_length=255, null=True)
@@ -142,7 +142,7 @@ DJ001.py:27:17: DJ001 Avoid using `null=True` on string-based fields such as Slu
 29 |     filepathfield = models.FilePathField(max_length=255, null=True)
    |
 
-DJ001.py:28:18: DJ001 Avoid using `null=True` on string-based fields such as EmailField
+DJ001.py:28:18: DJ001 Avoid using `null=True` on string-based fields such as `EmailField`
    |
 26 |     textfield = SmthCharField(max_length=255, null=True)
 27 |     slugfield = models.SlugField(max_length=255, null=True)
@@ -152,7 +152,7 @@ DJ001.py:28:18: DJ001 Avoid using `null=True` on string-based fields such as Ema
 30 |     urlfield = models.URLField(max_length=255, null=True)
    |
 
-DJ001.py:29:21: DJ001 Avoid using `null=True` on string-based fields such as FilePathField
+DJ001.py:29:21: DJ001 Avoid using `null=True` on string-based fields such as `FilePathField`
    |
 27 |     slugfield = models.SlugField(max_length=255, null=True)
 28 |     emailfield = models.EmailField(max_length=255, null=True)
@@ -161,7 +161,7 @@ DJ001.py:29:21: DJ001 Avoid using `null=True` on string-based fields such as Fil
 30 |     urlfield = models.URLField(max_length=255, null=True)
    |
 
-DJ001.py:30:16: DJ001 Avoid using `null=True` on string-based fields such as URLField
+DJ001.py:30:16: DJ001 Avoid using `null=True` on string-based fields such as `URLField`
    |
 28 |     emailfield = models.EmailField(max_length=255, null=True)
 29 |     filepathfield = models.FilePathField(max_length=255, null=True)

crates/ruff_linter/src/rules/flake8_django/snapshots/ruff_linter__rules__flake8_django__tests__DJ008_DJ008.py.snap

@@ -23,4 +23,12 @@ DJ008.py:36:7: DJ008 Model does not define `__str__` method
 37 |     new_field = models.CharField(max_length=10)
    |
 
+DJ008.py:182:7: DJ008 Model does not define `__str__` method
+    |
+181 | # Subclass without __str__
+182 | class SubclassTestModel3(TestModel1):
+    |       ^^^^^^^^^^^^^^^^^^ DJ008
+183 |     pass
+    |
+
 

crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs

@@ -77,7 +77,7 @@ fn runtime_required_base_class(
     base_classes: &[String],
     semantic: &SemanticModel,
 ) -> bool {
-    analyze::class::any_over_body(class_def, semantic, &|call_path| {
+    analyze::class::any_call_path(class_def, semantic, &|call_path| {
         base_classes
             .iter()
             .any(|base_class| from_qualified_name(base_class) == call_path)

crates/ruff_linter/src/rules/ruff/rules/helpers.rs

@@ -56,7 +56,7 @@ pub(super) fn has_default_copy_semantics(
     class_def: &ast::StmtClassDef,
     semantic: &SemanticModel,
 ) -> bool {
-    analyze::class::any_over_body(class_def, semantic, &|call_path| {
+    analyze::class::any_call_path(class_def, semantic, &|call_path| {
         matches!(
             call_path.as_slice(),
             ["pydantic", "BaseModel" | "BaseSettings"]

crates/ruff_python_semantic/src/analyze/class.rs

@@ -6,8 +6,8 @@ use ruff_python_ast::helpers::map_subscript;
 
 use crate::{BindingId, SemanticModel};
 
-/// Return `true` if any base class of a class definition matches a predicate.
-pub fn any_over_body(
+/// Return `true` if any base class matches a [`CallPath`] predicate.
+pub fn any_call_path(
     class_def: &ast::StmtClassDef,
     semantic: &SemanticModel,
     func: &dyn Fn(CallPath) -> bool,
@@ -55,3 +55,45 @@ pub fn any_over_body(
 
     inner(class_def, semantic, func, &mut FxHashSet::default())
 }
+
+/// Return `true` if any base class matches an [`ast::StmtClassDef`] predicate.
+pub fn any_super_class(
+    class_def: &ast::StmtClassDef,
+    semantic: &SemanticModel,
+    func: &dyn Fn(&ast::StmtClassDef) -> bool,
+) -> bool {
+    fn inner(
+        class_def: &ast::StmtClassDef,
+        semantic: &SemanticModel,
+        func: &dyn Fn(&ast::StmtClassDef) -> bool,
+        seen: &mut FxHashSet<BindingId>,
+    ) -> bool {
+        // If the function itself matches the pattern, then this does too.
+        if func(class_def) {
+            return true;
+        }
+
+        // Otherwise, check every base class.
+        class_def.bases().iter().any(|expr| {
+            // If the base class extends a class that matches the pattern, then this does too.
+            if let Some(id) = semantic.lookup_attribute(map_subscript(expr)) {
+                if seen.insert(id) {
+                    let binding = semantic.binding(id);
+                    if let Some(base_class) = binding
+                        .kind
+                        .as_class_definition()
+                        .map(|id| &semantic.scopes[*id])
+                        .and_then(|scope| scope.kind.as_class())
+                    {
+                        if inner(base_class, semantic, func, seen) {
+                            return true;
+                        }
+                    }
+                }
+            }
+            false
+        })
+    }
+
+    inner(class_def, semantic, func, &mut FxHashSet::default())
+}