From 8a31773bec354b9096c4b4961a6cdc1b39aebd60 Mon Sep 17 00:00:00 2001 From: Niicck Date: Sun, 3 Nov 2024 16:20:53 -0600 Subject: [PATCH 1/2] fix .values_list returns incorrect type for field with same name when selected from related model --- .gitignore | 1 + mypy_django_plugin/django/context.py | 10 +++++----- mypy_django_plugin/transformers/querysets.py | 2 +- .../managers/querysets/test_values_list.yml | 18 ++++++++++++++++++ 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index 1141379bd..18cff1611 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ *.egg-info .DS_Store .idea/ +.vscode/ .mypy_cache/ .pytest_cache/ .ruff_cache/ diff --git a/mypy_django_plugin/django/context.py b/mypy_django_plugin/django/context.py index 6b4ee12aa..7603948f8 100644 --- a/mypy_django_plugin/django/context.py +++ b/mypy_django_plugin/django/context.py @@ -409,7 +409,7 @@ def get_field_related_model_cls(self, field: Union["RelatedField[Any, Any]", For def _resolve_field_from_parts( self, field_parts: Iterable[str], model_cls: Type[Model] - ) -> Union["Field[Any, Any]", ForeignObjectRel]: + ) -> Tuple[Union["Field[Any, Any]", ForeignObjectRel], Type[Model]]: currently_observed_model = model_cls field: Union[Field[Any, Any], ForeignObjectRel, GenericForeignKey, None] = None for field_part in field_parts: @@ -429,7 +429,7 @@ def _resolve_field_from_parts( # Guaranteed by `query.solve_lookup_type` before. assert isinstance(field, (Field, ForeignObjectRel)) - return field + return field, currently_observed_model def solve_lookup_type( self, model_cls: Type[Model], lookup: str @@ -467,10 +467,10 @@ def solve_lookup_type( def resolve_lookup_into_field( self, model_cls: Type[Model], lookup: str - ) -> Union["Field[Any, Any]", ForeignObjectRel, None]: + ) -> Tuple[Union["Field[Any, Any]", ForeignObjectRel, None], Type[Model]]: solved_lookup = self.solve_lookup_type(model_cls, lookup) if solved_lookup is None: - return None + return None, model_cls lookup_parts, field_parts, is_expression = solved_lookup if lookup_parts: raise LookupsAreUnsupported() @@ -501,7 +501,7 @@ def resolve_lookup_expected_type( if is_expression: return AnyType(TypeOfAny.explicit) - field = self._resolve_field_from_parts(field_parts, model_cls) + field, _ = self._resolve_field_from_parts(field_parts, model_cls) lookup_cls = None if lookup_parts: diff --git a/mypy_django_plugin/transformers/querysets.py b/mypy_django_plugin/transformers/querysets.py index 2bc98d717..c2046acdb 100644 --- a/mypy_django_plugin/transformers/querysets.py +++ b/mypy_django_plugin/transformers/querysets.py @@ -63,7 +63,7 @@ def get_field_type_from_lookup( silent_on_error: bool = False, ) -> Optional[MypyType]: try: - lookup_field = django_context.resolve_lookup_into_field(model_cls, lookup) + lookup_field, model_cls = django_context.resolve_lookup_into_field(model_cls, lookup) except FieldError as exc: if not silent_on_error: ctx.api.fail(exc.args[0], ctx.context) diff --git a/tests/typecheck/managers/querysets/test_values_list.yml b/tests/typecheck/managers/querysets/test_values_list.yml index b20935b08..192f22be3 100644 --- a/tests/typecheck/managers/querysets/test_values_list.yml +++ b/tests/typecheck/managers/querysets/test_values_list.yml @@ -322,3 +322,21 @@ class Blog(models.Model): num_posts = models.IntegerField() text = models.CharField(max_length=100, blank=True) +- case: handles_field_with_same_name_on_other_model + main: | + from myapp.models import A + reveal_type(A.objects.values_list("b__name").get()) # N: Revealed type is "Tuple[builtins.str]" + installed_apps: + - myapp + files: + - path: myapp/__init__.py + - path: myapp/models.py + content: | + from django.db import models + + class B(models.Model): + name = models.CharField() + + class A(models.Model): + b = models.ForeignKey(B, on_delete=models.CASCADE) + name = models.CharField(null=True) From 4b9a07e1e1114ebb6e4ca392e411d26c03e172bc Mon Sep 17 00:00:00 2001 From: Niicck Date: Mon, 4 Nov 2024 08:41:30 -0600 Subject: [PATCH 2/2] test both names in handles_field_with_same_name_on_other_model --- tests/typecheck/managers/querysets/test_values_list.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/typecheck/managers/querysets/test_values_list.yml b/tests/typecheck/managers/querysets/test_values_list.yml index 192f22be3..9315babf6 100644 --- a/tests/typecheck/managers/querysets/test_values_list.yml +++ b/tests/typecheck/managers/querysets/test_values_list.yml @@ -325,7 +325,7 @@ - case: handles_field_with_same_name_on_other_model main: | from myapp.models import A - reveal_type(A.objects.values_list("b__name").get()) # N: Revealed type is "Tuple[builtins.str]" + reveal_type(A.objects.values_list("name", "b__name").get()) # N: Revealed type is "Tuple[builtins.int, builtins.str]" installed_apps: - myapp files: @@ -339,4 +339,4 @@ class A(models.Model): b = models.ForeignKey(B, on_delete=models.CASCADE) - name = models.CharField(null=True) + name = models.IntegerField()