Declare manager class attributes on models as ClassVars (#1672)

* Move ModelBase.objects declaration to Model.objects, for mypy 1.5.0

mypy 1.5.0 was fixed to understand that metaclass attributes take
precedence over attributes in the regular class.  So we need to
declare `objects` in the regular class to allow it to be overridden in
subclasses.

Fixes #1648.

Signed-off-by: Anders Kaseorg <[email protected]>

* Declare manager class attributes on models as `ClassVar`s

Inclusions:

- Adjustments for the plugin to make generated managers `ClassVar`s
- Changes the default 'objects' to 'ClassVar' and controls it via the
  plugin
- Plugin ensures to only add the 'objects' manager to models it exists
  on during runtime

* fixup! Declare manager class attributes on models as `ClassVar`s

* Enforce appropriate keyword only arguments

Co-authored-by: Nikita Sobolev <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* Bump mypy

* Remove a bunch of `-redefinition` lines from `allowlist_todo.txt`

---------

Signed-off-by: Anders Kaseorg <[email protected]>
Co-authored-by: Anders Kaseorg <[email protected]>
Co-authored-by: Nikita Sobolev <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

django-stubs/contrib/admin/models.pyi

@@ -1,4 +1,4 @@
-from typing import Any
+from typing import Any, ClassVar
 from uuid import UUID
 
 from django.db import models
@@ -28,7 +28,7 @@ class LogEntry(models.Model):
     object_repr: models.CharField
     action_flag: models.PositiveSmallIntegerField
     change_message: models.TextField
-    objects: LogEntryManager
+    objects: ClassVar[LogEntryManager]
     def is_addition(self) -> bool: ...
     def is_change(self) -> bool: ...
     def is_deletion(self) -> bool: ...

django-stubs/contrib/auth/models.pyi

@@ -1,5 +1,5 @@
 from collections.abc import Iterable
-from typing import Any, Literal, TypeVar
+from typing import Any, ClassVar, Literal, TypeVar
 
 from django.contrib.auth.base_user import AbstractBaseUser as AbstractBaseUser
 from django.contrib.auth.base_user import BaseUserManager as BaseUserManager
@@ -10,7 +10,7 @@ from django.db.models import QuerySet
 from django.db.models.base import Model
 from django.db.models.manager import EmptyManager
 from django.utils.functional import _StrOrPromise
-from typing_extensions import TypeAlias
+from typing_extensions import Self, TypeAlias
 
 _AnyUser: TypeAlias = Model | AnonymousUser
 
@@ -21,7 +21,7 @@ class PermissionManager(models.Manager[Permission]):
 
 class Permission(models.Model):
     content_type_id: int
-    objects: PermissionManager
+    objects: ClassVar[PermissionManager]
 
     name = models.CharField(max_length=255)
     content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
@@ -32,7 +32,7 @@ class GroupManager(models.Manager[Group]):
     def get_by_natural_key(self, name: str) -> Group: ...
 
 class Group(models.Model):
-    objects: GroupManager
+    objects: ClassVar[GroupManager]
 
     name = models.CharField(max_length=150)
     permissions = models.ManyToManyField(Permission)
@@ -81,6 +81,8 @@ class AbstractUser(AbstractBaseUser, PermissionsMixin):
     is_active = models.BooleanField()
     date_joined = models.DateTimeField()
 
+    objects: ClassVar[UserManager[Self]]
+
     EMAIL_FIELD: str
     USERNAME_FIELD: str
 

django-stubs/contrib/contenttypes/models.pyi

@@ -1,4 +1,4 @@
-from typing import Any
+from typing import Any, ClassVar
 
 from django.db import models
 from django.db.models.base import Model
@@ -15,7 +15,7 @@ class ContentType(models.Model):
     id: int
     app_label: models.CharField
     model: models.CharField
-    objects: ContentTypeManager
+    objects: ClassVar[ContentTypeManager]
     @property
     def name(self) -> str: ...
     def model_class(self) -> type[Model] | None: ...

django-stubs/contrib/gis/db/backends/oracle/models.pyi

@@ -1,12 +1,15 @@
-from typing import Any
+from typing import Any, ClassVar
 
 from django.contrib.gis.db import models
 from django.contrib.gis.db.backends.base.models import SpatialRefSysMixin
+from django.db.models.manager import Manager
+from typing_extensions import Self
 
 class OracleGeometryColumns(models.Model):
     table_name: Any
     column_name: Any
     srid: Any
+    objects: ClassVar[Manager[Self]]
 
     class Meta:
         app_label: str
@@ -24,6 +27,7 @@ class OracleSpatialRefSys(models.Model, SpatialRefSysMixin):
     auth_name: Any
     wktext: Any
     cs_bounds: Any
+    objects: ClassVar[Manager[Self]]
 
     class Meta:
         app_label: str

django-stubs/contrib/gis/db/backends/postgis/models.pyi

@@ -1,7 +1,8 @@
-from typing import Any
+from typing import Any, ClassVar
 
 from django.contrib.gis.db.backends.base.models import SpatialRefSysMixin
 from django.db import models
+from typing_extensions import Self
 
 class PostGISGeometryColumns(models.Model):
     f_table_catalog: Any
@@ -11,6 +12,7 @@ class PostGISGeometryColumns(models.Model):
     coord_dimension: Any
     srid: Any
     type: Any
+    objects: ClassVar[models.Manager[Self]]
 
     class Meta:
         app_label: str
@@ -27,6 +29,7 @@ class PostGISSpatialRefSys(models.Model, SpatialRefSysMixin):
     auth_srid: Any
     srtext: Any
     proj4text: Any
+    objects: ClassVar[models.Manager[Self]]
 
     class Meta:
         app_label: str

django-stubs/contrib/gis/db/backends/spatialite/models.pyi

@@ -1,7 +1,8 @@
-from typing import Any
+from typing import Any, ClassVar
 
 from django.contrib.gis.db.backends.base.models import SpatialRefSysMixin
 from django.db import models
+from typing_extensions import Self
 
 class SpatialiteGeometryColumns(models.Model):
     f_table_name: Any
@@ -10,6 +11,7 @@ class SpatialiteGeometryColumns(models.Model):
     srid: Any
     spatial_index_enabled: Any
     type: Any
+    objects: ClassVar[models.Manager[Self]]
 
     class Meta:
         app_label: str
@@ -27,6 +29,7 @@ class SpatialiteSpatialRefSys(models.Model, SpatialRefSysMixin):
     ref_sys_name: Any
     proj4text: Any
     srtext: Any
+    objects: ClassVar[models.Manager[Self]]
 
     class Meta:
         app_label: str

django-stubs/contrib/sites/models.pyi

@@ -1,4 +1,4 @@
-from typing import Any
+from typing import Any, ClassVar
 
 from django.db import models
 from django.http.request import HttpRequest
@@ -11,7 +11,7 @@ class SiteManager(models.Manager[Site]):
     def get_by_natural_key(self, domain: str) -> Site: ...
 
 class Site(models.Model):
-    objects: SiteManager
+    objects: ClassVar[SiteManager]
 
     domain = models.CharField(max_length=100)
     name = models.CharField(max_length=50)

django-stubs/db/migrations/recorder.pyi

@@ -1,14 +1,17 @@
-from typing import Any
+from typing import Any, ClassVar
 
 from django.db.backends.base.base import BaseDatabaseWrapper
 from django.db.models.base import Model
+from django.db.models.manager import Manager
 from django.db.models.query import QuerySet
+from typing_extensions import Self
 
 class MigrationRecorder:
     class Migration(Model):
         app: Any
         name: Any
         applied: Any
+        objects: ClassVar[Manager[Self]]
     connection: BaseDatabaseWrapper
     def __init__(self, connection: BaseDatabaseWrapper) -> None: ...
     @property

django-stubs/db/models/base.pyi

@@ -1,11 +1,11 @@
 from collections.abc import Collection, Iterable, Sequence
-from typing import Any, Final, TypeVar
+from typing import Any, ClassVar, Final, TypeVar
 
 from django.core.checks.messages import CheckMessage
 from django.core.exceptions import MultipleObjectsReturned as BaseMultipleObjectsReturned
 from django.core.exceptions import ObjectDoesNotExist, ValidationError
 from django.db.models import BaseConstraint, Field
-from django.db.models.manager import BaseManager
+from django.db.models.manager import BaseManager, Manager
 from django.db.models.options import Options
 from typing_extensions import Self
 
@@ -19,8 +19,6 @@ class ModelState:
     fields_cache: ModelStateFieldsCacheDescriptor
 
 class ModelBase(type):
-    @property
-    def objects(cls: type[_Self]) -> BaseManager[_Self]: ...  # type: ignore[misc]
     @property
     def _default_manager(cls: type[_Self]) -> BaseManager[_Self]: ...  # type: ignore[misc]
     @property
@@ -34,6 +32,9 @@ 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]]
 
     class Meta: ...
     _meta: Options[Any]

