mirror of https://github.com/astral-sh/ruff
Implement flake8-django plugin rules (#2586)
This commit is contained in:
parent
fda93c6245
commit
6769a5bce7
13
README.md
13
README.md
|
|
@ -137,6 +137,7 @@ This README is also available as [documentation](https://beta.ruff.rs/docs/).
|
|||
1. [flake8-comprehensions (C4)](#flake8-comprehensions-c4)
|
||||
1. [flake8-datetimez (DTZ)](#flake8-datetimez-dtz)
|
||||
1. [flake8-debugger (T10)](#flake8-debugger-t10)
|
||||
1. [flake8-django (DJ)](#flake8-django-dj)
|
||||
1. [flake8-errmsg (EM)](#flake8-errmsg-em)
|
||||
1. [flake8-executable (EXE)](#flake8-executable-exe)
|
||||
1. [flake8-implicit-str-concat (ISC)](#flake8-implicit-str-concat-isc)
|
||||
|
|
@ -1077,6 +1078,16 @@ For more, see [flake8-debugger](https://pypi.org/project/flake8-debugger/) on Py
|
|||
| ---- | ---- | ------- | --- |
|
||||
| T100 | debugger | Trace found: `{name}` used | |
|
||||
|
||||
### flake8-django (DJ)
|
||||
|
||||
For more, see [flake8-django](https://pypi.org/project/flake8-django/) on PyPI.
|
||||
|
||||
| Code | Name | Message | Fix |
|
||||
| ---- | ---- | ------- | --- |
|
||||
| DJ001 | [model-string-field-nullable](https://github.com/charliermarsh/ruff/blob/main/docs/rules/model-string-field-nullable.md) | Avoid using `null=True` on string-based fields such as {field_name} | |
|
||||
| DJ008 | [model-dunder-str](https://github.com/charliermarsh/ruff/blob/main/docs/rules/model-dunder-str.md) | Model does not define `__str__` method | |
|
||||
| DJ013 | [receiver-decorator-checker](https://github.com/charliermarsh/ruff/blob/main/docs/rules/receiver-decorator-checker.md) | `@receiver` decorator must be on top of all the other decorators | |
|
||||
|
||||
### flake8-errmsg (EM)
|
||||
|
||||
For more, see [flake8-errmsg](https://pypi.org/project/flake8-errmsg/) on PyPI.
|
||||
|
|
@ -1722,6 +1733,7 @@ natively, including:
|
|||
* [flake8-comprehensions](https://pypi.org/project/flake8-comprehensions/)
|
||||
* [flake8-datetimez](https://pypi.org/project/flake8-datetimez/)
|
||||
* [flake8-debugger](https://pypi.org/project/flake8-debugger/)
|
||||
* [flake8-django](https://pypi.org/project/flake8-django/) ([#2817](https://github.com/charliermarsh/ruff/issues/2817))
|
||||
* [flake8-docstrings](https://pypi.org/project/flake8-docstrings/)
|
||||
* [flake8-eradicate](https://pypi.org/project/flake8-eradicate/)
|
||||
* [flake8-errmsg](https://pypi.org/project/flake8-errmsg/)
|
||||
|
|
@ -1820,6 +1832,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl
|
|||
* [flake8-comprehensions](https://pypi.org/project/flake8-comprehensions/)
|
||||
* [flake8-datetimez](https://pypi.org/project/flake8-datetimez/)
|
||||
* [flake8-debugger](https://pypi.org/project/flake8-debugger/)
|
||||
* [flake8-django](https://pypi.org/project/flake8-django/) ([#2817](https://github.com/charliermarsh/ruff/issues/2817))
|
||||
* [flake8-docstrings](https://pypi.org/project/flake8-docstrings/)
|
||||
* [flake8-eradicate](https://pypi.org/project/flake8-eradicate/)
|
||||
* [flake8-errmsg](https://pypi.org/project/flake8-errmsg/)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,39 @@
|
|||
from django.db.models import Model as DjangoModel
|
||||
from django.db import models
|
||||
from django.db.models import CharField as SmthCharField
|
||||
|
||||
|
||||
class IncorrectModel(models.Model):
|
||||
charfield = models.CharField(max_length=255, null=True)
|
||||
textfield = models.TextField(max_length=255, null=True)
|
||||
slugfield = models.SlugField(max_length=255, null=True)
|
||||
emailfield = models.EmailField(max_length=255, null=True)
|
||||
filepathfield = models.FilePathField(max_length=255, null=True)
|
||||
urlfield = models.URLField(max_length=255, null=True)
|
||||
|
||||
|
||||
class IncorrectModelWithAliasedBase(DjangoModel):
|
||||
charfield = DjangoModel.CharField(max_length=255, null=True)
|
||||
textfield = SmthCharField(max_length=255, null=True)
|
||||
slugfield = models.SlugField(max_length=255, null=True)
|
||||
emailfield = models.EmailField(max_length=255, null=True)
|
||||
filepathfield = models.FilePathField(max_length=255, null=True)
|
||||
urlfield = models.URLField(max_length=255, null=True)
|
||||
|
||||
|
||||
class CorrectModel(models.Model):
|
||||
charfield = models.CharField(max_length=255, null=False, blank=True)
|
||||
textfield = models.TextField(max_length=255, null=False, blank=True)
|
||||
slugfield = models.SlugField(max_length=255, null=False, blank=True)
|
||||
emailfield = models.EmailField(max_length=255, null=False, blank=True)
|
||||
filepathfield = models.FilePathField(max_length=255, null=False, blank=True)
|
||||
urlfield = models.URLField(max_length=255, null=False, blank=True)
|
||||
|
||||
charfieldu = models.CharField(max_length=255, null=True, blank=True, unique=True)
|
||||
textfieldu = models.TextField(max_length=255, null=True, blank=True, unique=True)
|
||||
slugfieldu = models.SlugField(max_length=255, null=True, blank=True, unique=True)
|
||||
emailfieldu = models.EmailField(max_length=255, null=True, blank=True, unique=True)
|
||||
filepathfieldu = models.FilePathField(
|
||||
max_length=255, null=True, blank=True, unique=True
|
||||
)
|
||||
urlfieldu = models.URLField(max_length=255, null=True, blank=True, unique=True)
|
||||
|
|
@ -0,0 +1,167 @@
|
|||
from django.db import models
|
||||
from django.db.models import Model
|
||||
|
||||
|
||||
# Models without __str__
|
||||
class TestModel1(models.Model):
|
||||
new_field = models.CharField(max_length=10)
|
||||
|
||||
class Meta:
|
||||
verbose_name = "test model"
|
||||
verbose_name_plural = "test models"
|
||||
|
||||
@property
|
||||
def my_brand_new_property(self):
|
||||
return 1
|
||||
|
||||
def my_beautiful_method(self):
|
||||
return 2
|
||||
|
||||
|
||||
class TestModel2(Model):
|
||||
new_field = models.CharField(max_length=10)
|
||||
|
||||
class Meta:
|
||||
verbose_name = "test model"
|
||||
verbose_name_plural = "test models"
|
||||
|
||||
@property
|
||||
def my_brand_new_property(self):
|
||||
return 1
|
||||
|
||||
def my_beautiful_method(self):
|
||||
return 2
|
||||
|
||||
|
||||
class TestModel3(Model):
|
||||
new_field = models.CharField(max_length=10)
|
||||
|
||||
class Meta:
|
||||
abstract = False
|
||||
|
||||
@property
|
||||
def my_brand_new_property(self):
|
||||
return 1
|
||||
|
||||
def my_beautiful_method(self):
|
||||
return 2
|
||||
|
||||
|
||||
# Models with __str__
|
||||
class TestModel4(Model):
|
||||
new_field = models.CharField(max_length=10)
|
||||
|
||||
class Meta:
|
||||
verbose_name = "test model"
|
||||
verbose_name_plural = "test models"
|
||||
|
||||
def __str__(self):
|
||||
return self.new_field
|
||||
|
||||
@property
|
||||
def my_brand_new_property(self):
|
||||
return 1
|
||||
|
||||
def my_beautiful_method(self):
|
||||
return 2
|
||||
|
||||
|
||||
class TestModel5(models.Model):
|
||||
new_field = models.CharField(max_length=10)
|
||||
|
||||
class Meta:
|
||||
verbose_name = "test model"
|
||||
verbose_name_plural = "test models"
|
||||
|
||||
def __str__(self):
|
||||
return self.new_field
|
||||
|
||||
@property
|
||||
def my_brand_new_property(self):
|
||||
return 1
|
||||
|
||||
def my_beautiful_method(self):
|
||||
return 2
|
||||
|
||||
|
||||
# Abstract models without str
|
||||
class AbstractTestModel1(models.Model):
|
||||
new_field = models.CharField(max_length=10)
|
||||
|
||||
class Meta:
|
||||
abstract = True
|
||||
|
||||
@property
|
||||
def my_brand_new_property(self):
|
||||
return 1
|
||||
|
||||
def my_beautiful_method(self):
|
||||
return 2
|
||||
|
||||
|
||||
class AbstractTestModel2(Model):
|
||||
new_field = models.CharField(max_length=10)
|
||||
|
||||
class Meta:
|
||||
abstract = True
|
||||
|
||||
@property
|
||||
def my_brand_new_property(self):
|
||||
return 1
|
||||
|
||||
def my_beautiful_method(self):
|
||||
return 2
|
||||
|
||||
|
||||
# Abstract models with __str__
|
||||
|
||||
|
||||
class AbstractTestModel3(Model):
|
||||
new_field = models.CharField(max_length=10)
|
||||
|
||||
class Meta:
|
||||
abstract = True
|
||||
|
||||
def __str__(self):
|
||||
return self.new_field
|
||||
|
||||
@property
|
||||
def my_brand_new_property(self):
|
||||
return 1
|
||||
|
||||
def my_beautiful_method(self):
|
||||
return 2
|
||||
|
||||
|
||||
class AbstractTestModel4(models.Model):
|
||||
new_field = models.CharField(max_length=10)
|
||||
|
||||
class Meta:
|
||||
abstract = True
|
||||
|
||||
def __str__(self):
|
||||
return self.new_field
|
||||
|
||||
@property
|
||||
def my_brand_new_property(self):
|
||||
return 1
|
||||
|
||||
def my_beautiful_method(self):
|
||||
return 2
|
||||
|
||||
|
||||
class AbstractTestModel5(models.Model):
|
||||
new_field = models.CharField(max_length=10)
|
||||
|
||||
class Meta:
|
||||
abstract = False
|
||||
|
||||
def __str__(self):
|
||||
return self.new_field
|
||||
|
||||
@property
|
||||
def my_brand_new_property(self):
|
||||
return 1
|
||||
|
||||
def my_beautiful_method(self):
|
||||
return 2
|
||||
|
|
@ -0,0 +1,17 @@
|
|||
from django.db.models.signals import pre_save
|
||||
from django.dispatch import receiver
|
||||
from myapp.models import MyModel
|
||||
|
||||
test_decorator = lambda func: lambda *args, **kwargs: func(*args, **kwargs)
|
||||
|
||||
|
||||
@receiver(pre_save, sender=MyModel)
|
||||
@test_decorator
|
||||
def correct_pre_save_handler():
|
||||
pass
|
||||
|
||||
|
||||
@test_decorator
|
||||
@receiver(pre_save, sender=MyModel)
|
||||
def incorrect_pre_save_handler():
|
||||
pass
|
||||
|
|
@ -35,9 +35,9 @@ use crate::registry::{Diagnostic, Rule};
|
|||
use crate::rules::{
|
||||
flake8_2020, flake8_annotations, flake8_bandit, flake8_blind_except, flake8_boolean_trap,
|
||||
flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_datetimez, flake8_debugger,
|
||||
flake8_errmsg, flake8_implicit_str_concat, flake8_import_conventions, flake8_logging_format,
|
||||
flake8_pie, flake8_print, flake8_pyi, flake8_pytest_style, flake8_raise, flake8_return,
|
||||
flake8_self, flake8_simplify, flake8_tidy_imports, flake8_type_checking,
|
||||
flake8_django, flake8_errmsg, flake8_implicit_str_concat, flake8_import_conventions,
|
||||
flake8_logging_format, flake8_pie, flake8_print, flake8_pyi, flake8_pytest_style, flake8_raise,
|
||||
flake8_return, flake8_self, flake8_simplify, flake8_tidy_imports, flake8_type_checking,
|
||||
flake8_unused_arguments, flake8_use_pathlib, mccabe, pandas_vet, pep8_naming, pycodestyle,
|
||||
pydocstyle, pyflakes, pygrep_hooks, pylint, pyupgrade, ruff, tryceratops,
|
||||
};
|
||||
|
|
@ -464,6 +464,15 @@ where
|
|||
body,
|
||||
..
|
||||
} => {
|
||||
if self.settings.rules.enabled(&Rule::ReceiverDecoratorChecker) {
|
||||
if let Some(diagnostic) =
|
||||
flake8_django::rules::receiver_decorator_checker(decorator_list, |expr| {
|
||||
self.resolve_call_path(expr)
|
||||
})
|
||||
{
|
||||
self.diagnostics.push(diagnostic);
|
||||
}
|
||||
}
|
||||
if self.settings.rules.enabled(&Rule::AmbiguousFunctionName) {
|
||||
if let Some(diagnostic) =
|
||||
pycodestyle::rules::ambiguous_function_name(name, || {
|
||||
|
|
@ -771,6 +780,19 @@ where
|
|||
decorator_list,
|
||||
body,
|
||||
} => {
|
||||
if self.settings.rules.enabled(&Rule::ModelStringFieldNullable) {
|
||||
self.diagnostics
|
||||
.extend(flake8_django::rules::model_string_field_nullable(
|
||||
self, bases, body,
|
||||
));
|
||||
}
|
||||
if self.settings.rules.enabled(&Rule::ModelDunderStr) {
|
||||
if let Some(diagnostic) =
|
||||
flake8_django::rules::model_dunder_str(self, bases, body, stmt)
|
||||
{
|
||||
self.diagnostics.push(diagnostic);
|
||||
}
|
||||
}
|
||||
if self.settings.rules.enabled(&Rule::UselessObjectInheritance) {
|
||||
pyupgrade::rules::useless_object_inheritance(self, stmt, name, bases, keywords);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -553,6 +553,10 @@ ruff_macros::define_rule_mapping!(
|
|||
RUF004 => rules::ruff::rules::KeywordArgumentBeforeStarArgument,
|
||||
RUF005 => rules::ruff::rules::UnpackInsteadOfConcatenatingToCollectionLiteral,
|
||||
RUF100 => rules::ruff::rules::UnusedNOQA,
|
||||
// flake8-django
|
||||
DJ001 => rules::flake8_django::rules::ModelStringFieldNullable,
|
||||
DJ008 => rules::flake8_django::rules::ModelDunderStr,
|
||||
DJ013 => rules::flake8_django::rules::ReceiverDecoratorChecker,
|
||||
);
|
||||
|
||||
#[derive(EnumIter, Debug, PartialEq, Eq, RuleNamespace)]
|
||||
|
|
@ -612,6 +616,9 @@ pub enum Linter {
|
|||
/// [flake8-debugger](https://pypi.org/project/flake8-debugger/)
|
||||
#[prefix = "T10"]
|
||||
Flake8Debugger,
|
||||
/// [flake8-django](https://pypi.org/project/flake8-django/)
|
||||
#[prefix = "DJ"]
|
||||
Flake8Django,
|
||||
/// [flake8-errmsg](https://pypi.org/project/flake8-errmsg/)
|
||||
#[prefix = "EM"]
|
||||
Flake8ErrMsg,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,27 @@
|
|||
//! Rules from [django-flake8](https://pypi.org/project/flake8-django/)
|
||||
pub(crate) mod rules;
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::path::Path;
|
||||
|
||||
use anyhow::Result;
|
||||
use test_case::test_case;
|
||||
|
||||
use crate::registry::Rule;
|
||||
use crate::test::test_path;
|
||||
use crate::{assert_yaml_snapshot, settings};
|
||||
|
||||
#[test_case(Rule::ModelStringFieldNullable, Path::new("DJ001.py"); "DJ001")]
|
||||
#[test_case(Rule::ModelDunderStr, Path::new("DJ008.py"); "DJ008")]
|
||||
#[test_case(Rule::ReceiverDecoratorChecker, Path::new("DJ013.py"); "DJ013")]
|
||||
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||
let snapshot = format!("{}_{}", rule_code.code(), path.to_string_lossy());
|
||||
let diagnostics = test_path(
|
||||
Path::new("flake8_django").join(path).as_path(),
|
||||
&settings::Settings::for_rule(rule_code),
|
||||
)?;
|
||||
assert_yaml_snapshot!(snapshot, diagnostics);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
use rustpython_parser::ast::Expr;
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
|
||||
/// Return `true` if a Python class appears to be a Django model based on a base class.
|
||||
pub fn is_model(checker: &Checker, base: &Expr) -> bool {
|
||||
checker.resolve_call_path(base).map_or(false, |call_path| {
|
||||
call_path.as_slice() == ["django", "db", "models", "Model"]
|
||||
})
|
||||
}
|
||||
|
||||
pub fn get_model_field_name<'a>(checker: &'a Checker, expr: &'a Expr) -> Option<&'a str> {
|
||||
checker.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;
|
||||
}
|
||||
call_path.last().copied()
|
||||
})
|
||||
}
|
||||
|
|
@ -0,0 +1,8 @@
|
|||
mod helpers;
|
||||
mod model_dunder_str;
|
||||
mod model_string_field_nullable;
|
||||
mod receiver_decorator_checker;
|
||||
|
||||
pub use model_dunder_str::{model_dunder_str, ModelDunderStr};
|
||||
pub use model_string_field_nullable::{model_string_field_nullable, ModelStringFieldNullable};
|
||||
pub use receiver_decorator_checker::{receiver_decorator_checker, ReceiverDecoratorChecker};
|
||||
|
|
@ -0,0 +1,123 @@
|
|||
use rustpython_parser::ast::{Constant, Expr, StmtKind};
|
||||
use rustpython_parser::ast::{ExprKind, Stmt};
|
||||
|
||||
use ruff_macros::{define_violation, derive_message_formats};
|
||||
|
||||
use crate::ast::types::Range;
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::registry::Diagnostic;
|
||||
use crate::violation::Violation;
|
||||
|
||||
use super::helpers;
|
||||
|
||||
define_violation!(
|
||||
/// ## What it does
|
||||
/// Checks that `__str__` method is defined in Django models.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// Django models should define `__str__` method to return a string representation
|
||||
/// of the model instance, as Django calls this method to display the object in
|
||||
/// the Django Admin and elsewhere.
|
||||
///
|
||||
/// Models without `__str__` method will display a non-meaningful representation
|
||||
/// of the object in the Django Admin.
|
||||
///
|
||||
/// ## Example
|
||||
/// ```python
|
||||
/// from django.db import models
|
||||
///
|
||||
/// class MyModel(models.Model):
|
||||
/// field = models.CharField(max_length=255)
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```python
|
||||
/// from django.db import models
|
||||
///
|
||||
/// class MyModel(models.Model):
|
||||
/// field = models.CharField(max_length=255)
|
||||
///
|
||||
/// def __str__(self):
|
||||
/// return f"{self.field}"
|
||||
/// ```
|
||||
pub struct ModelDunderStr;
|
||||
);
|
||||
impl Violation for ModelDunderStr {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("Model does not define `__str__` method")
|
||||
}
|
||||
}
|
||||
|
||||
/// DJ008
|
||||
pub fn model_dunder_str(
|
||||
checker: &Checker,
|
||||
bases: &[Expr],
|
||||
body: &[Stmt],
|
||||
class_location: &Stmt,
|
||||
) -> Option<Diagnostic> {
|
||||
if !checker_applies(checker, bases, body) {
|
||||
return None;
|
||||
}
|
||||
if !has_dunder_method(body) {
|
||||
return Some(Diagnostic::new(
|
||||
ModelDunderStr,
|
||||
Range::from_located(class_location),
|
||||
));
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
fn has_dunder_method(body: &[Stmt]) -> bool {
|
||||
body.iter().any(|val| match &val.node {
|
||||
StmtKind::FunctionDef { name, .. } => {
|
||||
if name == "__str__" {
|
||||
return true;
|
||||
}
|
||||
false
|
||||
}
|
||||
_ => false,
|
||||
})
|
||||
}
|
||||
|
||||
fn checker_applies(checker: &Checker, bases: &[Expr], body: &[Stmt]) -> bool {
|
||||
for base in bases.iter() {
|
||||
if is_model_abstract(body) {
|
||||
continue;
|
||||
}
|
||||
if helpers::is_model(checker, base) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
/// Check if class is abstract, in terms of Django model inheritance.
|
||||
fn is_model_abstract(body: &[Stmt]) -> bool {
|
||||
for element in body.iter() {
|
||||
let StmtKind::ClassDef {name, body, ..} = &element.node else {
|
||||
continue
|
||||
};
|
||||
if name != "Meta" {
|
||||
continue;
|
||||
}
|
||||
for element in body.iter() {
|
||||
let StmtKind::Assign {targets, value, ..} = &element.node else {
|
||||
continue;
|
||||
};
|
||||
for target in targets.iter() {
|
||||
let ExprKind::Name {id , ..} = &target.node else {
|
||||
continue;
|
||||
};
|
||||
if id != "abstract" {
|
||||
continue;
|
||||
}
|
||||
let ExprKind::Constant{value: Constant::Bool(true), ..} = &value.node else {
|
||||
continue;
|
||||
};
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
|
@ -0,0 +1,124 @@
|
|||
use super::helpers;
|
||||
use crate::ast::types::Range;
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::registry::Diagnostic;
|
||||
use crate::violation::Violation;
|
||||
use ruff_macros::{define_violation, derive_message_formats};
|
||||
use rustpython_parser::ast::Constant::Bool;
|
||||
use rustpython_parser::ast::{Expr, ExprKind, Stmt, StmtKind};
|
||||
|
||||
define_violation!(
|
||||
/// ## What it does
|
||||
/// Checks nullable string-based fields (like `CharField` and `TextField`)
|
||||
/// in Django models.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// If a string-based field is nullable, then your model will have two possible
|
||||
/// representations for "no data": `None` and the empty string. This can lead to
|
||||
/// confusion, as clients of the API have to check for both `None` and the
|
||||
/// empty string when trying to determine if the field has data.
|
||||
///
|
||||
/// The Django convention is to use the empty string in lieu of `None` for
|
||||
/// string-based fields.
|
||||
///
|
||||
/// ## Example
|
||||
/// ```python
|
||||
/// from django.db import models
|
||||
///
|
||||
/// class MyModel(models.Model):
|
||||
/// field = models.CharField(max_length=255, null=True)
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```python
|
||||
/// from django.db import models
|
||||
///
|
||||
/// class MyModel(models.Model):
|
||||
/// field = models.CharField(max_length=255, default="")
|
||||
/// ```
|
||||
pub struct ModelStringFieldNullable {
|
||||
pub field_name: String,
|
||||
}
|
||||
);
|
||||
impl Violation for ModelStringFieldNullable {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
let ModelStringFieldNullable { field_name } = self;
|
||||
format!("Avoid using `null=True` on string-based fields such as {field_name}")
|
||||
}
|
||||
}
|
||||
|
||||
const NOT_NULL_TRUE_FIELDS: [&str; 6] = [
|
||||
"CharField",
|
||||
"TextField",
|
||||
"SlugField",
|
||||
"EmailField",
|
||||
"FilePathField",
|
||||
"URLField",
|
||||
];
|
||||
|
||||
/// DJ001
|
||||
pub fn model_string_field_nullable(
|
||||
checker: &Checker,
|
||||
bases: &[Expr],
|
||||
body: &[Stmt],
|
||||
) -> Vec<Diagnostic> {
|
||||
if !bases.iter().any(|base| helpers::is_model(checker, base)) {
|
||||
return vec![];
|
||||
}
|
||||
|
||||
let mut errors = Vec::new();
|
||||
for statement in body.iter() {
|
||||
let StmtKind::Assign {value, ..} = &statement.node else {
|
||||
continue
|
||||
};
|
||||
if let Some(field_name) = check_nullable_field(checker, value) {
|
||||
errors.push(Diagnostic::new(
|
||||
ModelStringFieldNullable {
|
||||
field_name: field_name.to_string(),
|
||||
},
|
||||
Range::from_located(value),
|
||||
));
|
||||
}
|
||||
}
|
||||
errors
|
||||
}
|
||||
|
||||
fn check_nullable_field<'a>(checker: &'a Checker, value: &'a Expr) -> Option<&'a str> {
|
||||
let ExprKind::Call {func, keywords, ..} = &value.node else {
|
||||
return None;
|
||||
};
|
||||
|
||||
let Some(valid_field_name) = helpers::get_model_field_name(checker, func) else {
|
||||
return None;
|
||||
};
|
||||
|
||||
if !NOT_NULL_TRUE_FIELDS.contains(&valid_field_name) {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut null_key = false;
|
||||
let mut blank_key = false;
|
||||
let mut unique_key = false;
|
||||
for keyword in keywords.iter() {
|
||||
let ExprKind::Constant {value: Bool(true), ..} = &keyword.node.value.node else {
|
||||
continue
|
||||
};
|
||||
let Some(argument) = &keyword.node.arg else {
|
||||
continue
|
||||
};
|
||||
match argument.as_str() {
|
||||
"blank" => blank_key = true,
|
||||
"null" => null_key = true,
|
||||
"unique" => unique_key = true,
|
||||
_ => continue,
|
||||
}
|
||||
}
|
||||
if blank_key && unique_key {
|
||||
return None;
|
||||
}
|
||||
if !null_key {
|
||||
return None;
|
||||
}
|
||||
Some(valid_field_name)
|
||||
}
|
||||
|
|
@ -0,0 +1,75 @@
|
|||
use rustpython_parser::ast::{Expr, ExprKind};
|
||||
|
||||
use ruff_macros::{define_violation, derive_message_formats};
|
||||
|
||||
use crate::ast::types::{CallPath, Range};
|
||||
use crate::registry::Diagnostic;
|
||||
use crate::violation::Violation;
|
||||
|
||||
define_violation!(
|
||||
/// ## What it does
|
||||
/// Checks that Django's `@receiver` decorator is listed first, prior to
|
||||
/// any other decorators.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// Django's `@receiver` decorator is special in that it does not return
|
||||
/// a wrapped function. Rather, `@receiver` connects the decorated function
|
||||
/// to a signal. If any other decorators are listed before `@receiver`,
|
||||
/// the decorated function will not be connected to the signal.
|
||||
///
|
||||
/// ## Example
|
||||
/// ```python
|
||||
/// from django.dispatch import receiver
|
||||
/// from django.db.models.signals import post_save
|
||||
///
|
||||
/// @transaction.atomic
|
||||
/// @receiver(post_save, sender=MyModel)
|
||||
/// def my_handler(sender, instance, created, **kwargs):
|
||||
/// pass
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```python
|
||||
/// from django.dispatch import receiver
|
||||
/// from django.db.models.signals import post_save
|
||||
///
|
||||
/// @receiver(post_save, sender=MyModel)
|
||||
/// @transaction.atomic
|
||||
/// def my_handler(sender, instance, created, **kwargs):
|
||||
/// pass
|
||||
/// ```
|
||||
pub struct ReceiverDecoratorChecker;
|
||||
);
|
||||
impl Violation for ReceiverDecoratorChecker {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`@receiver` decorator must be on top of all the other decorators")
|
||||
}
|
||||
}
|
||||
|
||||
/// DJ013
|
||||
pub fn receiver_decorator_checker<'a, F>(
|
||||
decorator_list: &'a [Expr],
|
||||
resolve_call_path: F,
|
||||
) -> Option<Diagnostic>
|
||||
where
|
||||
F: Fn(&'a Expr) -> Option<CallPath<'a>>,
|
||||
{
|
||||
for (i, decorator) in decorator_list.iter().enumerate() {
|
||||
if i == 0 {
|
||||
continue;
|
||||
}
|
||||
let ExprKind::Call{ func, ..} = &decorator.node else {
|
||||
continue;
|
||||
};
|
||||
if resolve_call_path(func).map_or(false, |call_path| {
|
||||
call_path.as_slice() == ["django", "dispatch", "receiver"]
|
||||
}) {
|
||||
return Some(Diagnostic::new(
|
||||
ReceiverDecoratorChecker,
|
||||
Range::from_located(decorator),
|
||||
));
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
|
@ -0,0 +1,137 @@
|
|||
---
|
||||
source: crates/ruff/src/rules/flake8_django/mod.rs
|
||||
expression: diagnostics
|
||||
---
|
||||
- kind:
|
||||
ModelStringFieldNullable:
|
||||
field_name: CharField
|
||||
location:
|
||||
row: 7
|
||||
column: 16
|
||||
end_location:
|
||||
row: 7
|
||||
column: 59
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
ModelStringFieldNullable:
|
||||
field_name: TextField
|
||||
location:
|
||||
row: 8
|
||||
column: 16
|
||||
end_location:
|
||||
row: 8
|
||||
column: 59
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
ModelStringFieldNullable:
|
||||
field_name: SlugField
|
||||
location:
|
||||
row: 9
|
||||
column: 16
|
||||
end_location:
|
||||
row: 9
|
||||
column: 59
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
ModelStringFieldNullable:
|
||||
field_name: EmailField
|
||||
location:
|
||||
row: 10
|
||||
column: 17
|
||||
end_location:
|
||||
row: 10
|
||||
column: 61
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
ModelStringFieldNullable:
|
||||
field_name: FilePathField
|
||||
location:
|
||||
row: 11
|
||||
column: 20
|
||||
end_location:
|
||||
row: 11
|
||||
column: 67
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
ModelStringFieldNullable:
|
||||
field_name: URLField
|
||||
location:
|
||||
row: 12
|
||||
column: 15
|
||||
end_location:
|
||||
row: 12
|
||||
column: 57
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
ModelStringFieldNullable:
|
||||
field_name: CharField
|
||||
location:
|
||||
row: 16
|
||||
column: 16
|
||||
end_location:
|
||||
row: 16
|
||||
column: 64
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
ModelStringFieldNullable:
|
||||
field_name: CharField
|
||||
location:
|
||||
row: 17
|
||||
column: 16
|
||||
end_location:
|
||||
row: 17
|
||||
column: 56
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
ModelStringFieldNullable:
|
||||
field_name: SlugField
|
||||
location:
|
||||
row: 18
|
||||
column: 16
|
||||
end_location:
|
||||
row: 18
|
||||
column: 59
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
ModelStringFieldNullable:
|
||||
field_name: EmailField
|
||||
location:
|
||||
row: 19
|
||||
column: 17
|
||||
end_location:
|
||||
row: 19
|
||||
column: 61
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
ModelStringFieldNullable:
|
||||
field_name: FilePathField
|
||||
location:
|
||||
row: 20
|
||||
column: 20
|
||||
end_location:
|
||||
row: 20
|
||||
column: 67
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
ModelStringFieldNullable:
|
||||
field_name: URLField
|
||||
location:
|
||||
row: 21
|
||||
column: 15
|
||||
end_location:
|
||||
row: 21
|
||||
column: 57
|
||||
fix: ~
|
||||
parent: ~
|
||||
|
||||
|
|
@ -0,0 +1,35 @@
|
|||
---
|
||||
source: crates/ruff/src/rules/flake8_django/mod.rs
|
||||
expression: diagnostics
|
||||
---
|
||||
- kind:
|
||||
ModelDunderStr: ~
|
||||
location:
|
||||
row: 6
|
||||
column: 0
|
||||
end_location:
|
||||
row: 18
|
||||
column: 16
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
ModelDunderStr: ~
|
||||
location:
|
||||
row: 21
|
||||
column: 0
|
||||
end_location:
|
||||
row: 33
|
||||
column: 16
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
ModelDunderStr: ~
|
||||
location:
|
||||
row: 36
|
||||
column: 0
|
||||
end_location:
|
||||
row: 47
|
||||
column: 16
|
||||
fix: ~
|
||||
parent: ~
|
||||
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
---
|
||||
source: crates/ruff/src/rules/flake8_django/mod.rs
|
||||
expression: diagnostics
|
||||
---
|
||||
- kind:
|
||||
ReceiverDecoratorChecker: ~
|
||||
location:
|
||||
row: 15
|
||||
column: 1
|
||||
end_location:
|
||||
row: 15
|
||||
column: 35
|
||||
fix: ~
|
||||
parent: ~
|
||||
|
||||
|
|
@ -11,6 +11,7 @@ pub mod flake8_commas;
|
|||
pub mod flake8_comprehensions;
|
||||
pub mod flake8_datetimez;
|
||||
pub mod flake8_debugger;
|
||||
pub mod flake8_django;
|
||||
pub mod flake8_errmsg;
|
||||
pub mod flake8_executable;
|
||||
pub mod flake8_implicit_str_concat;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,33 @@
|
|||
# model-dunder-str (DJ008)
|
||||
|
||||
Derived from the **flake8-django** linter.
|
||||
|
||||
## What it does
|
||||
Checks that `__str__` method is defined in Django models.
|
||||
|
||||
## Why is this bad?
|
||||
Django models should define `__str__` method to return a string representation
|
||||
of the model instance, as Django calls this method to display the object in
|
||||
the Django Admin and elsewhere.
|
||||
|
||||
Models without `__str__` method will display a non-meaningful representation
|
||||
of the object in the Django Admin.
|
||||
|
||||
## Example
|
||||
```python
|
||||
from django.db import models
|
||||
|
||||
class MyModel(models.Model):
|
||||
field = models.CharField(max_length=255)
|
||||
```
|
||||
|
||||
Use instead:
|
||||
```python
|
||||
from django.db import models
|
||||
|
||||
class MyModel(models.Model):
|
||||
field = models.CharField(max_length=255)
|
||||
|
||||
def __str__(self):
|
||||
return f"{self.field}"
|
||||
```
|
||||
|
|
@ -0,0 +1,32 @@
|
|||
# model-string-field-nullable (DJ001)
|
||||
|
||||
Derived from the **flake8-django** linter.
|
||||
|
||||
## What it does
|
||||
Checks nullable string-based fields (like `CharField` and `TextField`)
|
||||
in Django models.
|
||||
|
||||
## Why is this bad?
|
||||
If a string-based field is nullable, then your model will have two possible
|
||||
representations for "no data": `None` and the empty string. This can lead to
|
||||
confusion, as clients of the API have to check for both `None` and the
|
||||
empty string when trying to determine if the field has data.
|
||||
|
||||
The Django convention is to use the empty string in lieu of `None` for
|
||||
string-based fields.
|
||||
|
||||
## Example
|
||||
```python
|
||||
from django.db import models
|
||||
|
||||
class MyModel(models.Model):
|
||||
field = models.CharField(max_length=255, null=True)
|
||||
```
|
||||
|
||||
Use instead:
|
||||
```python
|
||||
from django.db import models
|
||||
|
||||
class MyModel(models.Model):
|
||||
field = models.CharField(max_length=255, default="")
|
||||
```
|
||||
|
|
@ -0,0 +1,35 @@
|
|||
# receiver-decorator-checker (DJ013)
|
||||
|
||||
Derived from the **flake8-django** linter.
|
||||
|
||||
## What it does
|
||||
Checks that Django's `@receiver` decorator is listed first, prior to
|
||||
any other decorators.
|
||||
|
||||
## Why is this bad?
|
||||
Django's `@receiver` decorator is special in that it does not return
|
||||
a wrapped function. Rather, `@receiver` connects the decorated function
|
||||
to a signal. If any other decorators are listed before `@receiver`,
|
||||
the decorated function will not be connected to the signal.
|
||||
|
||||
## Example
|
||||
```python
|
||||
from django.dispatch import receiver
|
||||
from django.db.models.signals import post_save
|
||||
|
||||
@transaction.atomic
|
||||
@receiver(post_save, sender=MyModel)
|
||||
def my_handler(sender, instance, created, **kwargs):
|
||||
pass
|
||||
```
|
||||
|
||||
Use instead:
|
||||
```python
|
||||
from django.dispatch import receiver
|
||||
from django.db.models.signals import post_save
|
||||
|
||||
@receiver(post_save, sender=MyModel)
|
||||
@transaction.atomic
|
||||
def my_handler(sender, instance, created, **kwargs):
|
||||
pass
|
||||
```
|
||||
|
|
@ -1455,6 +1455,13 @@
|
|||
"D417",
|
||||
"D418",
|
||||
"D419",
|
||||
"DJ",
|
||||
"DJ0",
|
||||
"DJ00",
|
||||
"DJ001",
|
||||
"DJ008",
|
||||
"DJ01",
|
||||
"DJ013",
|
||||
"DTZ",
|
||||
"DTZ0",
|
||||
"DTZ00",
|
||||
|
|
|
|||
Loading…
Reference in New Issue