Don't remove objects attribute from Model in plugin

Partially reverts typeddjango#1672

Author: flaeppe

django-stubs/db/models/base.pyi

@@ -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]]

mypy_django_plugin/transformers/models.py

@@ -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 = helpers.fill_manager(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)
@@ -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 = helpers.fill_manager(manager_info, self.model_classdef.info.self_type)
-            self.add_new_node_to_model_class(manager_name, manager_type, is_classvar=True)
+            # 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)
+            if manager_fullname == manager_info.fullname and manager_node and isinstance(manager_node.node, Var):
+                manager_node.node.type = manager_type
+            else:
+                self.add_new_node_to_model_class(manager_name, manager_type, is_classvar=True)
 
         if incomplete_manager_defs:
             if not self.api.final_iteration:
@@ -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
                     manager_type = helpers.fill_manager(fallback_manager_info, self.model_classdef.info.self_type)
                     self.add_new_node_to_model_class(manager_name, manager_type, is_classvar=True)
@@ -958,12 +983,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():

tests/typecheck/fields/test_related.yml

@@ -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()
@@ -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:
@@ -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: |
@@ -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')
@@ -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
@@ -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')