Skip to content

Commit

Permalink
Remove some unnecessary defer rounds
Browse files Browse the repository at this point in the history
Change so that we don't trigger unnecessary defer rounds during manager
creation via the `from_queryset` method call.
  • Loading branch information
flaeppe committed Jul 13, 2024
1 parent a16b9ba commit 07503e6
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 49 deletions.
55 changes: 31 additions & 24 deletions mypy_django_plugin/transformers/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,17 +315,20 @@ 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)
ctx.api.add_symbol_table_node(ctx.name, SymbolTableNode(GDEF, ph))
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:
Expand All @@ -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
Expand All @@ -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,
Expand Down
15 changes: 8 additions & 7 deletions mypy_django_plugin/transformers/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions tests/typecheck/fields/test_related.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
7 changes: 7 additions & 0 deletions tests/typecheck/managers/querysets/test_from_queryset.yml
Original file line number Diff line number Diff line change
Expand Up @@ -906,3 +906,10 @@
main:12: error: Argument 1 to "from_queryset" of "BaseManager" has incompatible type "<typing special form>"; 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]
29 changes: 13 additions & 16 deletions tests/typecheck/managers/test_managers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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"
Expand Down

0 comments on commit 07503e6

Please sign in to comment.