Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't remove objects attribute from Model in plugin #2280

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 = 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)
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 = 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:
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
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)
Expand Down Expand Up @@ -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():
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
Loading