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

Make ReverseManyToOneDescriptor generic over a model #2227

Merged
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
24 changes: 12 additions & 12 deletions django-stubs/db/models/fields/related_descriptors.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -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::

Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions ext/django_stubs_ext/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from django.db.models.expressions import ExpressionWrapper
from django.db.models.fields import Field
from django.db.models.fields.related import ForeignKey
from django.db.models.fields.related_descriptors import ReverseManyToOneDescriptor
from django.db.models.lookups import Lookup
from django.db.models.manager import BaseManager
from django.db.models.query import QuerySet
Expand Down Expand Up @@ -71,6 +72,7 @@ def __repr__(self) -> str:
MPGeneric(Lookup),
MPGeneric(BaseConnectionHandler),
MPGeneric(ExpressionWrapper),
MPGeneric(ReverseManyToOneDescriptor),
# These types do have native `__class_getitem__` method since django 3.1:
MPGeneric(QuerySet, (3, 1)),
MPGeneric(BaseManager, (3, 1)),
Expand Down
1 change: 1 addition & 0 deletions mypy_django_plugin/lib/fullnames.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
24 changes: 18 additions & 6 deletions mypy_django_plugin/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()

Expand Down
52 changes: 52 additions & 0 deletions mypy_django_plugin/transformers/manytoone.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
from typing import Optional

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_model_of_related_manager(ctx: MethodContext) -> Optional[Instance]:
"""
Returns the type parameter (_To) instance of a `RelatedManager` instance when it's a
model. Otherwise `None` is returned.

For example: if given a `RelatedManager[A]` where `A` is a model then `A` is
returned.
"""
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.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.
"""
to_model_instance = get_model_of_related_manager(ctx)
if to_model_instance is None:
return ctx.default_return_type

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, [])
124 changes: 55 additions & 69 deletions mypy_django_plugin/transformers/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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 the related
# 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.
Expand Down
4 changes: 0 additions & 4 deletions scripts/stubtest/allowlist_todo.txt
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ django.contrib.auth.models.User.is_staff
django.contrib.auth.models.User.is_superuser
django.contrib.auth.models.User.last_login
django.contrib.auth.models.User.last_name
django.contrib.auth.models.User.logentry_set
django.contrib.auth.models.User.password
django.contrib.auth.models.User.user_permissions
django.contrib.auth.models.User.username
Expand Down Expand Up @@ -169,9 +168,7 @@ django.contrib.contenttypes.management.inject_rename_contenttypes_operations
django.contrib.contenttypes.models.ContentType.app_label
django.contrib.contenttypes.models.ContentType.app_labeled_name
django.contrib.contenttypes.models.ContentType.id
django.contrib.contenttypes.models.ContentType.logentry_set
django.contrib.contenttypes.models.ContentType.model
django.contrib.contenttypes.models.ContentType.permission_set
django.contrib.contenttypes.models.ContentTypeManager.__init__
django.contrib.contenttypes.models.ContentTypeManager.__slotnames__
django.contrib.flatpages.admin.FlatPageAdmin
Expand Down Expand Up @@ -515,7 +512,6 @@ django.contrib.sites.models.Site.domain
django.contrib.sites.models.Site.flatpage_set
django.contrib.sites.models.Site.id
django.contrib.sites.models.Site.name
django.contrib.sites.models.Site.redirect_set
django.contrib.sites.models.SiteManager.__slotnames__
django.contrib.staticfiles.finders.BaseStorageFinder.storage
django.contrib.staticfiles.finders.DefaultStorageFinder.storage
Expand Down
Loading
Loading