diff --git a/mypy_django_plugin/errorcodes.py b/mypy_django_plugin/errorcodes.py index df77dfe45..76af6dbaa 100644 --- a/mypy_django_plugin/errorcodes.py +++ b/mypy_django_plugin/errorcodes.py @@ -1,4 +1,4 @@ from mypy.errorcodes import ErrorCode MANAGER_UNTYPED = ErrorCode("django-manager", "Untyped manager disallowed", "Django") -MANAGER_MISSING = ErrorCode("django-manager-missing", "Couldn't resolve related manager for model", "Django") +MANAGER_MISSING = ErrorCode("django-manager-missing", "Couldn't resolve manager for model", "Django") diff --git a/mypy_django_plugin/transformers/models.py b/mypy_django_plugin/transformers/models.py index 9b8fff204..17a227242 100644 --- a/mypy_django_plugin/transformers/models.py +++ b/mypy_django_plugin/transformers/models.py @@ -5,7 +5,7 @@ from django.db.models.fields.related import ForeignKey from django.db.models.fields.reverse_related import ManyToManyRel, ManyToOneRel, OneToOneRel from mypy.checker import TypeChecker -from mypy.nodes import ARG_STAR2, Argument, Context, FuncDef, TypeInfo, Var +from mypy.nodes import ARG_STAR2, Argument, AssignmentStmt, Context, FuncDef, NameExpr, TypeInfo, Var from mypy.plugin import AnalyzeTypeContext, AttributeContext, CheckerPluginInterface, ClassDefContext from mypy.plugins import common from mypy.semanal import SemanticAnalyzer @@ -234,7 +234,7 @@ def create_new_model_parametrized_manager(self, name: str, base_manager_info: Ty def run_with_model_cls(self, model_cls: Type[Model]) -> None: manager_info: Optional[TypeInfo] - encountered_incomplete_manager_def = False + incomplete_manager_defs = set() for manager_name, manager in model_cls._meta.managers_map.items(): manager_class_name = manager.__class__.__name__ manager_fullname = helpers.get_class_fullname(manager.__class__) @@ -243,13 +243,11 @@ def run_with_model_cls(self, model_cls: Type[Model]) -> None: except helpers.IncompleteDefnException as exc: # Check if manager is a generated (dynamic class) manager base_manager_fullname = helpers.get_class_fullname(manager.__class__.__bases__[0]) - manager_info = self.get_generated_manager_info(manager_fullname, base_manager_fullname) - if manager_info is None: + if manager_fullname not in self.get_generated_manager_mappings(base_manager_fullname): # Manager doesn't appear to be generated. Track that we encountered an # incomplete definition and skip - encountered_incomplete_manager_def = True - continue - _, manager_class_name = manager_info.fullname.rsplit(".", maxsplit=1) + incomplete_manager_defs.add(manager_name) + continue if manager_name not in self.model_classdef.info.names: manager_type = Instance(manager_info, [Instance(self.model_classdef.info, [])]) @@ -275,9 +273,38 @@ def run_with_model_cls(self, model_cls: Type[Model]) -> None: self.add_new_node_to_model_class(manager_name, custom_manager_type) - if encountered_incomplete_manager_def and not self.api.final_iteration: - # Unless we're on the final round, see if another round could figuring out all manager types + if incomplete_manager_defs and not self.api.final_iteration: + # Unless we're on the final round, see if another round could figure out all manager types raise helpers.IncompleteDefnException() + elif self.api.final_iteration: + for manager_name in incomplete_manager_defs: + # We act graceful and set the type as the bare minimum we know of + # (Django's default) before finishing. And emit an error, to allow for + # ignoring a more specialised manager not being resolved while still + # setting _some_ type + django_manager_info = self.lookup_typeinfo(fullnames.MANAGER_CLASS_FULLNAME) + assert ( + django_manager_info is not None + ), f"Type info for Django's {fullnames.MANAGER_CLASS_FULLNAME} missing" + self.add_new_node_to_model_class( + manager_name, Instance(django_manager_info, [Instance(self.model_classdef.info, [])]) + ) + # Find expression for e.g. `objects = SomeManager()` + manager_expr = [ + expr + for expr in self.ctx.cls.defs.body + if ( + isinstance(expr, AssignmentStmt) + and isinstance(expr.lvalues[0], NameExpr) + and expr.lvalues[0].name == manager_name + ) + ] + manager_fullname = f"{self.model_classdef.fullname}.{manager_name}" + self.api.fail( + f'Could not resolve manager type for "{manager_fullname}"', + manager_expr[0] if manager_expr else self.ctx.cls, + code=MANAGER_MISSING, + ) class AddDefaultManagerAttribute(ModelClassInitializer): diff --git a/tests/typecheck/fields/test_related.yml b/tests/typecheck/fields/test_related.yml index 675ade94f..11a8c60da 100644 --- a/tests/typecheck/fields/test_related.yml +++ b/tests/typecheck/fields/test_related.yml @@ -639,43 +639,6 @@ class Article(LibraryEntity): pass - -- case: test_related_managers_when_manager_is_dynamically_generated_and_cannot_be_imported - main: | - from myapp import models - installed_apps: - - myapp - files: - - path: myapp/__init__.py - - path: myapp/models.py - content: | - from django.db import models - - class User(models.Model): - name = models.TextField() - - def DynamicManager() -> models.Manager: - class InnerManager(models.Manager): - def some_method(self, arg: str) -> None: - return None - - return InnerManager() - - class Booking(models.Model): - renter = models.ForeignKey(User, on_delete=models.PROTECT) - owner = models.ForeignKey(User, on_delete=models.PROTECT, related_name='bookingowner_set') - - objects = DynamicManager() - - def process_booking(user: User): - reveal_type(user.bookingowner_set) - reveal_type(user.booking_set) - out: | - myapp/models:3: error: Couldn't resolve related manager for relation 'booking' (from myapp.models.Booking.myapp.Booking.renter). - myapp/models:3: error: Couldn't resolve related manager for relation 'bookingowner_set' (from myapp.models.Booking.myapp.Booking.owner). - myapp/models:20: note: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Booking]" - myapp/models:21: note: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Booking]" - - case: foreign_key_relationship_for_models_with_custom_manager main: | from myapp.models import Transaction @@ -686,10 +649,12 @@ - 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) class Transaction(models.Model): - objects = TransactionQuerySet.as_manager() + objects = TransactionManager() def test(self) -> None: self.transactionlog_set class TransactionLog(models.Model): diff --git a/tests/typecheck/managers/test_managers.yml b/tests/typecheck/managers/test_managers.yml index 1083451d7..99778f82d 100644 --- a/tests/typecheck/managers/test_managers.yml +++ b/tests/typecheck/managers/test_managers.yml @@ -442,3 +442,82 @@ class MyModel(models.Model): site = models.ForeignKey(Site, on_delete=models.CASCADE) on_site = CurrentSiteManager() + +- case: test_emits_error_for_unresolved_managers + main: | + from myapp import models + installed_apps: + - myapp + files: + - path: myapp/__init__.py + - path: myapp/models.py + content: | + from django.db import models + + def LocalManager() -> models.Manager: + """ + Returns a manager instance of an inlined manager type that can't + be resolved. + """ + class InnerManager(models.Manager): + ... + + return InnerManager() + + class User(models.Model): + name = models.TextField() + + class Booking(models.Model): + renter = models.ForeignKey(User, on_delete=models.PROTECT) + owner = models.ForeignKey(User, on_delete=models.PROTECT, related_name='bookingowner_set') + + objects = LocalManager() + + class TwoUnresolvable(models.Model): + objects = LocalManager() + second_objects = LocalManager() + + class AbstractUnresolvable(models.Model): + objects = LocalManager() + + class Meta: + abstract = True + + class InvisibleUnresolvable(AbstractUnresolvable): + text = models.TextField() + + def process_booking(user: User): + reveal_type(User.objects) + reveal_type(User._default_manager) + + reveal_type(Booking.objects) + reveal_type(Booking._default_manager) + + reveal_type(TwoUnresolvable.objects) + reveal_type(TwoUnresolvable.second_objects) + reveal_type(TwoUnresolvable._default_manager) + + reveal_type(InvisibleUnresolvable.objects) + reveal_type(InvisibleUnresolvable._default_manager) + + reveal_type(user.bookingowner_set) + reveal_type(user.booking_set) + out: | + myapp/models:13: error: Couldn't resolve related manager for relation 'booking' (from myapp.models.Booking.myapp.Booking.renter). + myapp/models:13: error: Couldn't resolve related manager for relation 'bookingowner_set' (from myapp.models.Booking.myapp.Booking.owner). + myapp/models:20: error: Could not resolve manager type for "myapp.models.Booking.objects" + myapp/models:23: error: Could not resolve manager type for "myapp.models.TwoUnresolvable.objects" + myapp/models:24: error: Could not resolve manager type for "myapp.models.TwoUnresolvable.second_objects" + myapp/models:27: error: Could not resolve manager type for "myapp.models.AbstractUnresolvable.objects" + myapp/models:32: error: Could not resolve manager type for "myapp.models.InvisibleUnresolvable.objects" + myapp/models:36: note: Revealed type is "django.db.models.manager.Manager[myapp.models.User]" + myapp/models:37: note: Revealed type is "django.db.models.manager.Manager[myapp.models.User]" + myapp/models:39: note: Revealed type is "django.db.models.manager.Manager[myapp.models.Booking]" + myapp/models:40: note: Revealed type is "django.db.models.manager.BaseManager[myapp.models.Booking]" + myapp/models:42: note: Revealed type is "django.db.models.manager.Manager[myapp.models.TwoUnresolvable]" + myapp/models:43: note: Revealed type is "django.db.models.manager.Manager[myapp.models.TwoUnresolvable]" + myapp/models:44: note: Revealed type is "django.db.models.manager.BaseManager[myapp.models.TwoUnresolvable]" + myapp/models:46: note: Revealed type is "django.db.models.manager.Manager[myapp.models.InvisibleUnresolvable]" + myapp/models:47: note: Revealed type is "django.db.models.manager.BaseManager[myapp.models.InvisibleUnresolvable]" + myapp/models:49: note: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Booking]" + myapp/models:50: note: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Booking]"