From 714b1d7736918c13cc8b9ae9d563c0bce32e7edb Mon Sep 17 00:00:00 2001 From: Petter Friberg Date: Fri, 8 Dec 2023 10:15:35 +0100 Subject: [PATCH 1/3] Ensure mypy plugin processes inherited many to many fields --- mypy_django_plugin/transformers/models.py | 49 ++++++++++++++----- tests/typecheck/fields/test_related.yml | 41 ++++++++++++++++ .../typecheck/models/test_contrib_models.yml | 11 +++++ 3 files changed, 89 insertions(+), 12 deletions(-) diff --git a/mypy_django_plugin/transformers/models.py b/mypy_django_plugin/transformers/models.py index b2f36a774..3299c65ae 100644 --- a/mypy_django_plugin/transformers/models.py +++ b/mypy_django_plugin/transformers/models.py @@ -1,3 +1,5 @@ +from collections import deque +from collections.abc import Iterable from functools import cached_property from typing import Any, Dict, List, Optional, Type, Union, cast @@ -15,6 +17,7 @@ Expression, NameExpr, RefExpr, + Statement, StrExpr, SymbolTableNode, TypeInfo, @@ -626,6 +629,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? @@ -642,26 +667,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 # = 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 diff --git a/tests/typecheck/fields/test_related.yml b/tests/typecheck/fields/test_related.yml index 9b49568a7..99de4ddbd 100644 --- a/tests/typecheck/fields/test_related.yml +++ b/tests/typecheck/fields/test_related.yml @@ -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") diff --git a/tests/typecheck/models/test_contrib_models.yml b/tests/typecheck/models/test_contrib_models.yml index 3daf86997..717044bc3 100644 --- a/tests/typecheck/models/test_contrib_models.yml +++ b/tests/typecheck/models/test_contrib_models.yml @@ -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' From d679a4f790a21f03e1efbaa89c666caea2b2c953 Mon Sep 17 00:00:00 2001 From: Petter Friberg Date: Fri, 8 Dec 2023 10:25:01 +0100 Subject: [PATCH 2/3] fixup! Ensure mypy plugin processes inherited many to many fields --- mypy_django_plugin/transformers/models.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mypy_django_plugin/transformers/models.py b/mypy_django_plugin/transformers/models.py index 3299c65ae..7a28fd899 100644 --- a/mypy_django_plugin/transformers/models.py +++ b/mypy_django_plugin/transformers/models.py @@ -1,7 +1,6 @@ from collections import deque -from collections.abc import Iterable 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 From 64882e97867ffaafd4fc99cfc07301a92316d7a0 Mon Sep 17 00:00:00 2001 From: Petter Friberg Date: Fri, 8 Dec 2023 10:27:24 +0100 Subject: [PATCH 3/3] fixup! fixup! Ensure mypy plugin processes inherited many to many fields --- scripts/stubtest/allowlist_todo.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/scripts/stubtest/allowlist_todo.txt b/scripts/stubtest/allowlist_todo.txt index a29a97a2b..bcbd450b2 100644 --- a/scripts/stubtest/allowlist_todo.txt +++ b/scripts/stubtest/allowlist_todo.txt @@ -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