Cyclic0007/ruff
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[flake8-django]: Implement rule DJ012 (#3659)

Browse Source
This commit is contained in:
Dhruv Manilawala
2023-03-22 08:37:58 +05:30
committed by GitHub
parent 5eae3fbbfb
commit 9e61956711
14 changed files with 377 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

113
crates/ruff/resources/test/fixtures/flake8_django/DJ012.py vendored Normal file
View File

@@ -0,0 +1,113 @@
from django.db import models
from django.db.models import Model
class StrBeforeRandomField(models.Model):
"""Model with `__str__` before a random property."""
class Meta:
verbose_name = "test"
verbose_name_plural = "tests"
def __str__(self):
return ""
random_property = "foo"
class StrBeforeFieldModel(models.Model):
"""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)
class ManagerBeforeField(models.Model):
"""Model with manager before fields."""
objects = "manager"
class Meta:
verbose_name = "test"
verbose_name_plural = "tests"
def __str__(self):
return "foobar"
first_name = models.CharField(max_length=32)
class CustomMethodBeforeStr(models.Model):
"""Model with a custom method before `__str__`."""
class Meta:
verbose_name = "test"
verbose_name_plural = "tests"
def my_method(self):
pass
def __str__(self):
return "foobar"
class GetAbsoluteUrlBeforeSave(Model):
"""Model with `get_absolute_url` method before `save` method.
Subclass this directly using the `Model` class.
"""
def get_absolute_url(self):
pass
def save(self):
pass
class ConstantsAreNotFields(models.Model):
"""Model with an assignment to a constant after `__str__`."""
first_name = models.CharField(max_length=32)
class Meta:
verbose_name = "test"
verbose_name_plural = "tests"
def __str__(self):
pass
MY_CONSTANT = id(1)
class PerfectlyFine(models.Model):
"""Model which has everything in perfect order."""
first_name = models.CharField(max_length=32)
last_name = models.CharField(max_length=32)
objects = "manager"
class Meta:
verbose_name = "test"
verbose_name_plural = "tests"
def __str__(self):
return "Perfectly fine!"
def save(self, **kwargs):
super(PerfectlyFine, self).save(**kwargs)
def get_absolute_url(self):
return "http://%s" % self
def my_method(self):
pass
@property
def random_property(self):
return "%s" % self

7
crates/ruff/src/checkers/ast/mod.rs
View File

@@ -708,6 +708,13 @@ where
self.diagnostics.push(diagnostic);
}
}
if self
.settings
.rules
.enabled(Rule::DjangoUnorderedBodyContentInModel)
{
flake8_django::rules::unordered_body_content_in_model(self, bases, body);
}
if self.settings.rules.enabled(Rule::GlobalStatement) {
pylint::rules::global_statement(self, name);
}

1
crates/ruff/src/codes.rs
View File

@@ -699,6 +699,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Flake8Django, "006") => Rule::DjangoExcludeWithModelForm,
(Flake8Django, "007") => Rule::DjangoAllWithModelForm,
(Flake8Django, "008") => Rule::DjangoModelWithoutDunderStr,
(Flake8Django, "012") => Rule::DjangoUnorderedBodyContentInModel,
(Flake8Django, "013") => Rule::DjangoNonLeadingReceiverDecorator,
_ => return None,

1
crates/ruff/src/registry.rs
View File

@@ -642,6 +642,7 @@ ruff_macros::register_rules!(
rules::flake8_django::rules::DjangoExcludeWithModelForm,
rules::flake8_django::rules::DjangoAllWithModelForm,
rules::flake8_django::rules::DjangoModelWithoutDunderStr,
rules::flake8_django::rules::DjangoUnorderedBodyContentInModel,
rules::flake8_django::rules::DjangoNonLeadingReceiverDecorator,
);

1
crates/ruff/src/rules/flake8_django/mod.rs
View File

@@ -18,6 +18,7 @@ mod tests {
#[test_case(Rule::DjangoExcludeWithModelForm, Path::new("DJ006.py"); "DJ006")]
#[test_case(Rule::DjangoAllWithModelForm, Path::new("DJ007.py"); "DJ007")]
#[test_case(Rule::DjangoModelWithoutDunderStr, Path::new("DJ008.py"); "DJ008")]
#[test_case(Rule::DjangoUnorderedBodyContentInModel, Path::new("DJ012.py"); "DJ012")]
#[test_case(Rule::DjangoNonLeadingReceiverDecorator, Path::new("DJ013.py"); "DJ013")]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());

2
crates/ruff/src/rules/flake8_django/rules/all_with_model_form.rs
View File

@@ -49,7 +49,7 @@ impl Violation for DjangoAllWithModelForm {
/// DJ007
pub fn all_with_model_form(checker: &Checker, bases: &[Expr], body: &[Stmt]) -> Option<Diagnostic> {
if !bases.iter().any(|base| is_model_form(checker, base)) {
if !bases.iter().any(|base| is_model_form(&checker.ctx, base)) {
return None;
}
for element in body.iter() {

2
crates/ruff/src/rules/flake8_django/rules/exclude_with_model_form.rs
View File

@@ -49,7 +49,7 @@ pub fn exclude_with_model_form(
bases: &[Expr],
body: &[Stmt],
) -> Option<Diagnostic> {
if !bases.iter().any(|base| is_model_form(checker, base)) {
if !bases.iter().any(|base| is_model_form(&checker.ctx, base)) {
return None;
}
for element in body.iter() {

40
crates/ruff/src/rules/flake8_django/rules/helpers.rs
View File

@@ -1,31 +1,33 @@
use ruff_python_ast::context::Context;
use rustpython_parser::ast::Expr;
use crate::checkers::ast::Checker;
/// Return `true` if a Python class appears to be a Django model, based on its base classes.
pub fn is_model(checker: &Checker, base: &Expr) -> bool {
checker
.ctx
.resolve_call_path(base)
.map_or(false, |call_path| {
call_path.as_slice() == ["django", "db", "models", "Model"]
})
pub fn is_model(context: &Context, base: &Expr) -> bool {
context.resolve_call_path(base).map_or(false, |call_path| {
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 fn is_model_form(checker: &Checker, base: &Expr) -> bool {
checker
.ctx
.resolve_call_path(base)
.map_or(false, |call_path| {
call_path.as_slice() == ["django", "forms", "ModelForm"]
|| call_path.as_slice() == ["django", "forms", "models", "ModelForm"]
})
pub fn is_model_form(context: &Context, base: &Expr) -> bool {
context.resolve_call_path(base).map_or(false, |call_path| {
call_path.as_slice() == ["django", "forms", "ModelForm"]
|| call_path.as_slice() == ["django", "forms", "models", "ModelForm"]
})
}
/// Return `true` if the expression is constructor for a Django model field.
pub fn is_model_field(context: &Context, expr: &Expr) -> bool {
context.resolve_call_path(expr).map_or(false, |call_path| {
call_path
.as_slice()
.starts_with(&["django", "db", "models"])
})
}
/// Return the name of the field type, if the expression is constructor for a Django model field.
pub fn get_model_field_name<'a>(checker: &'a Checker, expr: &'a Expr) -> Option<&'a str> {
checker.ctx.resolve_call_path(expr).and_then(|call_path| {
pub fn get_model_field_name<'a>(context: &'a Context, expr: &'a Expr) -> Option<&'a str> {
context.resolve_call_path(expr).and_then(|call_path| {
let call_path = call_path.as_slice();
if !call_path.starts_with(&["django", "db", "models"]) {
return None;

4
crates/ruff/src/rules/flake8_django/rules/mod.rs
View File

@@ -8,6 +8,9 @@ pub use non_leading_receiver_decorator::{
pub use nullable_model_string_field::{
nullable_model_string_field, DjangoNullableModelStringField,
};
pub use unordered_body_content_in_model::{
unordered_body_content_in_model, DjangoUnorderedBodyContentInModel,
};
mod all_with_model_form;
mod exclude_with_model_form;
@@ -16,3 +19,4 @@ mod locals_in_render_function;
mod model_without_dunder_str;
mod non_leading_receiver_decorator;
mod nullable_model_string_field;
mod unordered_body_content_in_model;

2
crates/ruff/src/rules/flake8_django/rules/model_without_dunder_str.rs
View File

@@ -86,7 +86,7 @@ fn checker_applies(checker: &Checker, bases: &[Expr], body: &[Stmt]) -> bool {
if is_model_abstract(body) {
continue;
}
if helpers::is_model(checker, base) {
if helpers::is_model(&checker.ctx, base) {
return true;
}
}

2
crates/ruff/src/rules/flake8_django/rules/nullable_model_string_field.rs
View File

@@ -85,7 +85,7 @@ fn is_nullable_field<'a>(checker: &'a Checker, value: &'a Expr) -> Option<&'a st
return None;
};
let Some(valid_field_name) = helpers::get_model_field_name(checker, func) else {
let Some(valid_field_name) = helpers::get_model_field_name(&checker.ctx, func) else {
return None;
};

167
crates/ruff/src/rules/flake8_django/rules/unordered_body_content_in_model.rs Normal file
View File

@@ -0,0 +1,167 @@
use std::fmt;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::types::Range;
use rustpython_parser::ast::{Expr, ExprKind, Stmt, StmtKind};
use crate::checkers::ast::Checker;
use super::helpers;
/// ## What it does
/// Checks for the order of Model's inner classes, methods, and fields as per
/// the [Django Style Guide].
///
/// ## Why is it bad?
/// The [Django Style Guide] specifies that the order of Model inner classes,
/// attributes and methods should be as follows:
///
/// 1. All database fields
/// 2. Custom manager attributes
/// 3. `class Meta`
/// 4. `def __str__()`
/// 5. `def save()`
/// 6. `def get_absolute_url()`
/// 7. Any custom methods
///
/// ## Examples
/// ```python
/// from django.db import models
///
///
/// class StrBeforeFieldModel(models.Model):
/// class Meta:
/// verbose_name = "test"
/// verbose_name_plural = "tests"
///
/// def __str__(self):
/// return "foobar"
///
/// first_name = models.CharField(max_length=32)
/// last_name = models.CharField(max_length=40)
/// ```
///
/// Use instead:
/// ```python
/// from django.db import models
///
///
/// class StrBeforeFieldModel(models.Model):
/// first_name = models.CharField(max_length=32)
/// last_name = models.CharField(max_length=40)
///
/// class Meta:
/// verbose_name = "test"
/// verbose_name_plural = "tests"
///
/// def __str__(self):
/// return "foobar"
/// ```
///
/// [Django Style Guide]: https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/#model-style
#[violation]
pub struct DjangoUnorderedBodyContentInModel {
pub elem_type: ContentType,
pub before: ContentType,
}
impl Violation for DjangoUnorderedBodyContentInModel {
#[derive_message_formats]
fn message(&self) -> String {
let DjangoUnorderedBodyContentInModel { elem_type, before } = self;
format!("Order of model's inner classes, methods, and fields does not follow the Django Style Guide: {elem_type} should come before {before}")
}
}
#[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq)]
pub enum ContentType {
FieldDeclaration,
ManagerDeclaration,
MetaClass,
StrMethod,
SaveMethod,
GetAbsoluteUrlMethod,
CustomMethod,
}
impl fmt::Display for ContentType {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ContentType::FieldDeclaration => f.write_str("field declaration"),
ContentType::ManagerDeclaration => f.write_str("manager declaration"),
ContentType::MetaClass => f.write_str("`Meta` class"),
ContentType::StrMethod => f.write_str("`__str__` method"),
ContentType::SaveMethod => f.write_str("`save` method"),
ContentType::GetAbsoluteUrlMethod => f.write_str("`get_absolute_url` method"),
ContentType::CustomMethod => f.write_str("custom method"),
}
}
}
fn get_element_type(checker: &Checker, element: &StmtKind) -> Option<ContentType> {
match element {
StmtKind::Assign { targets, value, .. } => {
if let ExprKind::Call { func, .. } = &value.node {
if helpers::is_model_field(&checker.ctx, func) {
return Some(ContentType::FieldDeclaration);
}
}
let Some(expr) = targets.first() else {
return None;
};
let ExprKind::Name { id, .. } = &expr.node else {
return None;
};
if id == "objects" {
Some(ContentType::ManagerDeclaration)
} else {
None
}
}
StmtKind::ClassDef { name, .. } => {
if name == "Meta" {
Some(ContentType::MetaClass)
} else {
None
}
}
StmtKind::FunctionDef { name, .. } => match name.as_str() {
"__str__" => Some(ContentType::StrMethod),
"save" => Some(ContentType::SaveMethod),
"get_absolute_url" => Some(ContentType::GetAbsoluteUrlMethod),
_ => Some(ContentType::CustomMethod),
},
_ => None,
}
}
/// DJ012
pub fn unordered_body_content_in_model(checker: &mut Checker, bases: &[Expr], body: &[Stmt]) {
if !bases
.iter()
.any(|base| helpers::is_model(&checker.ctx, base))
{
return;
}
let mut elements_type_found = Vec::new();
for element in body.iter() {
let Some(current_element_type) = get_element_type(checker, &element.node) else {
continue;
};
let Some(&element_type) = elements_type_found
.iter()
.find(|&&element_type| element_type > current_element_type) else {
elements_type_found.push(current_element_type);
continue;
};
let diagnostic = Diagnostic::new(
DjangoUnorderedBodyContentInModel {
elem_type: current_element_type,
before: element_type,
},
Range::from(element),
);
checker.diagnostics.push(diagnostic);
}
}

57
crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ012_DJ012.py.snap Normal file
View File

@@ -0,0 +1,57 @@
---
source: crates/ruff/src/rules/flake8_django/mod.rs
expression: diagnostics
---
- kind:
name: DjangoUnorderedBodyContentInModel
body: "Order of model's inner classes, methods, and fields does not follow the Django Style Guide: field declaration should come before `Meta` class"
suggestion: ~
fixable: false
location:
row: 28
column: 4
end_location:
row: 28
column: 48
fix: ~
parent: ~
- kind:
name: DjangoUnorderedBodyContentInModel
body: "Order of model's inner classes, methods, and fields does not follow the Django Style Guide: field declaration should come before manager declaration"
suggestion: ~
fixable: false
location:
row: 43
column: 4
end_location:
row: 43
column: 48
fix: ~
parent: ~
- kind:
name: DjangoUnorderedBodyContentInModel
body: "Order of model's inner classes, methods, and fields does not follow the Django Style Guide: `__str__` method should come before custom method"
suggestion: ~
fixable: false
location:
row: 56
column: 4
end_location:
row: 57
column: 23
fix: ~
parent: ~
- kind:
name: DjangoUnorderedBodyContentInModel
body: "Order of model's inner classes, methods, and fields does not follow the Django Style Guide: `save` method should come before `get_absolute_url` method"
suggestion: ~
fixable: false
location:
row: 69
column: 4
end_location:
row: 70
column: 12
fix: ~
parent: ~

1
ruff.schema.json generated
View File

@@ -1575,6 +1575,7 @@
"DJ007",
"DJ008",
"DJ01",
"DJ012",
"DJ013",
"DTZ",
"DTZ0",