Skip to content

Commit

Permalink
Fix type errors on other models' managers when using objects = models…
Browse files Browse the repository at this point in the history
….Manager() in Model. (#34)

* Fix bug where models with a class variable using a manager defined would interfere with other managers.

- Fill in the type argument for that particular instance of the manager, rather than modifying the bases of the Manager type.
- Instantiate a new Instance from determine_proper_manager_type so The code doesn't crash under mypy-mypyc.

* Use helpers.reparametrize_instance per review comment.

* Updated ignored errors in Django test for get_objects_or_404.

- For some reason, `Manager[nothing]` is now removed from expected types.
  However, I think this makes sense anyway, as Manager is a subclass of QuerySet.
  • Loading branch information
syastrov authored and mkurnikov committed Mar 8, 2019
1 parent 050c1b8 commit 86c63d7
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 5 deletions.
13 changes: 10 additions & 3 deletions mypy_django_plugin/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,20 @@ def determine_proper_manager_type(ctx: FunctionContext) -> Type:
if not isinstance(ret, Instance):
return ret

has_manager_base = False
for i, base in enumerate(ret.type.bases):
if base.type.fullname() in {helpers.MANAGER_CLASS_FULLNAME,
helpers.RELATED_MANAGER_CLASS_FULLNAME,
helpers.BASE_MANAGER_CLASS_FULLNAME}:
ret.type.bases[i] = Instance(base.type, [Instance(outer_model_info, [])])
return ret
return ret
has_manager_base = True
break

if has_manager_base:
# Fill in the manager's type argument from the outer model
new_type_args = [Instance(outer_model_info, [])]
return helpers.reparametrize_instance(ret, new_type_args)
else:
return ret


def return_user_model_hook(ctx: FunctionContext) -> Type:
Expand Down
4 changes: 2 additions & 2 deletions scripts/typecheck_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,9 @@
],
'get_object_or_404': [
'Argument 1 to "get_object_or_404" has incompatible type "str"; '
+ 'expected "Union[Type[<nothing>], Manager[<nothing>], QuerySet[<nothing>]]"',
+ 'expected "Union[Type[<nothing>], QuerySet[<nothing>]]"',
'Argument 1 to "get_list_or_404" has incompatible type "List[Type[Article]]"; '
+ 'expected "Union[Type[<nothing>], Manager[<nothing>], QuerySet[<nothing>]]"',
+ 'expected "Union[Type[<nothing>], QuerySet[<nothing>]]"',
'CustomClass'
],
'get_or_create': [
Expand Down
62 changes: 62 additions & 0 deletions test-data/typecheck/managers.test
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,66 @@ class AbstractBase2(models.Model):

class Child(AbstractBase1, AbstractBase2):
pass
[out]

[CASE managers_from_unrelated_models_dont_interfere]

from django.db import models

# Normal scenario where one model has a manager with an annotation of the same type as the model
class UnrelatedModel(models.Model):
objects = models.Manager[UnrelatedModel]()

class MyModel(models.Model):
pass

reveal_type(UnrelatedModel.objects) # E: Revealed type is 'django.db.models.manager.Manager[main.UnrelatedModel]'
reveal_type(UnrelatedModel.objects.first()) # E: Revealed type is 'Union[main.UnrelatedModel*, None]'

reveal_type(MyModel.objects) # E: Revealed type is 'django.db.models.manager.Manager[main.MyModel]'
reveal_type(MyModel.objects.first()) # E: Revealed type is 'Union[main.MyModel*, None]'

# Possible to specify objects without explicit annotation of models.Manager()
class UnrelatedModel2(models.Model):
objects = models.Manager()

class MyModel2(models.Model):
pass

reveal_type(UnrelatedModel2.objects) # E: Revealed type is 'django.db.models.manager.Manager[main.UnrelatedModel2]'
reveal_type(UnrelatedModel2.objects.first()) # E: Revealed type is 'Union[main.UnrelatedModel2*, None]'

reveal_type(MyModel2.objects) # E: Revealed type is 'django.db.models.manager.Manager[main.MyModel2]'
reveal_type(MyModel2.objects.first()) # E: Revealed type is 'Union[main.MyModel2*, None]'


# Inheritance works
class ParentOfMyModel3(models.Model):
objects = models.Manager()

class MyModel3(ParentOfMyModel3):
pass


reveal_type(ParentOfMyModel3.objects) # E: Revealed type is 'django.db.models.manager.Manager[main.ParentOfMyModel3]'
reveal_type(ParentOfMyModel3.objects.first()) # E: Revealed type is 'Union[main.ParentOfMyModel3*, None]'

reveal_type(MyModel3.objects) # E: Revealed type is 'django.db.models.manager.Manager[main.MyModel3]'
reveal_type(MyModel3.objects.first()) # E: Revealed type is 'Union[main.MyModel3*, None]'


# Inheritance works with explicit objects in child
class ParentOfMyModel4(models.Model):
objects = models.Manager()

class MyModel4(ParentOfMyModel4):
objects = models.Manager[MyModel4]()

reveal_type(ParentOfMyModel4.objects) # E: Revealed type is 'django.db.models.manager.Manager[main.ParentOfMyModel4]'
reveal_type(ParentOfMyModel4.objects.first()) # E: Revealed type is 'Union[main.ParentOfMyModel4*, None]'

reveal_type(MyModel4.objects) # E: Revealed type is 'django.db.models.manager.Manager[main.MyModel4]'
reveal_type(MyModel4.objects.first()) # E: Revealed type is 'Union[main.MyModel4*, None]'


[out]

0 comments on commit 86c63d7

Please sign in to comment.