Make ReverseManyToOneDescriptor generic over a model

This allows us to remove some plugin code, as we now can fall back to
the default related manager on the descriptor instead of having to
implement the fallback behaviour manually.

django-stubs/db/models/fields/related_descriptors.pyi

@@ -73,7 +73,7 @@ class ReverseOneToOneDescriptor(Generic[_From, _To]):
     def __set__(self, instance: _From, value: _To | None) -> None: ...
     def __reduce__(self) -> tuple[Callable[..., Any], tuple[type[_To], str]]: ...
 
-class ReverseManyToOneDescriptor:
+class ReverseManyToOneDescriptor(Generic[_To]):
     """
     In the example::
 
@@ -84,29 +84,29 @@ class ReverseManyToOneDescriptor:
     """
 
     rel: ManyToOneRel
-    field: ForeignKey
+    field: ForeignKey[_To, _To]
     def __init__(self, rel: ManyToOneRel) -> None: ...
     @cached_property
-    def related_manager_cls(self) -> type[RelatedManager[Any]]: ...
+    def related_manager_cls(self) -> type[RelatedManager[_To]]: ...
     @overload
     def __get__(self, instance: None, cls: Any = ...) -> Self: ...
     @overload
-    def __get__(self, instance: Model, cls: Any = ...) -> RelatedManager[Any]: ...
+    def __get__(self, instance: Model, cls: Any = ...) -> RelatedManager[_To]: ...
     def __set__(self, instance: Any, value: Any) -> NoReturn: ...
 
 # Fake class, Django defines 'RelatedManager' inside a function body
 @type_check_only
-class RelatedManager(Manager[_M], Generic[_M]):
+class RelatedManager(Manager[_To], Generic[_To]):
     related_val: tuple[int, ...]
-    def add(self, *objs: _M | int, bulk: bool = ...) -> None: ...
-    async def aadd(self, *objs: _M | int, bulk: bool = ...) -> None: ...
-    def remove(self, *objs: _M | int, bulk: bool = ...) -> None: ...
-    async def aremove(self, *objs: _M | int, bulk: bool = ...) -> None: ...
-    def set(self, objs: QuerySet[_M] | Iterable[_M | int], *, bulk: bool = ..., clear: bool = ...) -> None: ...
-    async def aset(self, objs: QuerySet[_M] | Iterable[_M | int], *, bulk: bool = ..., clear: bool = ...) -> None: ...
+    def add(self, *objs: _To | int, bulk: bool = ...) -> None: ...
+    async def aadd(self, *objs: _To | int, bulk: bool = ...) -> None: ...
+    def remove(self, *objs: _To | int, bulk: bool = ...) -> None: ...
+    async def aremove(self, *objs: _To | int, bulk: bool = ...) -> None: ...
+    def set(self, objs: QuerySet[_To] | Iterable[_To | int], *, bulk: bool = ..., clear: bool = ...) -> None: ...
+    async def aset(self, objs: QuerySet[_To] | Iterable[_To | int], *, bulk: bool = ..., clear: bool = ...) -> None: ...
     def clear(self) -> None: ...
     async def aclear(self) -> None: ...
-    def __call__(self, *, manager: str) -> RelatedManager[_M]: ...
+    def __call__(self, *, manager: str) -> RelatedManager[_To]: ...
 
 def create_reverse_many_to_one_manager(
     superclass: type[BaseManager[_M]], rel: ManyToOneRel

mypy_django_plugin/lib/fullnames.py

@@ -34,6 +34,7 @@
 }
 
 REVERSE_ONE_TO_ONE_DESCRIPTOR = "django.db.models.fields.related_descriptors.ReverseOneToOneDescriptor"
