Skip to content

Commit

Permalink
Don't remove objects attribute from Model in plugin
Browse files Browse the repository at this point in the history
Partially reverts typeddjango#1672
  • Loading branch information
flaeppe committed Aug 1, 2024
1 parent b76f9f4 commit dbe26ae
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 85 deletions.
3 changes: 1 addition & 2 deletions django-stubs/contrib/sites/managers.pyi
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
from typing import TypeVar

from django.contrib.sites.models import Site
from django.db import models

_T = TypeVar("_T", bound=Site)
_T = TypeVar("_T", bound=models.Model)

class CurrentSiteManager(models.Manager[_T]):
def __init__(self, field_name: str | None = ...) -> None: ...
3 changes: 1 addition & 2 deletions django-stubs/db/models/base.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ class Model(metaclass=ModelBase):
# and re-add them to correct concrete subclasses of 'Model'
DoesNotExist: Final[type[ObjectDoesNotExist]]
MultipleObjectsReturned: Final[type[BaseMultipleObjectsReturned]]
# This 'objects' attribute will be deleted, via the plugin, in favor of managing it
# to only exist on subclasses it exists on during runtime.

objects: ClassVar[Manager[Self]]

_meta: ClassVar[Options[Self]]
Expand Down
33 changes: 26 additions & 7 deletions mypy_django_plugin/transformers/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,11 +319,18 @@ def reparametrize_dynamically_created_manager(self, manager_name: str, manager_i
assert manager_info is not None
# Reparameterize dynamically created manager with model type
manager_type = Instance(manager_info, [Instance(self.model_classdef.info, [])])
manager_node = self.model_classdef.info.get(manager_name)
if manager_node and isinstance(manager_node.node, Var):
manager_node.node.type = manager_type
self.add_new_node_to_model_class(manager_name, manager_type, is_classvar=True)

def run_with_model_cls(self, model_cls: Type[Model]) -> None:
manager_info: Optional[TypeInfo]

def cast_var_to_classvar(symbol: Optional[SymbolTableNode]) -> None:
if symbol and isinstance(symbol.node, Var):
symbol.node.is_classvar = True

incomplete_manager_defs = set()
for manager_name, manager in model_cls._meta.managers_map.items():
manager_node = self.model_classdef.info.get(manager_name)
Expand All @@ -345,7 +352,24 @@ def run_with_model_cls(self, model_cls: Type[Model]) -> None:

assert self.model_classdef.info.self_type is not None
manager_type = Instance(manager_info, [self.model_classdef.info.self_type])
self.add_new_node_to_model_class(manager_name, manager_type, is_classvar=True)
if manager_fullname == manager_info.fullname and manager_node and isinstance(manager_node.node, Var):
manager_node.node.type = manager_type
else:
# It seems that the type checker fetches a Var from expressions, but
# looks through the symbol table for the type(at some later stage?).
# Currently we don't overwrite the reference mypy holds from an
# expression to a Var instance when adding a new node, we only overwrite
# the reference to the Var in the symbol table. That means there's a
# lingering Var instance attached to expressions and if we don't flip
# that to a ClassVar, the checker will emit an error for overriding a
# class variable with an instance variable. As mypy seems to check that
# via the expression and not the symbol table. Optimally we want to just
# set a type on the existing Var like
# manager_node.node.type = manager_type
# but for some reason that doesn't work. It only works replacing the
# existing Var with a new one in the symbol table.
cast_var_to_classvar(manager_node)
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 All @@ -360,6 +384,7 @@ def run_with_model_cls(self, model_cls: Type[Model]) -> None:
# setting _some_ type
fallback_manager_info = self.get_or_create_manager_with_any_fallback()
if fallback_manager_info is not None:
cast_var_to_classvar(self.model_classdef.info.get(manager_name))
assert self.model_classdef.info.self_type is not None
self.add_new_node_to_model_class(
manager_name,
Expand Down Expand Up @@ -961,12 +986,6 @@ def adjust_model_class(cls, ctx: ClassDefContext) -> None:
):
del ctx.cls.info.names["MultipleObjectsReturned"]

objects = ctx.cls.info.names.get("objects")
if objects is not None and isinstance(objects.node, Var) and not objects.plugin_generated:
del ctx.cls.info.names["objects"]

return

def get_exception_bases(self, name: str) -> List[Instance]:
bases = []
for model_base in self.model_classdef.info.direct_base_classes():
Expand Down
34 changes: 14 additions & 20 deletions tests/typecheck/fields/test_related.yml
Original file line number Diff line number Diff line change
Expand Up @@ -727,10 +727,9 @@
- path: myapp/models.py
content: |
from django.db import models
from django.db.models.manager import BaseManager
class TransactionQuerySet(models.QuerySet):
pass
TransactionManager = BaseManager.from_queryset(TransactionQuerySet)
TransactionManager = models.Manager.from_queryset(TransactionQuerySet)
class Transaction(models.Model):
pk = 0
objects = TransactionManager()
Expand All @@ -742,7 +741,7 @@
Transaction().test()
- case: foreign_key_relationship_for_models_with_custom_manager_unsolvable
- case: foreign_key_relationship_for_models_with_custom_manager_solvable_via_as_manager_type
main: |
from myapp.models import Transaction
installed_apps:
Expand All @@ -752,30 +751,27 @@
- path: myapp/models.py
content: |
from django.db import models
from django.db.models.manager import BaseManager
class TransactionQuerySet(models.QuerySet):
def custom(self) -> None:
pass
def TransactionManager() -> BaseManager:
return BaseManager.from_queryset(TransactionQuerySet)()
def TransactionManager() -> models.Manager:
return models.Manager.from_queryset(TransactionQuerySet)()
class Transaction(models.Model):
objects = TransactionManager()
def test(self) -> None:
reveal_type(self.transactionlog_set)
# We use a fallback Any type:
reveal_type(Transaction.objects)
reveal_type(Transaction.objects.custom())
reveal_type(self.transactionlog_set) # N: Revealed type is "django.db.models.fields.related_descriptors.RelatedManager[myapp.models.TransactionLog]"
# We get a lucky shot here as long as the plugin predeclares a
# manager for `.as_manager` for every base class of QuerySet.
# It's just lucky that the runtime's manager name is the same
# name as for the predeclared manager. Resolving this wouldn't
# be possible without inspection of the runtime
reveal_type(Transaction.objects) # N: Revealed type is "myapp.models.ManagerFromTransactionQuerySet[myapp.models.Transaction]"
reveal_type(Transaction.objects.custom()) # N: Revealed type is "None"
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:16: note: Revealed type is "Any"
- case: resolve_primary_keys_for_foreign_keys_with_abstract_self_model
main: |
Expand Down Expand Up @@ -913,12 +909,11 @@
- path: myapp/models/purchase.py
content: |
from django.db import models
from django.db.models.manager import BaseManager
from .querysets import PurchaseQuerySet
from .store import Store
from .user import User
PurchaseManager = BaseManager.from_queryset(PurchaseQuerySet)
PurchaseManager = models.Manager.from_queryset(PurchaseQuerySet)
class Purchase(models.Model):
objects = PurchaseManager()
store = models.ForeignKey(to=Store, on_delete=models.CASCADE, related_name='purchases')
Expand All @@ -936,7 +931,6 @@
- path: myapp/models.py
content: |
from django.db import models
from django.db.models.manager import BaseManager
class User(models.Model):
purchases: int
Expand All @@ -945,7 +939,7 @@
def queryset_method(self) -> "PurchaseQuerySet":
return self.all()
PurchaseManager = BaseManager.from_queryset(PurchaseQuerySet)
PurchaseManager = models.Manager.from_queryset(PurchaseQuerySet)
class Purchase(models.Model):
objects = PurchaseManager()
user = models.ForeignKey(to=User, on_delete=models.CASCADE, related_name='purchases')
Expand Down
Loading

0 comments on commit dbe26ae

Please sign in to comment.