From c9742862c37c9d6245631b00ed0dac2e835381fe Mon Sep 17 00:00:00 2001 From: Petter Friberg Date: Fri, 21 Jun 2024 14:46:14 +0200 Subject: [PATCH 1/4] 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. --- .../db/models/fields/related_descriptors.pyi | 24 ++-- mypy_django_plugin/lib/fullnames.py | 1 + mypy_django_plugin/main.py | 24 +++- mypy_django_plugin/transformers/manytoone.py | 56 ++++++++ mypy_django_plugin/transformers/models.py | 124 ++++++++---------- tests/typecheck/managers/test_managers.yml | 16 ++- .../typecheck/models/test_contrib_models.yml | 9 +- .../typecheck/models/test_related_fields.yml | 2 +- 8 files changed, 157 insertions(+), 99 deletions(-) create mode 100644 mypy_django_plugin/transformers/manytoone.py diff --git a/django-stubs/db/models/fields/related_descriptors.pyi b/django-stubs/db/models/fields/related_descriptors.pyi index a6efc796a..8b37d3d31 100644 --- a/django-stubs/db/models/fields/related_descriptors.pyi +++ b/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 diff --git a/mypy_django_plugin/lib/fullnames.py b/mypy_django_plugin/lib/fullnames.py index 29497ed10..8899ccba0 100644 --- a/mypy_django_plugin/lib/fullnames.py +++ b/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( diff --git a/mypy_django_plugin/main.py b/mypy_django_plugin/main.py index 533b3f204..e067d4987 100644 --- a/mypy_django_plugin/main.py +++ b/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() diff --git a/mypy_django_plugin/transformers/manytoone.py b/mypy_django_plugin/transformers/manytoone.py new file mode 100644 index 000000000..4683c4d07 --- /dev/null +++ b/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, []) diff --git a/mypy_django_plugin/transformers/models.py b/mypy_django_plugin/transformers/models.py index 24d1ed197..7641bdf49 100644 --- a/mypy_django_plugin/transformers/models.py +++ b/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. diff --git a/tests/typecheck/managers/test_managers.yml b/tests/typecheck/managers/test_managers.yml index ef534239a..89041f648 100644 --- a/tests/typecheck/managers/test_managers.yml +++ b/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]" diff --git a/tests/typecheck/models/test_contrib_models.yml b/tests/typecheck/models/test_contrib_models.yml index e055640f2..3223059c7 100644 --- a/tests/typecheck/models/test_contrib_models.yml +++ b/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' diff --git a/tests/typecheck/models/test_related_fields.yml b/tests/typecheck/models/test_related_fields.yml index cc00a5729..3f6189f15 100644 --- a/tests/typecheck/models/test_related_fields.yml +++ b/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: From a505004041f3aef6914073236ade04ebdc09611d Mon Sep 17 00:00:00 2001 From: Petter Friberg Date: Fri, 21 Jun 2024 14:55:10 +0200 Subject: [PATCH 2/4] fixup! Make `ReverseManyToOneDescriptor` generic over a model --- scripts/stubtest/allowlist_todo.txt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/scripts/stubtest/allowlist_todo.txt b/scripts/stubtest/allowlist_todo.txt index bdd2a3590..ab131e284 100644 --- a/scripts/stubtest/allowlist_todo.txt +++ b/scripts/stubtest/allowlist_todo.txt @@ -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 @@ -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 @@ -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 From 394bec1e81a661cc580a61e4df1aa48cef5eee00 Mon Sep 17 00:00:00 2001 From: Petter Friberg Date: Fri, 21 Jun 2024 15:15:46 +0200 Subject: [PATCH 3/4] fixup! fixup! Make `ReverseManyToOneDescriptor` generic over a model --- ext/django_stubs_ext/patch.py | 2 ++ mypy_django_plugin/transformers/manytoone.py | 24 ++++++++------------ 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/ext/django_stubs_ext/patch.py b/ext/django_stubs_ext/patch.py index 7bd131552..795726abb 100644 --- a/ext/django_stubs_ext/patch.py +++ b/ext/django_stubs_ext/patch.py @@ -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 @@ -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)), diff --git a/mypy_django_plugin/transformers/manytoone.py b/mypy_django_plugin/transformers/manytoone.py index 4683c4d07..88735d8bb 100644 --- a/mypy_django_plugin/transformers/manytoone.py +++ b/mypy_django_plugin/transformers/manytoone.py @@ -1,4 +1,4 @@ -from typing import Optional, Tuple +from typing import Optional from mypy.plugin import MethodContext from mypy.types import Instance @@ -7,16 +7,13 @@ from mypy_django_plugin.lib import fullnames, helpers -def get_related_manager_and_model(ctx: MethodContext) -> Optional[Tuple[Instance, Instance]]: +def get_model_of_related_manager(ctx: MethodContext) -> Optional[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)`. + 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) @@ -32,7 +29,7 @@ def get_related_manager_and_model(ctx: MethodContext) -> Optional[Tuple[Instance 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 related_manager.args[0] return None @@ -42,11 +39,10 @@ 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: + to_model_instance = get_model_of_related_manager(ctx) + if to_model_instance 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" From 62ece2ef6f2430b6e09bfcc9a416942d990c7bfb Mon Sep 17 00:00:00 2001 From: Petter Friberg Date: Fri, 21 Jun 2024 15:25:44 +0200 Subject: [PATCH 4/4] fixup! fixup! fixup! Make `ReverseManyToOneDescriptor` generic over a model --- mypy_django_plugin/transformers/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy_django_plugin/transformers/models.py b/mypy_django_plugin/transformers/models.py index 7641bdf49..f99923c40 100644 --- a/mypy_django_plugin/transformers/models.py +++ b/mypy_django_plugin/transformers/models.py @@ -531,8 +531,8 @@ def process_relation(self, relation: ForeignObjectRel) -> None: ) return - # Create a reverse manager subclassed from the default manager of this model - # and 'RelatedManager' + # 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