Skip to content

Commit

Permalink
Declare manager class attributes on models as ClassVars
Browse files Browse the repository at this point in the history
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
  • Loading branch information
flaeppe committed Sep 4, 2023
1 parent fc3ef75 commit 662f4ee
Show file tree
Hide file tree
Showing 15 changed files with 90 additions and 41 deletions.
4 changes: 2 additions & 2 deletions django-stubs/contrib/admin/models.pyi
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Any
from typing import Any, ClassVar
from uuid import UUID

from django.db import models
Expand Down Expand Up @@ -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: ...
Expand Down
10 changes: 6 additions & 4 deletions django-stubs/contrib/auth/models.pyi
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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

Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions django-stubs/contrib/contenttypes/models.pyi
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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: ...
Expand Down
6 changes: 5 additions & 1 deletion django-stubs/contrib/gis/db/backends/oracle/models.pyi
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion django-stubs/contrib/gis/db/backends/postgis/models.pyi
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion django-stubs/contrib/gis/db/backends/spatialite/models.pyi
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions django-stubs/contrib/sites/models.pyi
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Any
from typing import Any, ClassVar

from django.db import models
from django.http.request import HttpRequest
Expand All @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion django-stubs/db/migrations/recorder.pyi
Original file line number Diff line number Diff line change
@@ -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
Expand Down
9 changes: 5 additions & 4 deletions django-stubs/db/models/base.pyi
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -25,15 +25,16 @@ class ModelBase(type):
def _base_manager(cls: type[_Self]) -> BaseManager[_Self]: ... # type: ignore[misc]

class Model(metaclass=ModelBase):
objects: BaseManager[Self]

# Note: these two metaclass generated attributes don't really exist on the 'Model'
# class, runtime they are only added on concrete subclasses of 'Model'. The
# metaclass also sets up correct inheritance from concrete parent models exceptions.
# Our mypy plugin aligns with this behaviour and will remove the 2 attributes below
# 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]
Expand Down
5 changes: 4 additions & 1 deletion mypy_django_plugin/lib/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
21 changes: 16 additions & 5 deletions mypy_django_plugin/transformers/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand All @@ -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:
Expand All @@ -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()`
Expand Down Expand Up @@ -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]:
Expand Down
24 changes: 17 additions & 7 deletions tests/typecheck/managers/test_managers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,16 @@
- case: managers_inherited_from_abstract_classes_multiple_inheritance
main: |
from myapp.models import Child
from myapp.models import AbstractBase1, AbstractBase2, Child
reveal_type(Child.manager1)
reveal_type(Child.restricted)
reveal_type(AbstractBase1.manager1)
reveal_type(AbstractBase2.restricted)
out: |
main:2: note: Revealed type is "myapp.models.CustomManager1[myapp.models.Child]"
main:3: note: Revealed type is "myapp.models.CustomManager2[myapp.models.Child]"
main:4: note: Revealed type is "myapp.models.CustomManager1[myapp.models.AbstractBase1]"
main:5: note: Revealed type is "myapp.models.CustomManager2[myapp.models.AbstractBase2]"
installed_apps:
- myapp
files:
Expand Down Expand Up @@ -297,12 +306,13 @@
- path: myapp/__init__.py
- path: myapp/models.py
content: |
from typing import ClassVar
from django.db import models
class ParentOfMyModel4(models.Model):
objects = models.Manager()
class MyModel4(ParentOfMyModel4):
objects = models.Manager['MyModel4']()
objects: ClassVar[models.Manager["MyModel4"]] = models.Manager["MyModel4"]()
# TODO: make it work someday
#- case: inheritance_of_two_models_with_custom_objects_manager
Expand Down Expand Up @@ -570,7 +580,7 @@
- path: myapp/__init__.py
- path: myapp/models.py
content: |
from typing import TypeVar
from typing import ClassVar, TypeVar
from django.db import models
T = TypeVar("T", bound="MyModel")
Expand All @@ -585,7 +595,7 @@
pass
class MySubModel(MyModel):
objects = MySubManager()
objects: ClassVar[MySubManager["MySubModel"]] = MySubManager()
- case: subclass_manager_without_type_parameters_disallow_any_generics
main: |
Expand All @@ -598,14 +608,14 @@
[mypy-myapp.models]
disallow_any_generics = true
out: |
main:2: note: Revealed type is "myapp.models.MySubManager[myapp.models.MySubModel]"
main:2: note: Revealed type is "myapp.models.MySubManager"
main:3: note: Revealed type is "Any"
myapp/models:9: error: Missing type parameters for generic type "MyManager"
files:
- path: myapp/__init__.py
- path: myapp/models.py
content: |
from typing import TypeVar
from typing import ClassVar, TypeVar
from django.db import models
T = TypeVar("T", bound="MyModel")
Expand All @@ -620,7 +630,7 @@
pass
class MySubModel(MyModel):
objects = MySubManager()
objects: ClassVar[MySubManager] = MySubManager()
- case: nested_manager_class_definition
main: |
Expand Down
5 changes: 2 additions & 3 deletions tests/typecheck/models/test_abstract.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@
Recursive(parent=Recursive(parent=None))
Concrete(parent=Concrete(parent=None))
out: |
main:4: error: Access to generic instance variables via class is ambiguous
main:4: error: Unexpected attribute "parent" for model "Recursive"
main:4: error: "Type[Recursive]" has no attribute "objects"
main:5: error: Cannot instantiate abstract class "Recursive" with abstract attributes "DoesNotExist" and "MultipleObjectsReturned"
main:5: error: Unexpected attribute "parent" for model "Recursive"
installed_apps:
Expand Down Expand Up @@ -210,4 +209,4 @@
def create_animal_generic(klass: Type[T], name: str) -> T:
reveal_type(klass) # N: Revealed type is "Type[T`-1]"
return klass.objects.create(name=name) # E: Incompatible return value type (got "Animal", expected "T")
return klass._default_manager.create(name=name)
Loading

0 comments on commit 662f4ee

Please sign in to comment.