[flake8-django] DJ003, DJ006, DJ007 (#3236)

Implements [flake8-django](https://github.com/rocioar/flake8-django) rules:
- DJ03
- DJ06
- DJ07
This commit is contained in:
Luc Khai Hai 2023-02-27 07:29:42 +09:00 committed by GitHub
parent 3a78b59314
commit bc79f540e4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 426 additions and 7 deletions

View File

@ -0,0 +1,21 @@
from django.shortcuts import render
def test_view1(request):
return render(request, "index.html", locals())
def test_view2(request):
return render(request, "index.html", context=locals())
def test_view3(request):
return render(request, "index.html")
def test_view4(request):
return render(request, "index.html", {})
def test_view5(request):
return render(request, "index.html", context={})

View File

@ -0,0 +1,11 @@
from django import forms
class TestModelForm1(forms.ModelForm):
class Meta:
exclude = ["bar"]
class TestModelForm2(forms.ModelForm):
class Meta:
fields = ["foo"]

View File

@ -0,0 +1,16 @@
from django import forms
class TestModelForm1(forms.ModelForm):
class Meta:
fields = "__all__"
class TestModelForm2(forms.ModelForm):
class Meta:
fields = b"__all__"
class TestModelForm3(forms.ModelForm):
class Meta:
fields = ["foo"]

View File

@ -812,6 +812,21 @@ where
self, body,
));
}
if self.settings.rules.enabled(&Rule::ExcludeWithModelForm) {
if let Some(diagnostic) =
flake8_django::rules::exclude_with_model_form(self, bases, body)
{
self.diagnostics.push(diagnostic);
}
}
if self.settings.rules.enabled(&Rule::AllWithModelForm) {
if let Some(diagnostic) =
flake8_django::rules::all_with_model_form(self, bases, body)
{
self.diagnostics.push(diagnostic);
}
}
if self.settings.rules.enabled(&Rule::ModelWithoutDunderStr) {
if let Some(diagnostic) =
flake8_django::rules::model_without_dunder_str(self, bases, body, stmt)
@ -2941,6 +2956,11 @@ where
{
pylint::rules::logging_call(self, func, args, keywords);
}
// flake8-django
if self.settings.rules.enabled(&Rule::LocalsInRenderFunction) {
flake8_django::rules::locals_in_render_function(self, func, args, keywords);
}
}
ExprKind::Dict { keys, values } => {
if self

View File

@ -621,6 +621,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
// flake8-django
(Flake8Django, "001") => Rule::NullableModelStringField,
(Flake8Django, "003") => Rule::LocalsInRenderFunction,
(Flake8Django, "006") => Rule::ExcludeWithModelForm,
(Flake8Django, "007") => Rule::AllWithModelForm,
(Flake8Django, "008") => Rule::ModelWithoutDunderStr,
(Flake8Django, "013") => Rule::NonLeadingReceiverDecorator,

View File

@ -581,6 +581,9 @@ ruff_macros::register_rules!(
rules::ruff::rules::UnusedNOQA,
// flake8-django
rules::flake8_django::rules::NullableModelStringField,
rules::flake8_django::rules::LocalsInRenderFunction,
rules::flake8_django::rules::ExcludeWithModelForm,
rules::flake8_django::rules::AllWithModelForm,
rules::flake8_django::rules::ModelWithoutDunderStr,
rules::flake8_django::rules::NonLeadingReceiverDecorator,
);

View File

@ -14,6 +14,9 @@ mod tests {
use crate::test::test_path;
#[test_case(Rule::NullableModelStringField, Path::new("DJ001.py"); "DJ001")]
#[test_case(Rule::LocalsInRenderFunction, Path::new("DJ003.py"); "DJ003")]
#[test_case(Rule::ExcludeWithModelForm, Path::new("DJ006.py"); "DJ006")]
#[test_case(Rule::AllWithModelForm, Path::new("DJ007.py"); "DJ007")]
#[test_case(Rule::ModelWithoutDunderStr, Path::new("DJ008.py"); "DJ008")]
#[test_case(Rule::NonLeadingReceiverDecorator, Path::new("DJ013.py"); "DJ013")]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {

View File

@ -0,0 +1,96 @@
use rustpython_parser::ast::{Constant, Expr, ExprKind, Stmt, StmtKind};
use ruff_macros::{define_violation, derive_message_formats};
use crate::rules::flake8_django::rules::helpers::is_model_form;
use crate::violation::Violation;
use crate::{checkers::ast::Checker, registry::Diagnostic, Range};
define_violation!(
/// ## What it does
/// Checks for the use of `fields = "__all__"` in Django `ModelForm`
/// classes.
///
/// ## Why is this bad?
/// If a `ModelForm` includes the `fields = "__all__"` attribute, any new
/// field that is added to the model will automatically be exposed for
/// modification.
///
/// ## Example
/// ```python
/// from django.forms import ModelForm
///
/// class PostForm(ModelForm):
/// class Meta:
/// model = Post
/// fields = "__all__"
/// ```
///
/// Use instead:
/// ```python
/// from django.forms import ModelForm
///
/// class PostForm(ModelForm):
/// class Meta:
/// model = Post
/// fields = ["title", "content"]
/// ```
pub struct AllWithModelForm;
);
impl Violation for AllWithModelForm {
#[derive_message_formats]
fn message(&self) -> String {
format!("Do not use `__all__` with `ModelForm`, use `fields` instead")
}
}
/// 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)) {
return None;
}
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 != "fields" {
continue;
}
let ExprKind::Constant { value, .. } = &value.node else {
continue;
};
match &value {
Constant::Str(s) => {
if s == "__all__" {
return Some(Diagnostic::new(
AllWithModelForm,
Range::from_located(element),
));
}
}
Constant::Bytes(b) => {
if b == "__all__".as_bytes() {
return Some(Diagnostic::new(
AllWithModelForm,
Range::from_located(element),
));
}
}
_ => (),
};
}
}
}
None
}