+REVERSE_MANY_TO_ONE_DESCRIPTOR = "django.db.models.fields.related_descriptors.ReverseManyToOneDescriptor"
 MANY_TO_MANY_DESCRIPTOR = "django.db.models.fields.related_descriptors.ManyToManyDescriptor"
 MANY_RELATED_MANAGER = "django.db.models.fields.related_descriptors.ManyRelatedManager"
 RELATED_FIELDS_CLASSES = frozenset(

mypy_django_plugin/main.py

@@ -24,7 +24,17 @@
 from mypy_django_plugin.django.context import DjangoContext
 from mypy_django_plugin.exceptions import UnregisteredModelError
 from mypy_django_plugin.lib import fullnames, helpers
-from mypy_django_plugin.transformers import fields, forms, init_create, manytomany, meta, querysets, request, settings
+from mypy_django_plugin.transformers import (
+    fields,
+    forms,
+    init_create,
+    manytomany,
+    manytoone,
+    meta,
+    querysets,
+    request,
+    settings,
+)
 from mypy_django_plugin.transformers.functional import resolve_str_promise_attribute
 from mypy_django_plugin.transformers.managers import (
     create_new_manager_class_from_as_manager_method,
@@ -190,11 +200,13 @@ def get_method_hook(self, fullname: str) -> Optional[Callable[[MethodContext], M
             if info and info.has_base(fullnames.FORM_MIXIN_CLASS_FULLNAME):
                 return forms.extract_proper_type_for_get_form
 
-        elif method_name == "__get__" and class_fullname in {
-            fullnames.MANYTOMANY_FIELD_FULLNAME,
-            fullnames.MANY_TO_MANY_DESCRIPTOR,
-        }:
-            return manytomany.refine_many_to_many_related_manager
+        elif method_name == "__get__":
+            hooks = {
+                fullnames.MANYTOMANY_FIELD_FULLNAME: manytomany.refine_many_to_many_related_manager,
+                fullnames.MANY_TO_MANY_DESCRIPTOR: manytomany.refine_many_to_many_related_manager,
+                fullnames.REVERSE_MANY_TO_ONE_DESCRIPTOR: manytoone.refine_many_to_one_related_manager,
+            }
+            return hooks.get(class_fullname)
 
         manager_classes = self._get_current_manager_bases()
 

mypy_django_plugin/transformers/manytoone.py

@@ -0,0 +1,56 @@
+from typing import Optional, Tuple
+
+from mypy.plugin import MethodContext
+from mypy.types import Instance
+from mypy.types import Type as MypyType
+
+from mypy_django_plugin.lib import fullnames, helpers
+
+
+def get_related_manager_and_model(ctx: MethodContext) -> Optional[Tuple[Instance, Instance]]:
+    """
+    Returns a 2-tuple consisting of:
+      1. A `RelatedManager` instance
+      2. The type parameter (_To) instance of 1. when it's a model
+    When encountering a `RelatedManager` that has populated its first type parameter
+    with a model. Otherwise `None` is returned.
+
+    For example: if given a `RelatedManager[A]` where `A` is a model the following
+    2-tuple is returned: `(RelatedManager[A], A)`.
+    """
+    if (
+        isinstance(ctx.default_return_type, Instance)
+        and ctx.default_return_type.type.fullname == fullnames.RELATED_MANAGER_CLASS
+    ):
+        # This is a call to '__get__' overload with a model instance of
+        # 'ReverseManyToOneDescriptor'. Returning a 'RelatedManager'. Which we want to,
+        # just like Django, build from the default manager of the related model.
+        related_manager = ctx.default_return_type
+        # Require first type argument of 'RelatedManager' to be a model
+        if (
+            len(related_manager.args) >= 1
+            and isinstance(related_manager.args[0], Instance)
+            and helpers.is_model_type(related_manager.args[0].type)
+        ):
+            return related_manager, related_manager.args[0]
+
+    return None
+
+
+def refine_many_to_one_related_manager(ctx: MethodContext) -> MypyType:
+    """
+    Updates the 'RelatedManager' returned by e.g. 'ReverseManyToOneDescriptor' to be a subclass
+    of 'RelatedManager' and the related model's default manager.
+    """
+    related_objects = get_related_manager_and_model(ctx)
+    if related_objects is None:
+        return ctx.default_return_type
+
+    related_manager, to_model_instance = related_objects
+    checker = helpers.get_typechecker_api(ctx)
+    related_manager_info = helpers.get_reverse_manager_info(
+        checker, to_model_instance.type, derived_from="_default_manager"
+    )
+    if related_manager_info is None:
+        return ctx.default_return_type
+    return Instance(related_manager_info, [])

mypy_django_plugin/transformers/models.py

@@ -114,7 +114,7 @@ def get_generated_manager_info(self, manager_fullname: str, base_manager_fullnam
         # Not a generated manager
         return None
 
-    def get_or_create_manager_with_any_fallback(self, related_manager: bool = False) -> Optional[TypeInfo]:
+    def get_or_create_manager_with_any_fallback(self) -> Optional[TypeInfo]:
         """
         Create a Manager subclass with fallback to Any for unknown attributes
         and methods. This is used for unresolved managers, where we don't know
@@ -123,7 +123,7 @@ def get_or_create_manager_with_any_fallback(self, related_manager: bool = False)
         The created class is reused if multiple unknown managers are encountered.
         """
 
-        name = "UnknownManager" if not related_manager else "UnknownRelatedManager"
+        name = "UnknownManager"
 
         # Check if we've already created a fallback manager class for this
         # module, and if so reuse that.
@@ -134,9 +134,7 @@ def get_or_create_manager_with_any_fallback(self, related_manager: bool = False)
         fallback_queryset = self.get_or_create_queryset_with_any_fallback()
         if fallback_queryset is None:
             return None
-        base_manager_fullname = (
-            fullnames.MANAGER_CLASS_FULLNAME if not related_manager else fullnames.RELATED_MANAGER_CLASS
-        )
+        base_manager_fullname = fullnames.MANAGER_CLASS_FULLNAME
         base_manager_info = self.lookup_typeinfo(base_manager_fullname)
         if base_manager_info is None:
             return None
@@ -455,6 +453,10 @@ class AddReverseLookups(ModelClassInitializer):
     def reverse_one_to_one_descriptor(self) -> TypeInfo:
         return self.lookup_typeinfo_or_incomplete_defn_error(fullnames.REVERSE_ONE_TO_ONE_DESCRIPTOR)
 
+    @cached_property
+    def reverse_many_to_one_descriptor(self) -> TypeInfo:
+        return self.lookup_typeinfo_or_incomplete_defn_error(fullnames.REVERSE_MANY_TO_ONE_DESCRIPTOR)
+
     @cached_property
     def many_to_many_descriptor(self) -> TypeInfo:
         return self.lookup_typeinfo_or_incomplete_defn_error(fullnames.MANY_TO_MANY_DESCRIPTOR)
@@ -466,23 +468,21 @@ def process_relation(self, relation: ForeignObjectRel) -> None:
             # explicitly declared(i.e. non-inferred) reverse accessors alone
             return
 
-        related_model_cls = self.django_context.get_field_related_model_cls(relation)
-        related_model_info = self.lookup_class_typeinfo_or_incomplete_defn_error(related_model_cls)
+        to_model_cls = self.django_context.get_field_related_model_cls(relation)
+        to_model_info = self.lookup_class_typeinfo_or_incomplete_defn_error(to_model_cls)
 
         if isinstance(relation, OneToOneRel):
             self.add_new_node_to_model_class(
                 attname,
                 Instance(
                     self.reverse_one_to_one_descriptor,
-                    [Instance(self.model_classdef.info, []), Instance(related_model_info, [])],
+                    [Instance(self.model_classdef.info, []), Instance(to_model_info, [])],
                 ),
             )
             return
 
         elif isinstance(relation, ManyToManyRel):
             # TODO: 'relation' should be based on `TypeInfo` instead of Django runtime.
-            to_fullname = helpers.get_class_fullname(relation.remote_field.model)
-            to_model_info = self.lookup_typeinfo_or_incomplete_defn_error(to_fullname)
             assert relation.through is not None
             through_fullname = helpers.get_class_fullname(relation.through)
             through_model_info = self.lookup_typeinfo_or_incomplete_defn_error(through_fullname)
@@ -492,79 +492,65 @@ def process_relation(self, relation: ForeignObjectRel) -> None:
             )
             return
 
-        related_manager_info = None
-        try:
-            related_manager_info = self.lookup_typeinfo_or_incomplete_defn_error(fullnames.RELATED_MANAGER_CLASS)
-            default_manager = related_model_info.names.get("_default_manager")
-            if not default_manager:
-                raise helpers.IncompleteDefnException()
-        except helpers.IncompleteDefnException as exc:
-            if not self.api.final_iteration:
-                raise exc
-
-            # If a django model has a Manager class that cannot be
-            # resolved statically (if it is generated in a way where we
-            # cannot import it, like `objects = my_manager_factory()`),
-            # we fallback to the default related manager, so you at
-            # least get a base level of working type checking.
-            #
-            # See https://github.com/typeddjango/django-stubs/pull/993
-            # for more information on when this error can occur.
-            fallback_manager = self.get_or_create_manager_with_any_fallback(related_manager=True)
-            if fallback_manager is not None:
-                self.add_new_node_to_model_class(
-                    attname, Instance(fallback_manager, [Instance(related_model_info, [])])
-                )
-            related_model_fullname = related_model_cls.__module__ + "." + related_model_cls.__name__
-            self.ctx.api.fail(
-                (
-                    "Couldn't resolve related manager for relation "
-                    f"{relation.name!r} (from {related_model_fullname}."
-                    f"{relation.field})."
-                ),
-                self.ctx.cls,
-                code=MANAGER_MISSING,
-            )
-            return
-
-        # Check if the related model has a related manager subclassed from the default manager
+        related_manager_info = self.lookup_typeinfo_or_incomplete_defn_error(fullnames.RELATED_MANAGER_CLASS)
         # TODO: Support other reverse managers than `_default_manager`
-        default_reverse_manager_info = helpers.get_reverse_manager_info(
-            self.api, model_info=related_model_info, derived_from="_default_manager"
-        )
-        if default_reverse_manager_info:
-            self.add_new_node_to_model_class(attname, Instance(default_reverse_manager_info, []))
-            return
+        default_manager = to_model_info.names.get("_default_manager")
+        if default_manager is None and not self.api.final_iteration:
+            raise helpers.IncompleteDefnException()
+        elif (
+            # When we get no default manager we can't customize the reverse manager any
+            # further and will just fall back to the manager declared on the descriptor
+            default_manager is None
+            # '_default_manager' attribute is a node type we can't process
+            or not isinstance(default_manager.type, Instance)
+            # Already has a related manager subclassed from the default manager
+            or helpers.get_reverse_manager_info(self.api, model_info=to_model_info, derived_from="_default_manager")
+            is not None
+            # When the default manager isn't custom there's no need to create a new type
+            # as `RelatedManager` has `models.Manager` as base
+            or default_manager.type.type.fullname == fullnames.MANAGER_CLASS_FULLNAME
+        ):
+            if default_manager is None and self.api.final_iteration:
+                # If a django model has a Manager class that cannot be
+                # resolved statically (if it is generated in a way where we
+                # cannot import it, like `objects = my_manager_factory()`),
+                #
+                # See https://github.com/typeddjango/django-stubs/pull/993
+                # for more information on when this error can occur.
+                self.ctx.api.fail(
+                    (
+                        f"Couldn't resolve related manager {attname!r} for relation "
+                        f"'{to_model_info.fullname}.{relation.field.name}'."
+                    ),
+                    self.ctx.cls,
+                    code=MANAGER_MISSING,
+                )
 
-        # The reverse manager we're looking for doesn't exist. So we
-        # create it. The (default) reverse manager type is built from a
-        # RelatedManager and the default manager on the related model
-        parametrized_related_manager_type = Instance(related_manager_info, [Instance(related_model_info, [])])
-        default_manager_type = default_manager.type
-        assert default_manager_type is not None
-        assert isinstance(default_manager_type, Instance)
-        # When the default manager isn't custom there's no need to create a new type
-        # as `RelatedManager` has `models.Manager` as base
-        if default_manager_type.type.fullname == fullnames.MANAGER_CLASS_FULLNAME:
-            self.add_new_node_to_model_class(attname, parametrized_related_manager_type)
+            self.add_new_node_to_model_class(
+                attname, Instance(self.reverse_many_to_one_descriptor, [Instance(to_model_info, [])])
+            )
             return
 
+        # Create a reverse manager subclassed from the default manager of this model
+        # and 'RelatedManager'
+        related_manager = Instance(related_manager_info, [Instance(to_model_info, [])])
         # The reverse manager is based on the related model's manager, so it makes most sense to add the new
         # related manager in that module
         new_related_manager_info = helpers.add_new_class_for_module(
-            module=self.api.modules[related_model_info.module_name],
-            name=f"{related_model_cls.__name__}_RelatedManager",
-            bases=[parametrized_related_manager_type, default_manager_type],
+            module=self.api.modules[to_model_info.module_name],
+            name=f"{to_model_info.name}_RelatedManager",
+            bases=[related_manager, default_manager.type],
         )
-        new_related_manager_info.metadata["django"] = {"related_manager_to_model": related_model_info.fullname}
         # Stash the new reverse manager type fullname on the related model, so we don't duplicate
         # or have to create it again for other reverse relations
         helpers.set_reverse_manager_info(
-            related_model_info,
+            to_model_info,
             derived_from="_default_manager",
             fullname=new_related_manager_info.fullname,
         )
-        self.add_new_node_to_model_class(attname, Instance(new_related_manager_info, []))
+        self.add_new_node_to_model_class(
+            attname, Instance(self.reverse_many_to_one_descriptor, [Instance(to_model_info, [])])
+        )
 
     def run_with_model_cls(self, model_cls: Type[Model]) -> None:
         # add related managers etc.

tests/typecheck/managers/test_managers.yml

@@ -530,15 +530,15 @@
                     reveal_type([booking for booking in Booking.objects.all().filter()])
 
 
-                    # Check QuerySet methods on UnknownRelatedManager
+                    # Check QuerySet methods for unresolvable manager
                     reveal_type(user.booking_set.all)
                     reveal_type(user.booking_set.custom)
                     reveal_type(user.booking_set.all().filter)
                     reveal_type(user.booking_set.all().custom)
                     reveal_type(user.booking_set.all().first())
     out: |
-        myapp/models:13: error: Couldn't resolve related manager for relation 'booking' (from myapp.models.Booking.myapp.Booking.renter).  [django-manager-missing]
-        myapp/models:13: error: Couldn't resolve related manager for relation 'bookingowner_set' (from myapp.models.Booking.myapp.Booking.owner).  [django-manager-missing]
+        myapp/models:13: error: Couldn't resolve related manager 'booking_set' for relation 'myapp.models.Booking.renter'.  [django-manager-missing]
+        myapp/models:13: error: Couldn't resolve related manager 'bookingowner_set' for relation 'myapp.models.Booking.owner'.  [django-manager-missing]
         myapp/models:20: error: Could not resolve manager type for "myapp.models.Booking.objects"  [django-manager-missing]
         myapp/models:23: error: Could not resolve manager type for "myapp.models.TwoUnresolvable.objects"  [django-manager-missing]
         myapp/models:24: error: Could not resolve manager type for "myapp.models.TwoUnresolvable.second_objects"  [django-manager-missing]
@@ -553,8 +553,8 @@
         myapp/models:44: note: Revealed type is "django.db.models.manager.Manager[myapp.models.TwoUnresolvable]"
         myapp/models:46: note: Revealed type is "myapp.models.UnknownManager[myapp.models.InvisibleUnresolvable]"
         myapp/models:47: note: Revealed type is "django.db.models.manager.Manager[myapp.models.InvisibleUnresolvable]"
-        myapp/models:49: note: Revealed type is "myapp.models.UnknownRelatedManager[myapp.models.Booking]"
-        myapp/models:50: note: Revealed type is "myapp.models.UnknownRelatedManager[myapp.models.Booking]"
+        myapp/models:49: note: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Booking]"
+        myapp/models:50: note: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Booking]"
         myapp/models:53: note: Revealed type is "def () -> myapp.models.UnknownQuerySet[myapp.models.Booking, myapp.models.Booking]"
         myapp/models:54: note: Revealed type is "Any"
         myapp/models:55: note: Revealed type is "def (*args: Any, **kwargs: Any) -> myapp.models.UnknownQuerySet[myapp.models.Booking, myapp.models.Booking]"
@@ -563,9 +563,11 @@
         myapp/models:58: note: Revealed type is "myapp.models.Booking"
         myapp/models:59: note: Revealed type is "builtins.list[myapp.models.Booking]"
         myapp/models:60: note: Revealed type is "builtins.list[myapp.models.Booking]"
-        myapp/models:64: note: Revealed type is "def () -> myapp.models.UnknownQuerySet[myapp.models.Booking, myapp.models.Booking]"
+        myapp/models:64: note: Revealed type is "def () -> django.db.models.query.QuerySet[myapp.models.Booking, myapp.models.Booking]"
+        myapp/models:65: error: "RelatedManager[Booking]" has no attribute "custom"  [attr-defined]
         myapp/models:65: note: Revealed type is "Any"
-        myapp/models:66: note: Revealed type is "def (*args: Any, **kwargs: Any) -> myapp.models.UnknownQuerySet[myapp.models.Booking, myapp.models.Booking]"
+        myapp/models:66: note: Revealed type is "def (*args: Any, **kwargs: Any) -> django.db.models.query.QuerySet[myapp.models.Booking, myapp.models.Booking]"
+        myapp/models:67: error: "QuerySet[Booking, Booking]" has no attribute "custom"  [attr-defined]
         myapp/models:67: note: Revealed type is "Any"
         myapp/models:68: note: Revealed type is "Union[myapp.models.Booking, None]"
 

tests/typecheck/models/test_contrib_models.yml

@@ -160,13 +160,14 @@
               class Other(models.Model):
                   user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE)
 
--   case: test_permissions_inherited_reverese_relations
+-   case: test_permissions_inherited_reverse_relations
     main: |
-        from django.contrib.auth.models import Permission
+        from django.contrib.auth.models import Group, Permission
+        from django.contrib.contenttypes.models import ContentType
         reveal_type(Permission().user_set)  # N: Revealed type is "django.contrib.auth.models.User_ManyRelatedManager[django.contrib.auth.models.User_user_permissions]"
-
-        from django.contrib.auth.models import Group
         reveal_type(Group().user_set)  # N: Revealed type is "django.contrib.auth.models.User_ManyRelatedManager[django.contrib.auth.models.User_groups]"
+        reveal_type(ContentType.permission_set)  # N: Revealed type is "django.db.models.fields.related_descriptors.ReverseManyToOneDescriptor[django.contrib.auth.models.Permission]"
+        reveal_type(ContentType().permission_set)  # N: Revealed type is "django.contrib.auth.models.Permission_RelatedManager"
     custom_settings: |
         INSTALLED_APPS = ('django.contrib.contenttypes', 'django.contrib.auth')
         AUTH_USER_MODEL='auth.User'

tests/typecheck/models/test_related_fields.yml

@@ -94,7 +94,7 @@
 
       reveal_type(Model2.model_1.field) # N: Revealed type is "django.db.models.fields.related.ForeignObject[app1.models.Model1, app1.models.Model1]"
       reveal_type(Model2().model_1) # N: Revealed type is "app1.models.Model1"
-      reveal_type(Model1.model_2s) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[app1.models.Model2]"
+      reveal_type(Model1.model_2s) # N: Revealed type is "django.db.models.fields.related_descriptors.ReverseManyToOneDescriptor[app1.models.Model2]"
       reveal_type(Model1().model_2s) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[app1.models.Model2]"
 
     installed_apps: