From 998e2ef542c8469f52c6b9497a88f09f7abd1c18 Mon Sep 17 00:00:00 2001 From: Petter Friberg Date: Thu, 16 Jun 2022 21:51:07 +0200 Subject: [PATCH 1/2] Emit error and set fallback type for managers that can't be resolved --- mypy_django_plugin/errorcodes.py | 2 +- mypy_django_plugin/transformers/models.py | 37 ++++++++-- tests/typecheck/fields/test_related.yml | 37 ---------- tests/typecheck/managers/test_managers.yml | 80 ++++++++++++++++++++++ 4 files changed, 113 insertions(+), 43 deletions(-) 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..609512370 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__) @@ -247,7 +247,7 @@ def run_with_model_cls(self, model_cls: Type[Model]) -> None: if manager_info is None: # Manager doesn't appear to be generated. Track that we encountered an # incomplete definition and skip - encountered_incomplete_manager_def = True + incomplete_manager_defs.add(manager_name) continue _, manager_class_name = manager_info.fullname.rsplit(".", maxsplit=1) @@ -275,9 +275,36 @@ 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 + 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"Couldn't resolve manager type for {manager_fullname!r}", + 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..a3f720009 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 diff --git a/tests/typecheck/managers/test_managers.yml b/tests/typecheck/managers/test_managers.yml index 1083451d7..39cd8710a 100644 --- a/tests/typecheck/managers/test_managers.yml +++ b/tests/typecheck/managers/test_managers.yml @@ -442,3 +442,83 @@ 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): + def some_method(self, arg: str) -> None: + return None + + 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:14: error: Couldn't resolve related manager for relation 'booking' (from myapp.models.Booking.myapp.Booking.renter). + myapp/models:14: error: Couldn't resolve related manager for relation 'bookingowner_set' (from myapp.models.Booking.myapp.Booking.owner). + myapp/models:21: error: Couldn't resolve manager type for 'myapp.models.Booking.objects' + myapp/models:24: error: Couldn't resolve manager type for 'myapp.models.TwoUnresolvable.objects' + myapp/models:25: error: Couldn't resolve manager type for 'myapp.models.TwoUnresolvable.second_objects' + myapp/models:28: error: Couldn't resolve manager type for 'myapp.models.AbstractUnresolvable.objects' + myapp/models:33: error: Couldn't resolve manager type for 'myapp.models.InvisibleUnresolvable.objects' + myapp/models:37: note: Revealed type is "django.db.models.manager.Manager[myapp.models.User]" + myapp/models:38: note: Revealed type is "django.db.models.manager.Manager[myapp.models.User]" + myapp/models:40: note: Revealed type is "django.db.models.manager.Manager[myapp.models.Booking]" + myapp/models:41: note: Revealed type is "django.db.models.manager.BaseManager[myapp.models.Booking]" + 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.Manager[myapp.models.TwoUnresolvable]" + myapp/models:45: note: Revealed type is "django.db.models.manager.BaseManager[myapp.models.TwoUnresolvable]" + myapp/models:47: note: Revealed type is "django.db.models.manager.Manager[myapp.models.InvisibleUnresolvable]" + myapp/models:48: note: Revealed type is "django.db.models.manager.BaseManager[myapp.models.InvisibleUnresolvable]" + myapp/models:50: note: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Booking]" + myapp/models:51: note: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Booking]" From 6a9a2e9d15a5dcf235d9956f5debee8823f09309 Mon Sep 17 00:00:00 2001 From: Petter Friberg Date: Thu, 16 Jun 2022 22:30:47 +0200 Subject: [PATCH 2/2] fixup! Emit error and set fallback type for managers that can't be resolved --- mypy_django_plugin/transformers/models.py | 12 ++++---- tests/typecheck/fields/test_related.yml | 4 ++- tests/typecheck/managers/test_managers.yml | 33 +++++++++++----------- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/mypy_django_plugin/transformers/models.py b/mypy_django_plugin/transformers/models.py index 609512370..17a227242 100644 --- a/mypy_django_plugin/transformers/models.py +++ b/mypy_django_plugin/transformers/models.py @@ -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 incomplete_manager_defs.add(manager_name) - continue - _, manager_class_name = manager_info.fullname.rsplit(".", maxsplit=1) + continue if manager_name not in self.model_classdef.info.names: manager_type = Instance(manager_info, [Instance(self.model_classdef.info, [])]) @@ -285,7 +283,9 @@ def run_with_model_cls(self, model_cls: Type[Model]) -> None: # 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 + 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, [])]) ) @@ -301,7 +301,7 @@ def run_with_model_cls(self, model_cls: Type[Model]) -> None: ] manager_fullname = f"{self.model_classdef.fullname}.{manager_name}" self.api.fail( - f"Couldn't resolve manager type for {manager_fullname!r}", + f'Could not resolve manager type for "{manager_fullname}"', manager_expr[0] if manager_expr else self.ctx.cls, code=MANAGER_MISSING, ) diff --git a/tests/typecheck/fields/test_related.yml b/tests/typecheck/fields/test_related.yml index a3f720009..11a8c60da 100644 --- a/tests/typecheck/fields/test_related.yml +++ b/tests/typecheck/fields/test_related.yml @@ -649,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 39cd8710a..99778f82d 100644 --- a/tests/typecheck/managers/test_managers.yml +++ b/tests/typecheck/managers/test_managers.yml @@ -460,8 +460,7 @@ be resolved. """ class InnerManager(models.Manager): - def some_method(self, arg: str) -> None: - return None + ... return InnerManager() @@ -504,21 +503,21 @@ reveal_type(user.bookingowner_set) reveal_type(user.booking_set) out: | - myapp/models:14: error: Couldn't resolve related manager for relation 'booking' (from myapp.models.Booking.myapp.Booking.renter). - myapp/models:14: error: Couldn't resolve related manager for relation 'bookingowner_set' (from myapp.models.Booking.myapp.Booking.owner). - myapp/models:21: error: Couldn't resolve manager type for 'myapp.models.Booking.objects' - myapp/models:24: error: Couldn't resolve manager type for 'myapp.models.TwoUnresolvable.objects' - myapp/models:25: error: Couldn't resolve manager type for 'myapp.models.TwoUnresolvable.second_objects' - myapp/models:28: error: Couldn't resolve manager type for 'myapp.models.AbstractUnresolvable.objects' - myapp/models:33: error: Couldn't resolve manager type for 'myapp.models.InvisibleUnresolvable.objects' + 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:38: note: Revealed type is "django.db.models.manager.Manager[myapp.models.User]" - myapp/models:40: note: Revealed type is "django.db.models.manager.Manager[myapp.models.Booking]" - myapp/models:41: note: Revealed type is "django.db.models.manager.BaseManager[myapp.models.Booking]" + 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.Manager[myapp.models.TwoUnresolvable]" - myapp/models:45: note: Revealed type is "django.db.models.manager.BaseManager[myapp.models.TwoUnresolvable]" - myapp/models:47: note: Revealed type is "django.db.models.manager.Manager[myapp.models.InvisibleUnresolvable]" - myapp/models:48: note: Revealed type is "django.db.models.manager.BaseManager[myapp.models.InvisibleUnresolvable]" + 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]" - myapp/models:51: note: Revealed type is "django.db.models.manager.RelatedManager[myapp.models.Booking]"