View File

@ -0,0 +1,79 @@
use rustpython_parser::ast::{Expr, ExprKind, Stmt, StmtKind};
use ruff_macros::{define_violation, derive_message_formats};
use crate::rules::flake8_django::rules::helpers::is_model_form;
use crate::violation::Violation;
use crate::{checkers::ast::Checker, registry::Diagnostic, Range};
define_violation!(
/// ## What it does
/// Checks for the use of `exclude` in Django `ModelForm` classes.
///
/// ## Why is this bad?
/// If a `ModelForm` includes the `exclude` attribute, any new field that
/// is added to the model will automatically be exposed for modification.
///
/// ## Example
/// ```python
/// from django.forms import ModelForm
///
/// class PostForm(ModelForm):
/// class Meta:
/// model = Post
/// exclude = ["author"]
/// ```
///
/// Use instead:
/// ```python
/// from django.forms import ModelForm
///
/// class PostForm(ModelForm):
/// class Meta:
/// model = Post
/// fields = ["title", "content"]
/// ```
pub struct ExcludeWithModelForm;
);
impl Violation for ExcludeWithModelForm {
#[derive_message_formats]
fn message(&self) -> String {
format!("Do not use `exclude` with `ModelForm`, use `fields` instead")
}
}
/// DJ006
pub fn exclude_with_model_form(
checker: &Checker,
bases: &[Expr],
body: &[Stmt],
) -> Option<Diagnostic> {
if !bases.iter().any(|base| is_model_form(checker, base)) {
return None;
}
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, .. } = &element.node else {
continue;
};
for target in targets.iter() {
let ExprKind::Name { id, .. } = &target.node else {
continue;
};
if id == "exclude" {
return Some(Diagnostic::new(
ExcludeWithModelForm,
Range::from_located(target),
));
}
}
}
}
None
}

View File

@ -2,13 +2,22 @@ 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.
/// 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.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.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 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.resolve_call_path(expr).and_then(|call_path| {
let call_path = call_path.as_slice();

View File

