Skip to content

Commit

Permalink
fixup! Don't remove objects attribute from Model in plugin
Browse files Browse the repository at this point in the history
  • Loading branch information
flaeppe committed Aug 2, 2024
1 parent a7f78fd commit c7a4ae4
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 33 deletions.
3 changes: 2 additions & 1 deletion django-stubs/contrib/sites/managers.pyi
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
from typing import TypeVar

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

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

class CurrentSiteManager(models.Manager[_T]):
def __init__(self, field_name: str | None = ...) -> None: ...
28 changes: 14 additions & 14 deletions mypy_django_plugin/transformers/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,23 +352,23 @@ def cast_var_to_classvar(symbol: Optional[SymbolTableNode]) -> None:

assert self.model_classdef.info.self_type is not None
manager_type = Instance(manager_info, [self.model_classdef.info.self_type])
# 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:
# 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:
Expand Down
28 changes: 12 additions & 16 deletions tests/typecheck/managers/test_managers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -169,29 +169,28 @@
main: |
from myapp.models import AbstractPerson, Book
reveal_type(AbstractPerson.abstract_persons) # N: Revealed type is "django.db.models.manager.Manager[myapp.models.AbstractPerson]"
reveal_type(Book.published_objects) # N: Revealed type is "myapp.models.PublishedBookManager"
reveal_type(Book.published_objects) # N: Revealed type is "myapp.models.PublishedBookManager[myapp.models.Book]"
Book.published_objects.create(title='hello')
reveal_type(Book.annotated_objects) # N: Revealed type is "myapp.models.AnnotatedBookManager"
reveal_type(Book.annotated_objects) # N: Revealed type is "myapp.models.AnnotatedBookManager[myapp.models.Book]"
Book.annotated_objects.create(title='hello')
installed_apps:
- myapp
files:
- path: myapp/__init__.py
- path: myapp/models.py
content: |
from typing import ClassVar
from django.db import models
class AbstractPerson(models.Model):
abstract_persons: ClassVar[models.Manager["AbstractPerson"]] = models.Manager['AbstractPerson']()
abstract_persons = models.Manager['AbstractPerson']()
class PublishedBookManager(models.Manager['Book']):
pass
class AnnotatedBookManager(models.Manager['Book']):
pass
class Book(models.Model):
title = models.CharField(max_length=50)
published_objects: ClassVar[PublishedBookManager] = PublishedBookManager()
annotated_objects: ClassVar[AnnotatedBookManager] = AnnotatedBookManager()
published_objects = PublishedBookManager()
annotated_objects = AnnotatedBookManager()
- case: managers_inherited_from_abstract_classes_multiple_inheritance
main: |
Expand All @@ -201,32 +200,31 @@
reveal_type(AbstractBase1.manager1)
reveal_type(AbstractBase2.restricted)
out: |
main:2: note: Revealed type is "myapp.models.CustomManager1"
main:3: note: Revealed type is "myapp.models.CustomManager2"
main:4: note: Revealed type is "myapp.models.CustomManager1"
main:5: note: Revealed type is "myapp.models.CustomManager2"
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:
- path: myapp/__init__.py
- path: myapp/models.py
content: |
from typing import ClassVar
from django.db import models
class CustomManager1(models.Manager['AbstractBase1']):
pass
class AbstractBase1(models.Model):
class Meta:
abstract = True
name = models.CharField(max_length=50)
manager1: ClassVar[CustomManager1] = CustomManager1()
manager1 = CustomManager1()
class CustomManager2(models.Manager['AbstractBase2']):
pass
class AbstractBase2(models.Model):
class Meta:
abstract = True
value = models.CharField(max_length=50)
restricted: ClassVar[CustomManager2] = CustomManager2()
restricted = CustomManager2()
class Child(AbstractBase1, AbstractBase2):
pass
Expand Down Expand Up @@ -453,15 +451,13 @@
- path: myapp/__init__.py
- path: myapp/models.py
content: |
from typing import ClassVar
from typing_extensions import Self
from django.db import models
from django.contrib.sites.models import Site
from django.contrib.sites.managers import CurrentSiteManager
class MyModel(models.Model):
site = models.ForeignKey(Site, on_delete=models.CASCADE)
on_site: ClassVar[CurrentSiteManager[Self]] = CurrentSiteManager()
on_site = CurrentSiteManager()
- case: test_emits_error_for_unresolved_managers
main: |
Expand Down
4 changes: 2 additions & 2 deletions tests/typecheck/models/test_contrib_models.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
- case: can_override_abstract_user_manager
main: |
from myapp.models import MyBaseUser, MyUser
reveal_type(MyBaseUser.objects) # N: Revealed type is "myapp.models.MyBaseUserManager"
reveal_type(MyBaseUser.objects) # N: Revealed type is "myapp.models.MyBaseUserManager[myapp.models.MyBaseUser]"
reveal_type(MyBaseUser.objects.all()) # N: Revealed type is "django.db.models.query.QuerySet[myapp.models.MyBaseUser, myapp.models.MyBaseUser]"
reveal_type(MyUser.objects) # N: Revealed type is "myapp.models.MyUserManager"
reveal_type(MyUser.objects.all()) # N: Revealed type is "django.db.models.query.QuerySet[myapp.models.MyUser, myapp.models.MyUser]"
Expand All @@ -56,7 +56,7 @@
...
class MyBaseUser(AbstractBaseUser):
objects: ClassVar[MyBaseUserManager] = MyBaseUserManager()
objects = MyBaseUserManager()
class MyUserManager(UserManager["MyUser"]):
...
Expand Down

0 comments on commit c7a4ae4

Please sign in to comment.