Skip to content

Commit 07503e6

Browse files
committed
Remove some unnecessary defer rounds
Change so that we don't trigger unnecessary defer rounds during manager creation via the `from_queryset` method call.
1 parent a16b9ba commit 07503e6

File tree

5 files changed

+61
-49
lines changed

5 files changed

+61
-49
lines changed

mypy_django_plugin/transformers/managers.py

+31-24
Original file line numberDiff line numberDiff line change
@@ -315,17 +315,20 @@ def create_new_manager_class_from_from_queryset_method(ctx: DynamicClassDefConte
315315
# This is just a deferral run where our work is already finished
316316
return
317317

318-
new_manager_info = create_manager_info_from_from_queryset_call(semanal_api, ctx.call, ctx.name)
319-
if new_manager_info is None:
318+
try:
319+
new_manager_info = create_manager_info_from_from_queryset_call(semanal_api, ctx.call, ctx.name)
320+
except helpers.IncompleteDefnException:
320321
if not ctx.api.final_iteration:
321322
# XXX: hack for python/mypy#17402
322323
ph = PlaceholderNode(ctx.api.qualified_name(ctx.name), ctx.call, ctx.call.line, becomes_typeinfo=True)
323324
ctx.api.add_symbol_table_node(ctx.name, SymbolTableNode(GDEF, ph))
324325
ctx.api.defer()
325326
return
326327

327-
# So that the plugin will reparameterize the manager when it is constructed inside of a Model definition
328-
helpers.add_new_manager_base(semanal_api, new_manager_info.fullname)
328+
if new_manager_info is not None:
329+
# So that the plugin will reparameterize the manager when it is constructed
330+
# inside of a Model definition
331+
helpers.add_new_manager_base(semanal_api, new_manager_info.fullname)
329332

330333

331334
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(
345348
"""
346349

347350
if (
348-
# Check that this is a from_queryset call on a manager subclass
351+
# Check that this is a from_queryset call
349352
not isinstance(call_expr.callee, MemberExpr)
350353
or not isinstance(call_expr.callee.expr, RefExpr)
351-
or not isinstance(call_expr.callee.expr.node, TypeInfo)
352-
or not call_expr.callee.expr.node.has_base(fullnames.BASE_MANAGER_CLASS_FULLNAME)
353354
or not call_expr.callee.name == "from_queryset"
354-
# Check that the call has one or two arguments and that the first is a
355-
# QuerySet subclass
355+
# Check that the call has one or two arguments
356356
or not 1 <= len(call_expr.args) <= 2
357357
or not isinstance(call_expr.args[0], RefExpr)
358-
or not isinstance(call_expr.args[0].node, TypeInfo)
359-
or not call_expr.args[0].node.has_base(fullnames.QUERYSET_CLASS_FULLNAME)
360358
):
361359
return None
362360

363361
base_manager_info, queryset_info = call_expr.callee.expr.node, call_expr.args[0].node
364-
if queryset_info.fullname is None:
362+
if (
363+
# Handle potentially forwarded types
364+
not isinstance(base_manager_info, TypeInfo)
365+
or not isinstance(queryset_info, TypeInfo)
365366
# In some cases, due to the way the semantic analyzer works, only
366367
# passed_queryset.name is available. But it should be analyzed again,
367368
# so this isn't a problem.
368-
return None # type: ignore[unreachable]
369+
or queryset_info.fullname is None
370+
):
371+
raise helpers.IncompleteDefnException
372+
elif (
373+
# Check that the from_queryset call is on a manager subclass and that a first
374+
# argument is a QuerySet subclass. Otherwise we've encountered a function call
375+
# that is not relevant for us
376+
not base_manager_info.has_base(fullnames.BASE_MANAGER_CLASS_FULLNAME)
377+
or not queryset_info.has_base(fullnames.QUERYSET_CLASS_FULLNAME)
378+
):
379+
return None
369380

370381
if len(call_expr.args) == 2 and isinstance(call_expr.args[1], StrExpr):
371382
manager_name = call_expr.args[1].value
@@ -384,17 +395,13 @@ def create_manager_info_from_from_queryset_call(
384395
new_manager_info = manager_sym.node
385396
else:
386397
# Create a new `TypeInfo` instance for the manager type
387-
try:
388-
new_manager_info = create_manager_class(
389-
api=api,
390-
base_manager_info=base_manager_info,
391-
name=manager_name,
392-
line=call_expr.line,
393-
with_unique_name=name is not None and name != manager_name,
394-
)
395-
except helpers.IncompleteDefnException:
396-
return None
397-
398+
new_manager_info = create_manager_class(
399+
api=api,
400+
base_manager_info=base_manager_info,
401+
name=manager_name,
402+
line=call_expr.line,
403+
with_unique_name=name is not None and name != manager_name,
404+
)
398405
populate_manager_from_queryset(new_manager_info, queryset_info)
399406
register_dynamically_created_manager(
400407
fullname=new_manager_info.fullname,

mypy_django_plugin/transformers/models.py

+8-7
Original file line numberDiff line numberDiff line change
@@ -338,14 +338,15 @@ def run_with_model_cls(self, model_cls: Type[Model]) -> None:
338338

339339
if manager_info is None:
340340
# We couldn't find a manager type, see if we should create one
341-
manager_info = self.create_manager_from_from_queryset(manager_name)
342-
343-
if manager_info is None:
344-
incomplete_manager_defs.add(manager_name)
345-
continue
341+
try:
342+
manager_info = self.create_manager_from_from_queryset(manager_name)
343+
except helpers.IncompleteDefnException:
344+
incomplete_manager_defs.add(manager_name)
345+
continue
346346

347-
manager_type = Instance(manager_info, [Instance(self.model_classdef.info, [])])
348-
self.add_new_node_to_model_class(manager_name, manager_type, is_classvar=True)
347+
if manager_info is not None:
348+
manager_type = Instance(manager_info, [Instance(self.model_classdef.info, [])])
349+
self.add_new_node_to_model_class(manager_name, manager_type, is_classvar=True)
349350

350351
if incomplete_manager_defs:
351352
if not self.api.final_iteration:

tests/typecheck/fields/test_related.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -748,9 +748,9 @@
748748
class TransactionLog(models.Model):
749749
transaction = models.ForeignKey(Transaction, on_delete=models.CASCADE)
750750
out: |
751-
myapp/models:11: error: Could not resolve manager type for "myapp.models.Transaction.objects" [django-manager-missing]
752751
myapp/models:13: note: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.TransactionLog]"
753-
myapp/models:15: note: Revealed type is "myapp.models.UnknownManager[myapp.models.Transaction]"
752+
myapp/models:15: note: Revealed type is "django.db.models.manager.BaseManager[Any]"
753+
myapp/models:16: error: "BaseManager[Any]" has no attribute "custom" [attr-defined]
754754
myapp/models:16: note: Revealed type is "Any"
755755
756756

tests/typecheck/managers/querysets/test_from_queryset.yml

+7
Original file line numberDiff line numberDiff line change
@@ -906,3 +906,10 @@
906906
main:12: error: Argument 1 to "from_queryset" of "BaseManager" has incompatible type "<typing special form>"; expected "Type[QuerySet[Model, Model]]" [arg-type]
907907
main:17: note: Revealed type is "Type[django.db.models.manager.Manager[django.db.models.base.Model]]"
908908
main:17: error: Argument 1 to "from_queryset" of "BaseManager" has incompatible type "Type[NonQSGeneric[Any]]"; expected "Type[QuerySet[Model, Model]]" [arg-type]
909+
910+
- case: test_from_queryset_handles_passed_a_non_queryset
911+
main: |
912+
from typing import Any
913+
from django.db import models
914+
class Invalid: ...
915+
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]

tests/typecheck/managers/test_managers.yml

+13-16
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@
519519
reveal_type(user.bookingowner_set)
520520
reveal_type(user.booking_set)
521521
522-
# Check QuerySet methods on UnknownManager
522+
# Check QuerySet methods on unknown manager
523523
reveal_type(Booking.objects.all)
524524
reveal_type(Booking.objects.custom)
525525
reveal_type(Booking.objects.all().filter)
@@ -539,30 +539,27 @@
539539
out: |
540540
myapp/models:13: error: Couldn't resolve related manager 'booking_set' for relation 'myapp.models.Booking.renter'. [django-manager-missing]
541541
myapp/models:13: error: Couldn't resolve related manager 'bookingowner_set' for relation 'myapp.models.Booking.owner'. [django-manager-missing]
542-
myapp/models:20: error: Could not resolve manager type for "myapp.models.Booking.objects" [django-manager-missing]
543-
myapp/models:23: error: Could not resolve manager type for "myapp.models.TwoUnresolvable.objects" [django-manager-missing]
544-
myapp/models:24: error: Could not resolve manager type for "myapp.models.TwoUnresolvable.second_objects" [django-manager-missing]
545-
myapp/models:27: error: Could not resolve manager type for "myapp.models.AbstractUnresolvable.objects" [django-manager-missing]
546-
myapp/models:32: error: Could not resolve manager type for "myapp.models.InvisibleUnresolvable.objects" [django-manager-missing]
547542
myapp/models:36: note: Revealed type is "django.db.models.manager.Manager[myapp.models.User]"
548543
myapp/models:37: note: Revealed type is "django.db.models.manager.Manager[myapp.models.User]"
549-
myapp/models:39: note: Revealed type is "myapp.models.UnknownManager[myapp.models.Booking]"
544+
myapp/models:39: note: Revealed type is "django.db.models.manager.Manager[Any]"
550545
myapp/models:40: note: Revealed type is "django.db.models.manager.Manager[myapp.models.Booking]"
551-
myapp/models:42: note: Revealed type is "myapp.models.UnknownManager[myapp.models.TwoUnresolvable]"
552-
myapp/models:43: note: Revealed type is "myapp.models.UnknownManager[myapp.models.TwoUnresolvable]"
546+
myapp/models:42: note: Revealed type is "django.db.models.manager.Manager[Any]"
547+
myapp/models:43: note: Revealed type is "django.db.models.manager.Manager[Any]"
553548
myapp/models:44: note: Revealed type is "django.db.models.manager.Manager[myapp.models.TwoUnresolvable]"
554-
myapp/models:46: note: Revealed type is "myapp.models.UnknownManager[myapp.models.InvisibleUnresolvable]"
549+
myapp/models:46: note: Revealed type is "django.db.models.manager.Manager[Any]"
555550
myapp/models:47: note: Revealed type is "django.db.models.manager.Manager[myapp.models.InvisibleUnresolvable]"
556551
myapp/models:49: note: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Booking]"
557552
myapp/models:50: note: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.Booking]"
558-
myapp/models:53: note: Revealed type is "def () -> myapp.models.UnknownQuerySet[myapp.models.Booking, myapp.models.Booking]"
553+
myapp/models:53: note: Revealed type is "def () -> django.db.models.query.QuerySet[Any, Any]"
554+
myapp/models:54: error: "Manager[Any]" has no attribute "custom" [attr-defined]
559555
myapp/models:54: note: Revealed type is "Any"
560-
myapp/models:55: note: Revealed type is "def (*args: Any, **kwargs: Any) -> myapp.models.UnknownQuerySet[myapp.models.Booking, myapp.models.Booking]"
556+
myapp/models:55: note: Revealed type is "def (*args: Any, **kwargs: Any) -> django.db.models.query.QuerySet[Any, Any]"
557+
myapp/models:56: error: "QuerySet[Any, Any]" has no attribute "custom" [attr-defined]
561558
myapp/models:56: note: Revealed type is "Any"
562-
myapp/models:57: note: Revealed type is "Union[myapp.models.Booking, None]"
563-
myapp/models:58: note: Revealed type is "myapp.models.Booking"
564-
myapp/models:59: note: Revealed type is "builtins.list[myapp.models.Booking]"
565-
myapp/models:60: note: Revealed type is "builtins.list[myapp.models.Booking]"
559+
myapp/models:57: note: Revealed type is "Union[Any, None]"
560+
myapp/models:58: note: Revealed type is "Any"
561+
myapp/models:59: note: Revealed type is "builtins.list[Any]"
562+
myapp/models:60: note: Revealed type is "builtins.list[Any]"
566563
myapp/models:64: note: Revealed type is "def () -> django.db.models.query.QuerySet[myapp.models.Booking, myapp.models.Booking]"
567564
myapp/models:65: error: "RelatedManager[Booking]" has no attribute "custom" [attr-defined]
568565
myapp/models:65: note: Revealed type is "Any"

0 commit comments

Comments
 (0)