mirror of https://github.com/astral-sh/ruff
[`pep8-naming`] Allow Django model loads in `non-lowercase-variable-in-function` (`N806`) (#8917)
## Summary
Allows assignments of the form, e.g., `Attachment =
apps.get_model("zerver", "Attachment")`, for better compatibility with
Django.
Closes https://github.com/astral-sh/ruff/issues/7675.
## Test Plan
`cargo test`
This commit is contained in:
parent
774c77adae
commit
c324cb6202
|
|
@ -1,6 +1,6 @@
|
||||||
import collections
|
import collections
|
||||||
from collections import namedtuple
|
from collections import namedtuple
|
||||||
from typing import TypeAlias, TypeVar, NewType, NamedTuple, TypedDict
|
from typing import Type, TypeAlias, TypeVar, NewType, NamedTuple, TypedDict
|
||||||
|
|
||||||
GLOBAL: str = "foo"
|
GLOBAL: str = "foo"
|
||||||
|
|
||||||
|
|
@ -40,3 +40,15 @@ def loop_assign():
|
||||||
global CURRENT_PORT
|
global CURRENT_PORT
|
||||||
for CURRENT_PORT in range(5):
|
for CURRENT_PORT in range(5):
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
def model_assign() -> None:
|
||||||
|
Bad = apps.get_model("zerver", "Stream") # N806
|
||||||
|
Attachment = apps.get_model("zerver", "Attachment") # OK
|
||||||
|
Recipient = apps.get_model("zerver", model_name="Recipient") # OK
|
||||||
|
Address: Type = apps.get_model("zerver", "Address") # OK
|
||||||
|
|
||||||
|
from django.utils.module_loading import import_string
|
||||||
|
|
||||||
|
Bad = import_string("django.core.exceptions.ValidationError") # N806
|
||||||
|
ValidationError = import_string("django.core.exceptions.ValidationError") # OK
|
||||||
|
|
|
||||||
|
|
@ -205,19 +205,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
|
||||||
ExprContext::Store => {
|
ExprContext::Store => {
|
||||||
if checker.enabled(Rule::NonLowercaseVariableInFunction) {
|
if checker.enabled(Rule::NonLowercaseVariableInFunction) {
|
||||||
if checker.semantic.current_scope().kind.is_function() {
|
if checker.semantic.current_scope().kind.is_function() {
|
||||||
// Ignore globals.
|
pep8_naming::rules::non_lowercase_variable_in_function(
|
||||||
if !checker
|
checker, expr, id,
|
||||||
.semantic
|
);
|
||||||
.current_scope()
|
|
||||||
.get(id)
|
|
||||||
.is_some_and(|binding_id| {
|
|
||||||
checker.semantic.binding(binding_id).is_global()
|
|
||||||
})
|
|
||||||
{
|
|
||||||
pep8_naming::rules::non_lowercase_variable_in_function(
|
|
||||||
checker, expr, id,
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if checker.enabled(Rule::MixedCaseVariableInClassScope) {
|
if checker.enabled(Rule::MixedCaseVariableInClassScope) {
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,5 @@
|
||||||
use itertools::Itertools;
|
use itertools::Itertools;
|
||||||
|
use ruff_python_ast::call_path::collect_call_path;
|
||||||
use ruff_python_ast::{self as ast, Arguments, Expr, Stmt};
|
use ruff_python_ast::{self as ast, Arguments, Expr, Stmt};
|
||||||
|
|
||||||
use ruff_python_semantic::SemanticModel;
|
use ruff_python_semantic::SemanticModel;
|
||||||
|
|
@ -72,6 +73,7 @@ pub(super) fn is_type_alias_assignment(stmt: &Stmt, semantic: &SemanticModel) ->
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns `true` if the statement is an assignment to a `TypedDict`.
|
||||||
pub(super) fn is_typed_dict_class(arguments: Option<&Arguments>, semantic: &SemanticModel) -> bool {
|
pub(super) fn is_typed_dict_class(arguments: Option<&Arguments>, semantic: &SemanticModel) -> bool {
|
||||||
arguments.is_some_and(|arguments| {
|
arguments.is_some_and(|arguments| {
|
||||||
arguments
|
arguments
|
||||||
|
|
@ -81,6 +83,67 @@ pub(super) fn is_typed_dict_class(arguments: Option<&Arguments>, semantic: &Sema
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns `true` if a statement appears to be a dynamic import of a Django model.
|
||||||
|
///
|
||||||
|
/// For example, in Django, it's common to use `get_model` to access a model dynamically, as in:
|
||||||
|
/// ```python
|
||||||
|
/// def migrate_existing_attachment_data(
|
||||||
|
/// apps: StateApps, schema_editor: BaseDatabaseSchemaEditor
|
||||||
|
/// ) -> None:
|
||||||
|
/// Attachment = apps.get_model("zerver", "Attachment")
|
||||||
|
/// ```
|
||||||
|
pub(super) fn is_django_model_import(name: &str, stmt: &Stmt, semantic: &SemanticModel) -> bool {
|
||||||
|
fn match_model_import(name: &str, expr: &Expr, semantic: &SemanticModel) -> bool {
|
||||||
|
let Expr::Call(ast::ExprCall {
|
||||||
|
func, arguments, ..
|
||||||
|
}) = expr
|
||||||
|
else {
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
|
||||||
|
// Match against, e.g., `apps.get_model("zerver", "Attachment")`.
|
||||||
|
if let Some(call_path) = collect_call_path(func.as_ref()) {
|
||||||
|
if matches!(call_path.as_slice(), [.., "get_model"]) {
|
||||||
|
if let Some(argument) =
|
||||||
|
arguments.find_argument("model_name", arguments.args.len() - 1)
|
||||||
|
{
|
||||||
|
if let Some(string_literal) = argument.as_string_literal_expr() {
|
||||||
|
return string_literal.value.to_str() == name;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Match against, e.g., `import_string("zerver.models.Attachment")`.
|
||||||
|
if let Some(call_path) = semantic.resolve_call_path(func.as_ref()) {
|
||||||
|
if matches!(
|
||||||
|
call_path.as_slice(),
|
||||||
|
["django", "utils", "module_loading", "import_string"]
|
||||||
|
) {
|
||||||
|
if let Some(argument) = arguments.find_argument("dotted_path", 0) {
|
||||||
|
if let Some(string_literal) = argument.as_string_literal_expr() {
|
||||||
|
if let Some((.., model)) = string_literal.value.to_str().rsplit_once('.') {
|
||||||
|
return model == name;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
false
|
||||||
|
}
|
||||||
|
|
||||||
|
match stmt {
|
||||||
|
Stmt::AnnAssign(ast::StmtAnnAssign {
|
||||||
|
value: Some(value), ..
|
||||||
|
}) => match_model_import(name, value.as_ref(), semantic),
|
||||||
|
Stmt::Assign(ast::StmtAssign { value, .. }) => {
|
||||||
|
match_model_import(name, value.as_ref(), semantic)
|
||||||
|
}
|
||||||
|
_ => false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::{is_acronym, is_camelcase, is_mixed_case};
|
use super::{is_acronym, is_camelcase, is_mixed_case};
|
||||||
|
|
|
||||||
|
|
@ -53,6 +53,15 @@ impl Violation for NonLowercaseVariableInFunction {
|
||||||
|
|
||||||
/// N806
|
/// N806
|
||||||
pub(crate) fn non_lowercase_variable_in_function(checker: &mut Checker, expr: &Expr, name: &str) {
|
pub(crate) fn non_lowercase_variable_in_function(checker: &mut Checker, expr: &Expr, name: &str) {
|
||||||
|
// Ignore globals.
|
||||||
|
if checker
|
||||||
|
.semantic()
|
||||||
|
.lookup_symbol(name)
|
||||||
|
.is_some_and(|id| checker.semantic().binding(id).is_global())
|
||||||
|
{
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
if checker
|
if checker
|
||||||
.settings
|
.settings
|
||||||
.pep8_naming
|
.pep8_naming
|
||||||
|
|
@ -72,6 +81,7 @@ pub(crate) fn non_lowercase_variable_in_function(checker: &mut Checker, expr: &E
|
||||||
|| helpers::is_typed_dict_assignment(parent, checker.semantic())
|
|| helpers::is_typed_dict_assignment(parent, checker.semantic())
|
||||||
|| helpers::is_type_var_assignment(parent, checker.semantic())
|
|| helpers::is_type_var_assignment(parent, checker.semantic())
|
||||||
|| helpers::is_type_alias_assignment(parent, checker.semantic())
|
|| helpers::is_type_alias_assignment(parent, checker.semantic())
|
||||||
|
|| helpers::is_django_model_import(name, parent, checker.semantic())
|
||||||
{
|
{
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -20,4 +20,22 @@ N806.py:13:5: N806 Variable `CONSTANT` in function should be lowercase
|
||||||
14 | _ = 0
|
14 | _ = 0
|
||||||
|
|
|
|
||||||
|
|
||||||
|
N806.py:46:5: N806 Variable `Bad` in function should be lowercase
|
||||||
|
|
|
||||||
|
45 | def model_assign() -> None:
|
||||||
|
46 | Bad = apps.get_model("zerver", "Stream") # N806
|
||||||
|
| ^^^ N806
|
||||||
|
47 | Attachment = apps.get_model("zerver", "Attachment") # OK
|
||||||
|
48 | Recipient = apps.get_model("zerver", model_name="Recipient") # OK
|
||||||
|
|
|
||||||
|
|
||||||
|
N806.py:53:5: N806 Variable `Bad` in function should be lowercase
|
||||||
|
|
|
||||||
|
51 | from django.utils.module_loading import import_string
|
||||||
|
52 |
|
||||||
|
53 | Bad = import_string("django.core.exceptions.ValidationError") # N806
|
||||||
|
| ^^^ N806
|
||||||
|
54 | ValidationError = import_string("django.core.exceptions.ValidationError") # OK
|
||||||
|
|
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue