Skip to content

Commit

Permalink
[flake8-django]: Implement rule DJ012 (#3659)
Browse files Browse the repository at this point in the history
  • Loading branch information
dhruvmanila committed Mar 22, 2023
1 parent 5eae3fb commit 9e61956
Show file tree
Hide file tree
Showing 14 changed files with 377 additions and 23 deletions.
113 changes: 113 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_django/DJ012.py
Original file line number Diff line number Diff line change
@@ -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 changes: 7 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);

Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/flake8_django/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
40 changes: 21 additions & 19 deletions crates/ruff/src/rules/flake8_django/rules/helpers.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff/src/rules/flake8_django/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down

0 comments on commit 9e61956

Please sign in to comment.