Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure mypy plugin processes inherited many to many fields #1864

Merged
merged 3 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 37 additions & 13 deletions mypy_django_plugin/transformers/models.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from collections import deque
from functools import cached_property
from typing import Any, Dict, List, Optional, Type, Union, cast
from typing import Any, Dict, Iterable, List, Optional, Type, Union, cast

from django.db.models import Manager, Model
from django.db.models.fields import DateField, DateTimeField, Field
Expand All @@ -15,6 +16,7 @@
Expression,
NameExpr,
RefExpr,
Statement,
StrExpr,
SymbolTableNode,
TypeInfo,
Expand Down Expand Up @@ -626,6 +628,28 @@ class ProcessManyToManyFields(ModelClassInitializer):
where an explicit 'through' argument has been passed.
"""

def statements(self) -> Iterable[Statement]:
"""
Returns class body statements from the current model and any of its bases that
is an abstract model. Statements from any concrete parent class or parents of
that concrete class will be skipped.
"""
processed_models = set()
# Produce all statements from current class
model_bases = deque([self.model_classdef])
# Do a breadth first search over the current model and its bases, to find all
# abstract parent models that have not been "interrupted" by any concrete model.
while model_bases:
model = model_bases.popleft()
yield from model.defs.body
for base in model.info.bases:
# Only produce any additional statements from abstract model bases, as they
# simulate regular python inheritance. Avoid concrete models, and any of their
# parents, as they're handled differently by Django.
if helpers.is_abstract_model(base.type) and base.type.fullname not in processed_models:
model_bases.append(base.type.defn)
processed_models.add(base.type.fullname)

def run(self) -> None:
if self.is_model_abstract:
# TODO: Create abstract through models?
Expand All @@ -642,26 +666,26 @@ def run(self) -> None:
from_pk = self.get_pk_instance(self.model_classdef.info)
fk_set_type, fk_get_type = get_field_descriptor_types(fk_field, is_set_nullable=False, is_get_nullable=False)

for defn in self.model_classdef.defs.body:
for statement in self.statements():
# Check if this part of the class body is an assignment from a 'ManyToManyField' call
# <field> = ManyToManyField(...)
if (
isinstance(defn, AssignmentStmt)
and len(defn.lvalues) == 1
and isinstance(defn.lvalues[0], NameExpr)
and isinstance(defn.rvalue, CallExpr)
and len(defn.rvalue.args) > 0 # Need at least the 'to' argument
and isinstance(defn.rvalue.callee, RefExpr)
and isinstance(defn.rvalue.callee.node, TypeInfo)
and defn.rvalue.callee.node.has_base(fullnames.MANYTOMANY_FIELD_FULLNAME)
isinstance(statement, AssignmentStmt)
and len(statement.lvalues) == 1
and isinstance(statement.lvalues[0], NameExpr)
and isinstance(statement.rvalue, CallExpr)
and len(statement.rvalue.args) > 0 # Need at least the 'to' argument
and isinstance(statement.rvalue.callee, RefExpr)
and isinstance(statement.rvalue.callee.node, TypeInfo)
and statement.rvalue.callee.node.has_base(fullnames.MANYTOMANY_FIELD_FULLNAME)
):
m2m_field_name = defn.lvalues[0].name
m2m_field_symbol = self.model_classdef.info.names.get(m2m_field_name)
m2m_field_name = statement.lvalues[0].name
m2m_field_symbol = self.model_classdef.info.get(m2m_field_name)
# The symbol referred to by the assignment expression is expected to be a variable
if m2m_field_symbol is None or not isinstance(m2m_field_symbol.node, Var):
continue
# Resolve argument information of the 'ManyToManyField(...)' call
args = self.resolve_many_to_many_arguments(defn.rvalue, context=defn)
args = self.resolve_many_to_many_arguments(statement.rvalue, context=statement)
if (
# Ignore calls without required 'to' argument, mypy will complain
args is None
Expand Down
2 changes: 0 additions & 2 deletions scripts/stubtest/allowlist_todo.txt
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,12 @@ django.contrib.auth.models.AnonymousUser.__int__
django.contrib.auth.models.Group.id
django.contrib.auth.models.Group.name
django.contrib.auth.models.Group.permissions
django.contrib.auth.models.Group.user_set
django.contrib.auth.models.GroupManager.__slotnames__
django.contrib.auth.models.Permission.codename
django.contrib.auth.models.Permission.content_type
django.contrib.auth.models.Permission.content_type_id
django.contrib.auth.models.Permission.id
django.contrib.auth.models.Permission.name
django.contrib.auth.models.Permission.user_set
django.contrib.auth.models.PermissionManager.__slotnames__
django.contrib.auth.models.PermissionsMixin.Meta.abstract
django.contrib.auth.models.PermissionsMixin.groups
Expand Down
41 changes: 41 additions & 0 deletions tests/typecheck/fields/test_related.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1274,3 +1274,44 @@
field = models.ForeignKey("slef", on_delete=models.CASCADE) # E: Cannot find model 'slef' referenced in field 'field' [misc]
unknown = models.OneToOneField("unknown", on_delete=models.CASCADE) # E: Cannot find model 'unknown' referenced in field 'unknown' [misc]
bad = models.ManyToManyField("bad") # E: Need type annotation for "bad" [var-annotated]

- case: test_reverse_of_inherited_many_to_many_fields
main: |
from myapp.models import Other
other = Other()
reveal_type(other.abstract_of_concrete_parent) # N: Revealed type is "myapp.models.ConcreteParent_ManyRelatedManager"
reveal_type(other.concrete_parent) # N: Revealed type is "myapp.models.ConcreteParent_ManyRelatedManager"
reveal_type(other.abstract_of_abstract_parent) # N: Revealed type is "myapp.models.MyModel_ManyRelatedManager"
reveal_type(other.abstract_parent) # N: Revealed type is "myapp.models.MyModel_ManyRelatedManager"
reveal_type(other.mymodel) # N: Revealed type is "myapp.models.MyModel_ManyRelatedManager"
installed_apps:
- myapp
files:
- path: myapp/__init__.py
- path: myapp/models.py
content: |
from django.db import models

class Other(models.Model):
...

class AbstractOfConcreteParent(models.Model):
m2m_1 = models.ManyToManyField(Other, related_name="abstract_of_concrete_parent")
class Meta:
abstract = True

class ConcreteParent(AbstractOfConcreteParent):
m2m_2 = models.ManyToManyField(Other, related_name="concrete_parent")

class AbstractOfAbstractParent(models.Model):
m2m_3 = models.ManyToManyField(Other, related_name="abstract_of_abstract_parent")
class Meta:
abstract = True

class AbstractParent(AbstractOfAbstractParent):
m2m_4 = models.ManyToManyField(Other, related_name="abstract_parent")
class Meta:
abstract = True

class MyModel(ConcreteParent, AbstractParent):
m2m_5 = models.ManyToManyField(Other, related_name="mymodel")
11 changes: 11 additions & 0 deletions tests/typecheck/models/test_contrib_models.yml
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,14 @@

class Other(models.Model):
user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE)

- case: test_permissions_inherited_reverese_relations
main: |
from django.contrib.auth.models import Permission
reveal_type(Permission().user_set) # N: Revealed type is "django.contrib.auth.models.User_ManyRelatedManager"

from django.contrib.auth.models import Group
reveal_type(Group().user_set) # N: Revealed type is "django.contrib.auth.models.User_ManyRelatedManager"
custom_settings: |
INSTALLED_APPS = ('django.contrib.contenttypes', 'django.contrib.auth')
AUTH_USER_MODEL='auth.User'