mypy_django_plugin/lib/helpers.py

@@ -379,14 +379,17 @@ def check_types_compatible(
     api.check_subtype(actual_type, expected_type, ctx.context, error_message, "got", "expected")
 
 
-def add_new_sym_for_info(info: TypeInfo, *, name: str, sym_type: MypyType, no_serialize: bool = False) -> None:
+def add_new_sym_for_info(
+    info: TypeInfo, name: str, sym_type: MypyType, *, no_serialize: bool = False, is_classvar: bool = False
+) -> None:
     # type=: type of the variable itself
     var = Var(name=name, type=sym_type)
     # var.info: type of the object variable is bound to
     var.info = info
     var._fullname = info.fullname + "." + name
     var.is_initialized_in_class = True
     var.is_inferred = True
+    var.is_classvar = is_classvar
     info.names[name] = SymbolTableNode(MDEF, var, plugin_generated=True, no_serialize=no_serialize)
 
 

mypy_django_plugin/transformers/models.py

@@ -70,8 +70,13 @@ def create_new_var(self, name: str, typ: MypyType) -> Var:
         var.is_inferred = True
         return var
 
-    def add_new_node_to_model_class(self, name: str, typ: MypyType, no_serialize: bool = False) -> None:
-        helpers.add_new_sym_for_info(self.model_classdef.info, name=name, sym_type=typ, no_serialize=no_serialize)
+    def add_new_node_to_model_class(
+        self, name: str, typ: MypyType, *, no_serialize: bool = False, is_classvar: bool = False
+    ) -> None:
+        # TODO: Rename to signal that it is a `Var` that is added..
+        helpers.add_new_sym_for_info(
+            self.model_classdef.info, name=name, sym_type=typ, no_serialize=no_serialize, is_classvar=is_classvar
+        )
 
     def add_new_class_for_current_module(self, name: str, bases: List[Instance]) -> TypeInfo:
         current_module = self.api.modules[self.model_classdef.info.module_name]
@@ -311,7 +316,7 @@ 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, [])])
-        self.add_new_node_to_model_class(manager_name, 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]
@@ -336,7 +341,7 @@ def run_with_model_cls(self, model_cls: Type[Model]) -> None:
                 continue
 
             manager_type = Instance(manager_info, [Instance(self.model_classdef.info, [])])
