Skip to content

Commit 3dc240a

Browse files
Fix Field.get_attribute() return and RelatedField.to_representation() parameter for PKOnlyObject (#852)
Looking at the implementation of `Serializer.to_representation()`, the value returned by `Field.get_attribute()` is passed to `Field.to_representation()`, whose argument is declared to be `_VT`. Thus the declaration of `Field.get_attribute()` should be changed. Adjusts the typing for relational serializer fields so they return the actual model instances or the lightweight PKOnlyObject Django REST Framework sometimes uses, instead of the already-serialized values. This matches DRF’s real behavior: get_attribute hands to_representation either the full object or the PK-only placeholder, so the new annotations describe exactly what callers receive and avoid type errors when the optimization kicks in. Signed-off-by: Emmanuel Ferdman <[email protected]>
1 parent 7889877 commit 3dc240a

File tree

4 files changed

+34
-3
lines changed

4 files changed

+34
-3
lines changed

rest_framework-stubs/fields.pyi

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ class Field(Generic[_VT, _DT, _RP, _IN]):
113113
def get_validators(self) -> list[Validator[_VT]]: ...
114114
def get_initial(self) -> _VT | None: ...
115115
def get_value(self, dictionary: Mapping[Any, Any]) -> Any: ...
116-
def get_attribute(self, instance: _IN) -> _RP | None: ...
116+
def get_attribute(self, instance: _IN) -> _VT | None: ...
117117
def get_default(self) -> _VT | None: ...
118118
def validate_empty_values(self, data: Any) -> tuple[bool, Any]: ...
119119
def run_validation(self, data: Any = ...) -> Any: ...

rest_framework-stubs/relations.pyi

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ class RelatedField(Field[_MT, _DT, _PT, Any]):
6868
@property
6969
def grouped_choices(self) -> dict: ...
7070
def iter_options(self) -> Iterable[Option]: ...
71-
def get_attribute(self, instance: _MT) -> _PT | None: ...
71+
def get_attribute(self, instance: _MT) -> _MT | PKOnlyObject | None: ... # type: ignore[override]
72+
def to_representation(self, value: _MT | PKOnlyObject) -> _PT: ...
7273
def display_value(self, instance: _MT) -> str: ...
7374

7475
class StringRelatedField(RelatedField[_MT, _MT, str]): ...
@@ -160,7 +161,7 @@ class SlugRelatedField(RelatedField[_MT, str, str]):
160161
style: dict[str, str] | None = ...,
161162
) -> None: ...
162163
def to_internal_value(self, data: Any) -> _MT: ...
163-
def to_representation(self, obj: _MT) -> str: ...
164+
def to_representation(self, obj: _MT | PKOnlyObject) -> str: ...
164165

165166
class ManyRelatedField(Field[Sequence[Any], Sequence[Any], list[Any], Any]):
166167
default_empty_html: list[object]

tests/typecheck/test_fields.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,14 @@
5757
x: str | None = CharField().get_initial()
5858
y: int | None = CharField().get_initial() # E: Incompatible types in assignment (expression has type "str | None", variable has type "int | None") [assignment]
5959
60+
- case: field_get_attribute_returns_value_type
61+
main: |
62+
from rest_framework import serializers
63+
64+
field = serializers.CharField()
65+
result = field.get_attribute(object())
66+
reveal_type(result) # N: Revealed type is "builtins.str | None"
67+
6068
- case: float_field_args_fields
6169
main: |
6270
from rest_framework.fields import FloatField

tests/typecheck/test_serializers.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,28 @@
4949
class Meta:
5050
model = User
5151
52+
- case: related_field_get_attribute_model_or_pk
53+
main: |
54+
from typing import cast
55+
from django.contrib.auth.models import User
56+
from rest_framework import serializers
57+
from rest_framework.relations import PKOnlyObject
58+
59+
field = serializers.PrimaryKeyRelatedField(queryset=User.objects.all())
60+
user = cast(User, object())
61+
value = field.get_attribute(user)
62+
reveal_type(value) # N: Revealed type is "django.contrib.auth.models.User | rest_framework.relations.PKOnlyObject | None"
63+
64+
- case: slug_related_field_accepts_pk_object
65+
main: |
66+
from django.contrib.auth.models import User
67+
from rest_framework import serializers
68+
from rest_framework.relations import PKOnlyObject
69+
70+
field = serializers.SlugRelatedField(slug_field="username", queryset=User.objects.all())
71+
obj = PKOnlyObject(pk=1)
72+
reveal_type(field.to_representation(obj)) # N: Revealed type is "builtins.str"
73+
5274
- case: test_hyperlinked_model_serializer_with_customized_serializer_field_mapping
5375
main: |
5476
from rest_framework import serializers

0 commit comments

Comments
 (0)