@ -0,0 +1,88 @@
use rustpython_parser::ast::{Expr, ExprKind, Keyword};
use ruff_macros::{define_violation, derive_message_formats};
use crate::{checkers::ast::Checker, registry::Diagnostic, violation::Violation, Range};
define_violation!(
/// ## What it does
/// Checks for the use of `locals()` in `render` functions.
///
/// ## Why is this bad?
/// Using `locals()` can expose internal variables or other unintentional
/// data to the rendered template.
///
/// ## Example
/// ```python
/// from django.shortcuts import render
///
/// def index(request):
/// posts = Post.objects.all()
/// return render(request, "app/index.html", locals())
/// ```
///
/// Use instead:
/// ```python
/// from django.shortcuts import render
///
/// def index(request):
/// posts = Post.objects.all()
/// context = {"posts": posts}
/// return render(request, "app/index.html", context)
/// ```
pub struct LocalsInRenderFunction;
);
impl Violation for LocalsInRenderFunction {
#[derive_message_formats]
fn message(&self) -> String {
format!("Avoid passing `locals()` as context to a `render` function")
}
}
/// DJ003
pub fn locals_in_render_function(
checker: &mut Checker,
func: &Expr,
args: &[Expr],
keywords: &[Keyword],
) {
if !checker.resolve_call_path(func).map_or(false, |call_path| {
call_path.as_slice() == ["django", "shortcuts", "render"]
}) {
return;
}
let locals = if args.len() >= 3 {
if !is_locals_call(checker, &args[2]) {
return;
}
&args[2]
} else if let Some(keyword) = keywords.iter().find(|keyword| {
keyword
.node
.arg
.as_ref()
.map_or(false, |arg| arg == "context")
}) {
if !is_locals_call(checker, &keyword.node.value) {
return;
}
&keyword.node.value
} else {
return;
};
checker.diagnostics.push(Diagnostic::new(
LocalsInRenderFunction,
Range::from_located(locals),
));
}
fn is_locals_call(checker: &Checker, expr: &Expr) -> bool {
let ExprKind::Call { func, .. } = &expr.node else {
return false
};
checker
.resolve_call_path(func)
.map_or(false, |call_path| call_path.as_slice() == ["", "locals"])
}

View File

@ -1,10 +1,16 @@
pub use all_with_model_form::{all_with_model_form, AllWithModelForm};
pub use exclude_with_model_form::{exclude_with_model_form, ExcludeWithModelForm};
pub use locals_in_render_function::{locals_in_render_function, LocalsInRenderFunction};
pub use model_without_dunder_str::{model_without_dunder_str, ModelWithoutDunderStr};
pub use non_leading_receiver_decorator::{
non_leading_receiver_decorator, NonLeadingReceiverDecorator,
};
pub use nullable_model_string_field::{nullable_model_string_field, NullableModelStringField};
mod all_with_model_form;
mod exclude_with_model_form;
mod helpers;
mod locals_in_render_function;
mod model_without_dunder_str;
mod non_leading_receiver_decorator;
mod nullable_model_string_field;

View File

@ -0,0 +1,24 @@
---
source: crates/ruff/src/rules/flake8_django/mod.rs
expression: diagnostics
---
- kind:
LocalsInRenderFunction: ~
location:
row: 5
column: 41
end_location:
row: 5
column: 49
fix: ~
parent: ~
- kind:
LocalsInRenderFunction: ~
location:
row: 9
column: 49
end_location:
row: 9
column: 57
fix: ~
parent: ~

View File

@ -0,0 +1,14 @@
---
source: crates/ruff/src/rules/flake8_django/mod.rs
expression: diagnostics
---
- kind:
ExcludeWithModelForm: ~
location:
row: 6
column: 8
end_location:
row: 6
column: 15
fix: ~
parent: ~

View File

@ -0,0 +1,24 @@
---
source: crates/ruff/src/rules/flake8_django/mod.rs
expression: diagnostics
---
- kind:
AllWithModelForm: ~
location:
row: 6
column: 8
end_location:
row: 6
column: 26
fix: ~
parent: ~
- kind:
AllWithModelForm: ~
location:
row: 11
column: 8
end_location:
row: 11
column: 27
fix: ~
parent: ~

View File

@ -1,14 +1,13 @@
#![allow(dead_code)]
#![allow(dead_code, unused_imports)]
use rustpython_parser::ast::Location;
use rustpython_parser::Tok;
use ruff_macros::{define_violation, derive_message_formats};
use crate::registry::DiagnosticKind;
use crate::violation::Violation;
use crate::rules::pycodestyle::helpers::{is_keyword_token, is_singleton_token};
use rustpython_parser::ast::Location;
use rustpython_parser::Tok;
use crate::violation::Violation;
define_violation!(
pub struct MissingWhitespaceAfterKeyword;

3
ruff.schema.json generated
View File

@ -1538,6 +1538,9 @@
"DJ0",
"DJ00",
"DJ001",
"DJ003",
"DJ006",
"DJ007",
"DJ008",
"DJ01",
"DJ013",