-            self.add_new_node_to_model_class(manager_name, manager_type)
+            self.add_new_node_to_model_class(manager_name, manager_type, is_classvar=True)
 
         if incomplete_manager_defs:
             if not self.api.final_iteration:
@@ -351,7 +356,9 @@ 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()
                 self.add_new_node_to_model_class(
-                    manager_name, Instance(fallback_manager_info, [Instance(self.model_classdef.info, [])])
+                    manager_name,
+                    Instance(fallback_manager_info, [Instance(self.model_classdef.info, [])]),
+                    is_classvar=True,
                 )
 
                 # Find expression for e.g. `objects = SomeManager()`
@@ -433,7 +440,7 @@ def run_with_model_cls(self, model_cls: Type[Model]) -> None:
             default_manager_info = generated_manager_info
 
         default_manager = Instance(default_manager_info, [Instance(self.model_classdef.info, [])])
-        self.add_new_node_to_model_class("_default_manager", default_manager)
+        self.add_new_node_to_model_class("_default_manager", default_manager, is_classvar=True)
 
 
 class AddRelatedManagers(ModelClassInitializer):
@@ -623,6 +630,10 @@ 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]:

requirements.txt

@@ -12,4 +12,4 @@ Django==4.2.5
 -e .[compatible-mypy]
 
 # Overrides:
-mypy==1.4.1
+mypy==1.5.1