mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-26 20:10:09 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Add base-class inheritance detection to flake8-django rules (#9151)

Browse source 
## Summary

As elsewhere, this only applies to classes defined within the same file.

Closes https://github.com/astral-sh/ruff/issues/9150.
This commit is contained in:
Charlie Marsh 2023-12-15 13:01:32 -05:00 • committed by GitHub
parent 82731b8194
commit 6ecf844214
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 189 additions and 199 deletions
Download patch file Download diff file Expand all files Collapse all files

18
crates/ruff_linter/resources/test/fixtures/flake8_django/DJ012.py vendored
View file

@ -127,3 +127,21 @@ class MultipleConsecutiveFields(models.Model):
pass
middle_name = models.CharField(max_length=32)
class BaseModel(models.Model):
pass
class StrBeforeFieldInheritedModel(BaseModel):
"""Model with `__str__` before fields."""
class Meta:
verbose_name = "test"
verbose_name_plural = "tests"
def __str__(self):
return "foobar"
first_name = models.CharField(max_length=32)

20
crates/ruff_linter/src/checkers/ast/analyze/statement.rs
View file

@ -397,27 +397,13 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
flake8_django::rules::nullable_model_string_field(checker, body);
}
if checker.enabled(Rule::DjangoExcludeWithModelForm) {
if let Some(diagnostic) = flake8_django::rules::exclude_with_model_form(
checker,
arguments.as_deref(),
body,
) {
checker.diagnostics.push(diagnostic);
}
flake8_django::rules::exclude_with_model_form(checker, class_def);
}
if checker.enabled(Rule::DjangoAllWithModelForm) {
if let Some(diagnostic) =
flake8_django::rules::all_with_model_form(checker, arguments.as_deref(), body)
{
checker.diagnostics.push(diagnostic);
}
flake8_django::rules::all_with_model_form(checker, class_def);
}
if checker.enabled(Rule::DjangoUnorderedBodyContentInModel) {
flake8_django::rules::unordered_body_content_in_model(
checker,
arguments.as_deref(),
body,
);
flake8_django::rules::unordered_body_content_in_model(checker, class_def);
}
if !checker.source_type.is_stub() {
if checker.enabled(Rule::DjangoModelWithoutDunderStr) {

31
crates/ruff_linter/src/rules/flake8_django/rules/all_with_model_form.rs
View file

@ -1,7 +1,6 @@
use ruff_python_ast::{self as ast, Arguments, Expr, Stmt};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, Stmt};
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
@ -48,21 +47,12 @@ impl Violation for DjangoAllWithModelForm {
}
/// DJ007
pub(crate) fn all_with_model_form(
checker: &Checker,
arguments: Option<&Arguments>,
body: &[Stmt],
) -> Option<Diagnostic> {
if !arguments.is_some_and(|arguments| {
arguments
.args
.iter()
.any(|base| is_model_form(base, checker.semantic()))
}) {
return None;
pub(crate) fn all_with_model_form(checker: &mut Checker, class_def: &ast::StmtClassDef) {
if !is_model_form(class_def, checker.semantic()) {
return;
}
for element in body {
for element in &class_def.body {
let Stmt::ClassDef(ast::StmtClassDef { name, body, .. }) = element else {
continue;
};
@ -83,12 +73,18 @@ pub(crate) fn all_with_model_form(
match value.as_ref() {
Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => {
if value == "__all__" {
return Some(Diagnostic::new(DjangoAllWithModelForm, element.range()));
checker
.diagnostics
.push(Diagnostic::new(DjangoAllWithModelForm, element.range()));
return;
}
}
Expr::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => {
if value == "__all__".as_bytes() {
return Some(Diagnostic::new(DjangoAllWithModelForm, element.range()));
checker
.diagnostics
.push(Diagnostic::new(DjangoAllWithModelForm, element.range()));
return;
}
}
_ => (),
@ -96,5 +92,4 @@ pub(crate) fn all_with_model_form(
}
}
}
None
}

26
crates/ruff_linter/src/rules/flake8_django/rules/exclude_with_model_form.rs
View file

@ -1,7 +1,6 @@
use ruff_python_ast::{self as ast, Arguments, Expr, Stmt};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, Stmt};
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
@ -46,21 +45,12 @@ impl Violation for DjangoExcludeWithModelForm {
}
/// DJ006
pub(crate) fn exclude_with_model_form(
checker: &Checker,
arguments: Option<&Arguments>,
body: &[Stmt],
) -> Option<Diagnostic> {
if !arguments.is_some_and(|arguments| {
arguments
.args
.iter()
.any(|base| is_model_form(base, checker.semantic()))
}) {
return None;
pub(crate) fn exclude_with_model_form(checker: &mut Checker, class_def: &ast::StmtClassDef) {
if !is_model_form(class_def, checker.semantic()) {
return;
}
for element in body {
for element in &class_def.body {
let Stmt::ClassDef(ast::StmtClassDef { name, body, .. }) = element else {
continue;
};
@ -76,10 +66,12 @@ pub(crate) fn exclude_with_model_form(
continue;
};
if id == "exclude" {
return Some(Diagnostic::new(DjangoExcludeWithModelForm, target.range()));
checker
.diagnostics
.push(Diagnostic::new(DjangoExcludeWithModelForm, target.range()));
return;
}
}
}
}
None
}

12
crates/ruff_linter/src/rules/flake8_django/rules/helpers.rs
View file

@ -1,17 +1,17 @@
use ruff_python_ast::Expr;
use ruff_python_ast::{self as ast, Expr};
use ruff_python_semantic::SemanticModel;
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(base: &Expr, semantic: &SemanticModel) -> bool {
semantic.resolve_call_path(base).is_some_and(|call_path| {
pub(super) fn is_model(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool {
analyze::class::any_over_body(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(base: &Expr, semantic: &SemanticModel) -> bool {
semantic.resolve_call_path(base).is_some_and(|call_path| {
pub(super) fn is_model_form(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool {
analyze::class::any_over_body(class_def, semantic, &|call_path| {
matches!(
call_path.as_slice(),
["django", "forms", "ModelForm"] | ["django", "forms", "models", "ModelForm"]

61
crates/ruff_linter/src/rules/flake8_django/rules/model_without_dunder_str.rs
View file

@ -1,10 +1,9 @@
use ruff_python_ast::{self as ast, Arguments, 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_ast::identifier::Identifier;
use ruff_python_ast::{self as ast, Expr, Stmt};
use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
@ -52,57 +51,39 @@ impl Violation for DjangoModelWithoutDunderStr {
}
/// DJ008
pub(crate) fn model_without_dunder_str(
checker: &mut Checker,
ast::StmtClassDef {
name,
arguments,
body,
..
}: &ast::StmtClassDef,
) {
if !is_non_abstract_model(arguments.as_deref(), body, checker.semantic()) {
pub(crate) fn model_without_dunder_str(checker: &mut Checker, class_def: &ast::StmtClassDef) {
if !is_non_abstract_model(class_def, checker.semantic()) {
return;
}
if has_dunder_method(body) {
if has_dunder_method(class_def) {
return;
}
checker
.diagnostics
.push(Diagnostic::new(DjangoModelWithoutDunderStr, name.range()));
checker.diagnostics.push(Diagnostic::new(
DjangoModelWithoutDunderStr,
class_def.identifier(),
));
}
fn has_dunder_method(body: &[Stmt]) -> bool {
body.iter().any(|val| match val {
Stmt::FunctionDef(ast::StmtFunctionDef { name, .. }) => {
if name == "__str__" {
return true;
}
false
}
/// 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 is_non_abstract_model(
arguments: Option<&Arguments>,
body: &[Stmt],
semantic: &SemanticModel,
) -> bool {
let Some(Arguments { args: bases, .. }) = arguments else {
return false;
};
if is_model_abstract(body) {
return false;
/// Returns `true` if the class is a non-abstract Django model.
fn is_non_abstract_model(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool {
if class_def.bases().is_empty() || is_model_abstract(class_def) {
false
} else {
helpers::is_model(class_def, semantic)
}
bases.iter().any(|base| helpers::is_model(base, semantic))
}
/// Check if class is abstract, in terms of Django model inheritance.
fn is_model_abstract(body: &[Stmt]) -> bool {
for element in body {
fn is_model_abstract(class_def: &ast::StmtClassDef) -> bool {
for element in &class_def.body {
let Stmt::ClassDef(ast::StmtClassDef { name, body, .. }) = element else {
continue;
};

97
crates/ruff_linter/src/rules/flake8_django/rules/unordered_body_content_in_model.rs
View file

@ -1,9 +1,8 @@
use std::fmt;
use ruff_python_ast::{self as ast, Arguments, Expr, Stmt};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, Stmt};
use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged;
@ -79,6 +78,50 @@ impl Violation for DjangoUnorderedBodyContentInModel {
}
}
/// DJ012
pub(crate) fn unordered_body_content_in_model(
checker: &mut Checker,
class_def: &ast::StmtClassDef,
) {
if !helpers::is_model(class_def, checker.semantic()) {
return;
}
// Track all the element types we've seen so far.
let mut element_types = Vec::new();
let mut prev_element_type = None;
for element in &class_def.body {
let Some(element_type) = get_element_type(element, checker.semantic()) else {
continue;
};
// Skip consecutive elements of the same type. It's less noisy to only report
// violations at type boundaries (e.g., avoid raising a violation for _every_
// field declaration that's out of order).
if prev_element_type == Some(element_type) {
continue;
}
prev_element_type = Some(element_type);
if let Some(&prev_element_type) = element_types
.iter()
.find(|&&prev_element_type| prev_element_type > element_type)
{
let diagnostic = Diagnostic::new(
DjangoUnorderedBodyContentInModel {
element_type,
prev_element_type,
},
element.range(),
);
checker.diagnostics.push(diagnostic);
} else {
element_types.push(element_type);
}
}
}
#[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq)]
enum ContentType {
FieldDeclaration,
@ -140,53 +183,3 @@ fn get_element_type(element: &Stmt, semantic: &SemanticModel) -> Option<ContentT
_ => None,
}
}
/// DJ012
pub(crate) fn unordered_body_content_in_model(
checker: &mut Checker,
arguments: Option<&Arguments>,
body: &[Stmt],
) {
if !arguments.is_some_and(|arguments| {
arguments
.args
.iter()
.any(|base| helpers::is_model(base, checker.semantic()))
}) {
return;
}
// Track all the element types we've seen so far.
let mut element_types = Vec::new();
let mut prev_element_type = None;
for element in body {
let Some(element_type) = get_element_type(element, checker.semantic()) else {
continue;
};
// Skip consecutive elements of the same type. It's less noisy to only report
// violations at type boundaries (e.g., avoid raising a violation for _every_
// field declaration that's out of order).
if prev_element_type == Some(element_type) {
continue;
}
prev_element_type = Some(element_type);
if let Some(&prev_element_type) = element_types
.iter()
.find(|&&prev_element_type| prev_element_type > element_type)
{
let diagnostic = Diagnostic::new(
DjangoUnorderedBodyContentInModel {
element_type,
prev_element_type,
},
element.range(),
);
checker.diagnostics.push(diagnostic);
} else {
element_types.push(element_type);
}
}
}

8
crates/ruff_linter/src/rules/flake8_django/snapshots/ruff_linter__rules__flake8_django__tests__DJ012_DJ012.py.snap
View file

@ -54,4 +54,12 @@ DJ012.py:129:5: DJ012 Order of model's inner classes, methods, and fields does n
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ DJ012
|
DJ012.py:146:5: DJ012 Order of model's inner classes, methods, and fields does not follow the Django Style Guide: field declaration should come before `Meta` class
|
144 | return "foobar"
145 |
146 | first_name = models.CharField(max_length=32)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ DJ012
|

49
crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs
View file

@ -1,13 +1,12 @@
use anyhow::Result;
use rustc_hash::FxHashSet;
use ruff_diagnostics::Edit;
use ruff_python_ast::call_path::from_qualified_name;
use ruff_python_ast::helpers::{map_callable, map_subscript};
use ruff_python_ast::helpers::map_callable;
use ruff_python_ast::{self as ast, Expr};
use ruff_python_codegen::{Generator, Stylist};
use ruff_python_semantic::{
Binding, BindingId, BindingKind, NodeId, ResolvedReference, SemanticModel,
analyze, Binding, BindingKind, NodeId, ResolvedReference, SemanticModel,
};
use ruff_source_file::Locator;
use ruff_text_size::Ranged;
@ -59,57 +58,17 @@ pub(crate) fn runtime_required_class(
false
}
/// Return `true` if a class is a subclass of a runtime-required base class.
fn runtime_required_base_class(
class_def: &ast::StmtClassDef,
base_classes: &[String],
semantic: &SemanticModel,
) -> bool {
fn inner(
class_def: &ast::StmtClassDef,
base_classes: &[String],
semantic: &SemanticModel,
seen: &mut FxHashSet<BindingId>,
) -> bool {
class_def.bases().iter().any(|expr| {
// If the base class is itself runtime-required, then this is too.
// Ex) `class Foo(BaseModel): ...`
if semantic
.resolve_call_path(map_subscript(expr))
.is_some_and(|call_path| {
analyze::class::any_over_body(class_def, semantic, &|call_path| {
base_classes
.iter()
.any(|base_class| from_qualified_name(base_class) == call_path)
})
{
return true;
}
// If the base class extends a runtime-required class, then this does too.
// Ex) `class Bar(BaseModel): ...; class Foo(Bar): ...`
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, base_classes, semantic, seen) {
return true;
}
}
}
}
false
})
}
if base_classes.is_empty() {
return false;
}
inner(class_def, base_classes, semantic, &mut FxHashSet::default())
}
fn runtime_required_decorators(

57
crates/ruff_python_semantic/src/analyze/class.rs Normal file
View file

@ -0,0 +1,57 @@
use rustc_hash::FxHashSet;
use ruff_python_ast as ast;
use ruff_python_ast::call_path::CallPath;
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(
class_def: &ast::StmtClassDef,
semantic: &SemanticModel,
func: &dyn Fn(CallPath) -> bool,
) -> bool {
fn inner(
class_def: &ast::StmtClassDef,
semantic: &SemanticModel,
func: &dyn Fn(CallPath) -> bool,
seen: &mut FxHashSet<BindingId>,
) -> bool {
class_def.bases().iter().any(|expr| {
// If the base class itself matches the pattern, then this does too.
// Ex) `class Foo(BaseModel): ...`
if semantic
.resolve_call_path(map_subscript(expr))
.is_some_and(func)
{
return true;
}
// If the base class extends a class that matches the pattern, then this does too.
// Ex) `class Bar(BaseModel): ...; class Foo(Bar): ...`
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
})
}
if class_def.bases().is_empty() {
return false;
}
inner(class_def, semantic, func, &mut FxHashSet::default())
}

1
crates/ruff_python_semantic/src/analyze/mod.rs
View file

@ -1,3 +1,4 @@
pub mod class;
pub mod function_type;
pub mod imports;
pub mod logging;