diff --git a/README.md b/README.md index d58290b993..065443f01a 100644 --- a/README.md +++ b/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/) diff --git a/crates/ruff/resources/test/fixtures/flake8_django/DJ001.py b/crates/ruff/resources/test/fixtures/flake8_django/DJ001.py new file mode 100644 index 0000000000..4c1ec71e24 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_django/DJ001.py @@ -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) diff --git a/crates/ruff/resources/test/fixtures/flake8_django/DJ008.py b/crates/ruff/resources/test/fixtures/flake8_django/DJ008.py new file mode 100644 index 0000000000..27473e9b75 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_django/DJ008.py @@ -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 diff --git a/crates/ruff/resources/test/fixtures/flake8_django/DJ013.py b/crates/ruff/resources/test/fixtures/flake8_django/DJ013.py new file mode 100644 index 0000000000..42588a4add --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_django/DJ013.py @@ -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 diff --git a/crates/ruff/src/checkers/ast.rs b/crates/ruff/src/checkers/ast.rs index 14596b849b..010f960b57 100644 --- a/crates/ruff/src/checkers/ast.rs +++ b/crates/ruff/src/checkers/ast.rs @@ -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); } diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index b9632b67c2..8412c42e9b 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -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, diff --git a/crates/ruff/src/rules/flake8_django/mod.rs b/crates/ruff/src/rules/flake8_django/mod.rs new file mode 100644 index 0000000000..203de41b22 --- /dev/null +++ b/crates/ruff/src/rules/flake8_django/mod.rs @@ -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(()) + } +} diff --git a/crates/ruff/src/rules/flake8_django/rules/helpers.rs b/crates/ruff/src/rules/flake8_django/rules/helpers.rs new file mode 100644 index 0000000000..d308d7ebf6 --- /dev/null +++ b/crates/ruff/src/rules/flake8_django/rules/helpers.rs @@ -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() + }) +} diff --git a/crates/ruff/src/rules/flake8_django/rules/mod.rs b/crates/ruff/src/rules/flake8_django/rules/mod.rs new file mode 100644 index 0000000000..5a95bcb1b7 --- /dev/null +++ b/crates/ruff/src/rules/flake8_django/rules/mod.rs @@ -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}; diff --git a/crates/ruff/src/rules/flake8_django/rules/model_dunder_str.rs b/crates/ruff/src/rules/flake8_django/rules/model_dunder_str.rs new file mode 100644 index 0000000000..ab2a045d6e --- /dev/null +++ b/crates/ruff/src/rules/flake8_django/rules/model_dunder_str.rs @@ -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 { + 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 +} diff --git a/crates/ruff/src/rules/flake8_django/rules/model_string_field_nullable.rs b/crates/ruff/src/rules/flake8_django/rules/model_string_field_nullable.rs new file mode 100644 index 0000000000..91fd882c6c --- /dev/null +++ b/crates/ruff/src/rules/flake8_django/rules/model_string_field_nullable.rs @@ -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 { + 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) +} diff --git a/crates/ruff/src/rules/flake8_django/rules/receiver_decorator_checker.rs b/crates/ruff/src/rules/flake8_django/rules/receiver_decorator_checker.rs new file mode 100644 index 0000000000..437c8a299e --- /dev/null +++ b/crates/ruff/src/rules/flake8_django/rules/receiver_decorator_checker.rs @@ -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 +where + F: Fn(&'a Expr) -> Option>, +{ + 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 +} diff --git a/crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ001_DJ001.py.snap b/crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ001_DJ001.py.snap new file mode 100644 index 0000000000..e1a05d01f3 --- /dev/null +++ b/crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ001_DJ001.py.snap @@ -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: ~ + diff --git a/crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ008_DJ008.py.snap b/crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ008_DJ008.py.snap new file mode 100644 index 0000000000..b5f93d3239 --- /dev/null +++ b/crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ008_DJ008.py.snap @@ -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: ~ + diff --git a/crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ013_DJ013.py.snap b/crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ013_DJ013.py.snap new file mode 100644 index 0000000000..d8ee59da80 --- /dev/null +++ b/crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ013_DJ013.py.snap @@ -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: ~ + diff --git a/crates/ruff/src/rules/mod.rs b/crates/ruff/src/rules/mod.rs index 4acbceaa2b..0cced3250b 100644 --- a/crates/ruff/src/rules/mod.rs +++ b/crates/ruff/src/rules/mod.rs @@ -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; diff --git a/docs/rules/model-dunder-str.md b/docs/rules/model-dunder-str.md new file mode 100644 index 0000000000..cc09bc825c --- /dev/null +++ b/docs/rules/model-dunder-str.md @@ -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}" +``` \ No newline at end of file diff --git a/docs/rules/model-string-field-nullable.md b/docs/rules/model-string-field-nullable.md new file mode 100644 index 0000000000..00cc4529b2 --- /dev/null +++ b/docs/rules/model-string-field-nullable.md @@ -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="") +``` \ No newline at end of file diff --git a/docs/rules/receiver-decorator-checker.md b/docs/rules/receiver-decorator-checker.md new file mode 100644 index 0000000000..206443a35d --- /dev/null +++ b/docs/rules/receiver-decorator-checker.md @@ -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 +``` \ No newline at end of file diff --git a/ruff.schema.json b/ruff.schema.json index eb695f0d59..c1db1b62d5 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1455,6 +1455,13 @@ "D417", "D418", "D419", + "DJ", + "DJ0", + "DJ00", + "DJ001", + "DJ008", + "DJ01", + "DJ013", "DTZ", "DTZ0", "DTZ00",