Skip to content

Use field generic types for descriptors #2048

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

Merged
merged 4 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
20 changes: 19 additions & 1 deletion mypy_django_plugin/transformers/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@
from django.db.models.fields import AutoField, Field
from django.db.models.fields.related import RelatedField
from django.db.models.fields.reverse_related import ForeignObjectRel
from mypy.maptype import map_instance_to_supertype
from mypy.nodes import AssignmentStmt, NameExpr, TypeInfo
from mypy.plugin import FunctionContext
from mypy.types import AnyType, Instance, ProperType, TypeOfAny, UnionType
from mypy.types import AnyType, Instance, ProperType, TypeOfAny, UninhabitedType, UnionType
from mypy.types import Type as MypyType

from mypy_django_plugin.django.context import DjangoContext
from mypy_django_plugin.exceptions import UnregisteredModelError
from mypy_django_plugin.lib import fullnames, helpers
from mypy_django_plugin.lib.fullnames import FIELD_FULLNAME
from mypy_django_plugin.lib.helpers import parse_bool
from mypy_django_plugin.transformers import manytomany

Expand Down Expand Up @@ -150,6 +152,22 @@ def set_descriptor_types_for_field(
is_set_nullable=is_set_nullable or is_nullable,
is_get_nullable=is_get_nullable or is_nullable,
)

# reconcile set and get types with the base field class
base_field_type = next(base for base in default_return_type.type.mro if base.fullname == FIELD_FULLNAME)
mapped_instance = map_instance_to_supertype(default_return_type, base_field_type)
mapped_set_type, mapped_get_type = mapped_instance.args

# bail if either mapped_set_type or mapped_get_type have type Never
if not (isinstance(mapped_set_type, UninhabitedType) or isinstance(mapped_get_type, UninhabitedType)):
# only replace set_type and get_type with mapped types if their original value is Any
set_type = helpers.convert_any_to_type(
set_type, helpers.make_optional(mapped_set_type) if is_set_nullable or is_nullable else mapped_set_type
)
get_type = helpers.convert_any_to_type(
get_type, helpers.make_optional(mapped_get_type) if is_get_nullable or is_nullable else mapped_get_type
)

return helpers.reparametrize_instance(default_return_type, [set_type, get_type])


Expand Down
62 changes: 62 additions & 0 deletions tests/typecheck/fields/test_custom_fields.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
- case: test_custom_model_fields_with_generic_type
main: |
from myapp.models import User, CustomFieldValue
user = User()
reveal_type(user.id) # N: Revealed type is "builtins.int"
reveal_type(user.my_custom_field1) # N: Revealed type is "myapp.models.CustomFieldValue"
reveal_type(user.my_custom_field2) # N: Revealed type is "myapp.models.CustomFieldValue"
reveal_type(user.my_custom_field3) # N: Revealed type is "builtins.bool"
reveal_type(user.my_custom_field4) # N: Revealed type is "myapp.models.CustomFieldValue"
reveal_type(user.my_custom_field5) # N: Revealed type is "Union[myapp.models.CustomFieldValue, None]"
# user.my_custom_field6 is incorrectly typed as non-optional
# reveal_type(user.my_custom_field6) ## N: Revealed type is "Union[myapp.models.CustomFieldValue, None]"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect on master too and does not pick up on null=True. When debugging, the instance seemed correct and the args were optional so I think this issue is coming from another piece of code somewhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly off topic but still related:

I think #1900 is involved and/or need to be considered regarding null=True and optional.

IMO we should finish explore that approach primarily, before moving on to a plugin approach.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would really like to see #1900 making it as well, although in its current form it doesn't really simplify creating custom field subclasses (and I believe addressing #1900 (comment) would help in this case but requires significant work on mypy)

reveal_type(user.my_custom_field7) # N: Revealed type is "Union[builtins.bool, None]"
reveal_type(user.my_custom_field8) # N: Revealed type is "Union[myapp.models.CustomFieldValue, None]"
reveal_type(user.my_custom_field9) # N: Revealed type is "myapp.models.CustomFieldValue"
reveal_type(user.my_custom_field10) # N: Revealed type is "Union[myapp.models.CustomFieldValue, None]"
# Fields that set _pyi_private_set_type or _pyi_private_get_type retain these types
reveal_type(user.my_custom_field11) # N: Revealed type is "builtins.int"
reveal_type(user.my_custom_field12) # N: Revealed type is "builtins.int"
monkeypatch: true
installed_apps:
- myapp
files:
- path: myapp/__init__.py
- path: myapp/models.py
content: |
from django.db import models
from django.db.models import fields

from typing import Any, TypeVar, Generic, Union

_ST = TypeVar("_ST", contravariant=True)
_GT = TypeVar("_GT", covariant=True)

T = TypeVar("T")

class CustomFieldValue: ...

class GenericField(fields.Field[_ST, _GT]): ...

class SingleTypeField(fields.Field[T, T]): ...

class CustomValueField(fields.Field[Union[CustomFieldValue, int], CustomFieldValue]): ...

class AdditionalTypeVarField(fields.Field[_ST, _GT], Generic[_ST, _GT, T]): ...

class CustomSmallIntegerField(fields.SmallIntegerField[_ST, _GT]): ...

class User(models.Model):
id = models.AutoField(primary_key=True)
my_custom_field1 = GenericField[Union[CustomFieldValue, int], CustomFieldValue]()
my_custom_field2 = CustomValueField()
my_custom_field3 = SingleTypeField[bool]()
my_custom_field4 = AdditionalTypeVarField[Union[CustomFieldValue, int], CustomFieldValue, bool]()
my_custom_field5 = GenericField[Union[CustomFieldValue, int], CustomFieldValue](null=True)
my_custom_field6 = CustomValueField(null=True)
my_custom_field7 = SingleTypeField[bool](null=True)
my_custom_field8 = AdditionalTypeVarField[Union[CustomFieldValue, int], CustomFieldValue, bool](null=True)
my_custom_field9 = fields.Field[Union[CustomFieldValue, int], CustomFieldValue]()
my_custom_field10 = fields.Field[Union[CustomFieldValue, int], CustomFieldValue](null=True)
my_custom_field11 = fields.SmallIntegerField[bool, bool]()
my_custom_field12 = CustomSmallIntegerField[bool, bool]()