From 07503e6acd63733c5224bf158d3af42d27eef1f0 Mon Sep 17 00:00:00 2001 From: Petter Friberg Date: Fri, 5 Jul 2024 22:56:53 +0200 Subject: [PATCH] Remove some unnecessary defer rounds Change so that we don't trigger unnecessary defer rounds during manager creation via the `from_queryset` method call. --- mypy_django_plugin/transformers/managers.py | 55 +++++++++++-------- mypy_django_plugin/transformers/models.py | 15 ++--- tests/typecheck/fields/test_related.yml | 4 +- .../managers/querysets/test_from_queryset.yml | 7 +++ tests/typecheck/managers/test_managers.yml | 29 +++++----- 5 files changed, 61 insertions(+), 49 deletions(-) diff --git a/mypy_django_plugin/transformers/managers.py b/mypy_django_plugin/transformers/managers.py index e4be536a2..8155f98bd 100644 --- a/mypy_django_plugin/transformers/managers.py +++ b/mypy_django_plugin/transformers/managers.py @@ -315,8 +315,9 @@ def create_new_manager_class_from_from_queryset_method(ctx: DynamicClassDefConte # This is just a deferral run where our work is already finished return - new_manager_info = create_manager_info_from_from_queryset_call(semanal_api, ctx.call, ctx.name) - if new_manager_info is None: + try: + new_manager_info = create_manager_info_from_from_queryset_call(semanal_api, ctx.call, ctx.name) + except helpers.IncompleteDefnException: if not ctx.api.final_iteration: # XXX: hack for python/mypy#17402 ph = PlaceholderNode(ctx.api.qualified_name(ctx.name), ctx.call, ctx.call.line, becomes_typeinfo=True) @@ -324,8 +325,10 @@ def create_new_manager_class_from_from_queryset_method(ctx: DynamicClassDefConte ctx.api.defer() return - # So that the plugin will reparameterize the manager when it is constructed inside of a Model definition - helpers.add_new_manager_base(semanal_api, new_manager_info.fullname) + if new_manager_info is not None: + # So that the plugin will reparameterize the manager when it is constructed + # inside of a Model definition + helpers.add_new_manager_base(semanal_api, new_manager_info.fullname) def register_dynamically_created_manager(fullname: str, manager_name: str, manager_base: TypeInfo) -> None: @@ -345,27 +348,35 @@ def create_manager_info_from_from_queryset_call( """ if ( - # Check that this is a from_queryset call on a manager subclass + # Check that this is a from_queryset call not isinstance(call_expr.callee, MemberExpr) or not isinstance(call_expr.callee.expr, RefExpr) - or not isinstance(call_expr.callee.expr.node, TypeInfo) - or not call_expr.callee.expr.node.has_base(fullnames.BASE_MANAGER_CLASS_FULLNAME) or not call_expr.callee.name == "from_queryset" - # Check that the call has one or two arguments and that the first is a - # QuerySet subclass + # Check that the call has one or two arguments or not 1 <= len(call_expr.args) <= 2 or not isinstance(call_expr.args[0], RefExpr) - or not isinstance(call_expr.args[0].node, TypeInfo) - or not call_expr.args[0].node.has_base(fullnames.QUERYSET_CLASS_FULLNAME) ): return None base_manager_info, queryset_info = call_expr.callee.expr.node, call_expr.args[0].node - if queryset_info.fullname is None: + if ( + # Handle potentially forwarded types + not isinstance(base_manager_info, TypeInfo) + or not isinstance(queryset_info, TypeInfo) # In some cases, due to the way the semantic analyzer works, only # passed_queryset.name is available. But it should be analyzed again, # so this isn't a problem. - return None # type: ignore[unreachable] + or queryset_info.fullname is None + ): + raise helpers.IncompleteDefnException + elif ( + # Check that the from_queryset call is on a manager subclass and that a first + # argument is a QuerySet subclass. Otherwise we've encountered a function call + # that is not relevant for us + not base_manager_info.has_base(fullnames.BASE_MANAGER_CLASS_FULLNAME) + or not queryset_info.has_base(fullnames.QUERYSET_CLASS_FULLNAME) + ): + return None if len(call_expr.args) == 2 and isinstance(call_expr.args[1], StrExpr): manager_name = call_expr.args[1].value @@ -384,17 +395,13 @@ def create_manager_info_from_from_queryset_call( new_manager_info = manager_sym.node else: # Create a new `TypeInfo` instance for the manager type - try: - new_manager_info = create_manager_class( - api=api, - base_manager_info=base_manager_info, - name=manager_name, - line=call_expr.line, - with_unique_name=name is not None and name != manager_name, - ) - except helpers.IncompleteDefnException: - return None - + new_manager_info = create_manager_class( + api=api, + base_manager_info=base_manager_info, + name=manager_name, + line=call_expr.line, + with_unique_name=name is not None and name != manager_name, + ) populate_manager_from_queryset(new_manager_info, queryset_info) register_dynamically_created_manager( fullname=new_manager_info.fullname, diff --git a/mypy_django_plugin/transformers/models.py b/mypy_django_plugin/transformers/models.py index 0888fff58..fe0e3982e 100644 --- a/mypy_django_plugin/transformers/models.py +++ b/mypy_django_plugin/transformers/models.py @@ -338,14 +338,15 @@ def run_with_model_cls(self, model_cls: Type[Model]) -> None: if manager_info is None: # We couldn't find a manager type, see if we should create one - manager_info = self.create_manager_from_from_queryset(manager_name) - - if manager_info is None: - incomplete_manager_defs.add(manager_name) - continue + try: + manager_info = self.create_manager_from_from_queryset(manager_name) + except helpers.IncompleteDefnException: + incomplete_manager_defs.add(manager_name) + continue - manager_type = Instance(manager_info, [Instance(self.model_classdef.info, [])]) - self.add_new_node_to_model_class(manager_name, manager_type, is_classvar=True) + if manager_info is not None: + manager_type = Instance(manager_info, [Instance(self.model_classdef.info, [])]) + self.add_new_node_to_model_class(manager_name, manager_type, is_classvar=True) if incomplete_manager_defs: if not self.api.final_iteration: diff --git a/tests/typecheck/fields/test_related.yml b/tests/typecheck/fields/test_related.yml index 843bf3a93..3fe7f4e9e 100644 --- a/tests/typecheck/fields/test_related.yml +++ b/tests/typecheck/fields/test_related.yml @@ -748,9 +748,9 @@ class TransactionLog(models.Model): transaction = models.ForeignKey(Transaction, on_delete=models.CASCADE) out: | - myapp/models:11: error: Could not resolve manager type for "myapp.models.Transaction.objects" [django-manager-missing] myapp/models:13: note: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.TransactionLog]" - myapp/models:15: note: Revealed type is "myapp.models.UnknownManager[myapp.models.Transaction]" + myapp/models:15: note: Revealed type is "django.db.models.manager.BaseManager[Any]" + myapp/models:16: error: "BaseManager[Any]" has no attribute "custom" [attr-defined] myapp/models:16: note: Revealed type is "Any" diff --git a/tests/typecheck/managers/querysets/test_from_queryset.yml b/tests/typecheck/managers/querysets/test_from_queryset.yml index cb1da33b6..f3cdb6545 100644 --- a/tests/typecheck/managers/querysets/test_from_queryset.yml +++ b/tests/typecheck/managers/querysets/test_from_queryset.yml @@ -906,3 +906,10 @@ main:12: error: Argument 1 to "from_queryset" of "BaseManager" has incompatible type ""; expected "Type[QuerySet[Model, Model]]" [arg-type] main:17: note: Revealed type is "Type[django.db.models.manager.Manager[django.db.models.base.Model]]" main:17: error: Argument 1 to "from_queryset" of "BaseManager" has incompatible type "Type[NonQSGeneric[Any]]"; expected "Type[QuerySet[Model, Model]]" [arg-type] + +- case: test_from_queryset_handles_passed_a_non_queryset + main: | + from typing import Any + from django.db import models + class Invalid: ... + manager = models.Manager[Any].from_queryset(Invalid) # E: Argument 1 to "from_queryset" of "BaseManager" has incompatible type "Type[Invalid]"; expected "Type[QuerySet[Any, Any]]" [arg-type] diff --git a/tests/typecheck/managers/test_managers.yml b/tests/typecheck/managers/test_managers.yml index 1769c0cdf..5928d3f34 100644 --- a/tests/typecheck/managers/test_managers.yml +++ b/tests/typecheck/managers/test_managers.yml @@ -519,7 +519,7 @@ reveal_type(user.bookingowner_set) reveal_type(user.booking_set) - # Check QuerySet methods on UnknownManager + # Check QuerySet methods on unknown manager reveal_type(Booking.objects.all) reveal_type(Booking.objects.custom) reveal_type(Booking.objects.all().filter) @@ -539,30 +539,27 @@ out: | 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] - myapp/models:27: error: Could not resolve manager type for "myapp.models.AbstractUnresolvable.objects" [django-manager-missing] - myapp/models:32: error: Could not resolve manager type for "myapp.models.InvisibleUnresolvable.objects" [django-manager-missing] myapp/models:36: note: Revealed type is "django.db.models.manager.Manager[myapp.models.User]" myapp/models:37: note: Revealed type is "django.db.models.manager.Manager[myapp.models.User]" - myapp/models:39: note: Revealed type is "myapp.models.UnknownManager[myapp.models.Booking]" + myapp/models:39: note: Revealed type is "django.db.models.manager.Manager[Any]" myapp/models:40: note: Revealed type is "django.db.models.manager.Manager[myapp.models.Booking]" - myapp/models:42: note: Revealed type is "myapp.models.UnknownManager[myapp.models.TwoUnresolvable]" - myapp/models:43: note: Revealed type is "myapp.models.UnknownManager[myapp.models.TwoUnresolvable]" + myapp/models:42: note: Revealed type is "django.db.models.manager.Manager[Any]" + myapp/models:43: note: Revealed type is "django.db.models.manager.Manager[Any]" 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:46: note: Revealed type is "django.db.models.manager.Manager[Any]" myapp/models:47: note: Revealed type is "django.db.models.manager.Manager[myapp.models.InvisibleUnresolvable]" 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:53: note: Revealed type is "def () -> django.db.models.query.QuerySet[Any, Any]" + myapp/models:54: error: "Manager[Any]" has no attribute "custom" [attr-defined] 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]" + myapp/models:55: note: Revealed type is "def (*args: Any, **kwargs: Any) -> django.db.models.query.QuerySet[Any, Any]" + myapp/models:56: error: "QuerySet[Any, Any]" has no attribute "custom" [attr-defined] myapp/models:56: note: Revealed type is "Any" - myapp/models:57: note: Revealed type is "Union[myapp.models.Booking, None]" - 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:57: note: Revealed type is "Union[Any, None]" + myapp/models:58: note: Revealed type is "Any" + myapp/models:59: note: Revealed type is "builtins.list[Any]" + myapp/models:60: note: Revealed type is "builtins.list[Any]" 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"