From 5bd751e5e28c06bbd5a1d6eabc7aca671fbd0c5c Mon Sep 17 00:00:00 2001 From: Thibaut Decombe Date: Sat, 20 Sep 2025 17:47:07 +0200 Subject: [PATCH 1/5] uv lock --- uv.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/uv.lock b/uv.lock index e94916e1e..eb1477f90 100644 --- a/uv.lock +++ b/uv.lock @@ -195,7 +195,7 @@ wheels = [ [[package]] name = "django-stubs" -version = "5.2.3" +version = "5.2.5" source = { editable = "." } dependencies = [ { name = "django" }, @@ -295,7 +295,7 @@ tests = [ [[package]] name = "django-stubs-ext" -version = "5.2.3" +version = "5.2.5" source = { editable = "ext" } dependencies = [ { name = "django" }, From a1f80ead79dfcefb1d6e70bb11704b4330c1e3bb Mon Sep 17 00:00:00 2001 From: Thibaut Decombe Date: Sat, 13 Sep 2025 20:16:40 +0200 Subject: [PATCH 2/5] Track deselected field via `values`/`values_list` calls --- mypy_django_plugin/transformers/querysets.py | 79 +++++++++++++++++-- .../managers/querysets/test_annotate.yml | 69 ++++++++++++++++ 2 files changed, 140 insertions(+), 8 deletions(-) diff --git a/mypy_django_plugin/transformers/querysets.py b/mypy_django_plugin/transformers/querysets.py index 49771f242..387b38962 100644 --- a/mypy_django_plugin/transformers/querysets.py +++ b/mypy_django_plugin/transformers/querysets.py @@ -14,7 +14,17 @@ from mypy.errorcodes import NO_REDEF from mypy.nodes import ARG_NAMED, ARG_NAMED_OPT, ARG_STAR, CallExpr, Expression from mypy.plugin import FunctionContext, MethodContext -from mypy.types import AnyType, Instance, LiteralType, ProperType, TupleType, TypedDictType, TypeOfAny, get_proper_type +from mypy.types import ( + AnyType, + ExtraAttrs, + Instance, + LiteralType, + ProperType, + TupleType, + TypedDictType, + TypeOfAny, + get_proper_type, +) from mypy.types import Type as MypyType from mypy_django_plugin.django.context import DjangoContext, LookupsAreUnsupported @@ -168,7 +178,13 @@ def extract_proper_type_queryset_values_list(ctx: MethodContext, django_context: row_type = get_values_list_row_type( ctx, django_context, django_model.cls, is_annotated=django_model.is_annotated, flat=flat, named=named ) - return default_return_type.copy_modified(args=[django_model.typ, row_type]) + ret = default_return_type.copy_modified(args=[django_model.typ, row_type]) + if not named and (field_lookups := resolve_field_lookups(ctx.args[0], django_context)): + # For non-named values_list, the row type does not encode column names. + # Attach selected field names to the returned QuerySet instance so that + # subsequent annotate() can make an informed decision about name conflicts. + ret.extra_attrs = ExtraAttrs(attrs={}, immutable=set(field_lookups), mod_name=None) + return ret def gather_kwargs(ctx: MethodContext) -> dict[str, MypyType] | None: @@ -233,10 +249,11 @@ def extract_proper_type_queryset_annotate(ctx: MethodContext, django_context: Dj return ctx.default_return_type api = helpers.get_typechecker_api(ctx) + expression_types = { attr_name: typ for attr_name, typ in gather_expression_types(ctx).items() - if check_valid_attr_value(ctx, django_model, attr_name) + if check_valid_attr_value(ctx, django_context, django_model, attr_name) } annotated_type: ProperType = django_model.typ @@ -426,8 +443,42 @@ def gather_flat_args(ctx: MethodContext) -> list[tuple[Expression | None, Proper return lookups +def _get_selected_fields_from_queryset_type(qs_type: Instance) -> set[str] | None: + """ + Derive selected field names from a QuerySet type. + + Sources: + - values(): encoded in the row TypedDict keys + - values_list(named=True): row is a NamedTuple; extract field names from fallback TypeInfo + - values_list(named=False): stored in qs_type.extra_attrs.immutable + """ + if len(qs_type.args) > 1: + row_type = get_proper_type(qs_type.args[1]) + if isinstance(row_type, Instance) and helpers.is_model_type(row_type.type): + return None + if isinstance(row_type, TypedDictType): + return set(row_type.items.keys()) + if isinstance(row_type, TupleType): + if row_type.partial_fallback.type.has_base("typing.NamedTuple"): + return {name for name, sym in row_type.partial_fallback.type.names.items() if sym.plugin_generated} + else: + return set() + return set() + + # Fallback to explicit metadata attached to the QuerySet Instance + if qs_type.extra_attrs and qs_type.extra_attrs.immutable and isinstance(qs_type.extra_attrs.immutable, set): + return qs_type.extra_attrs.immutable + + return None + + def check_valid_attr_value( - ctx: MethodContext, model: DjangoModel, attr_name: str, new_attrs: dict[str, MypyType] | None = None + ctx: MethodContext, + django_context: DjangoContext, + model: DjangoModel, + attr_name: str, + *, + new_attr_names: set[str] | None = None, ) -> bool: """ Check if adding `attr_name` would conflict with existing symbols on `model`. @@ -435,13 +486,23 @@ def check_valid_attr_value( Args: - model: The Django model being analyzed - attr_name: The name of the attribute to be added - - new_attrs: A mapping of field names to types currently being added to the model + - new_attr_names: A mapping of field names to types currently being added to the model """ + deselected_fields: set[str] | None = None + if isinstance(ctx.type, Instance): + selected_fields = _get_selected_fields_from_queryset_type(ctx.type) + if selected_fields is not None: + model_field_names = {f.name for f in django_context.get_model_fields(model.cls)} + deselected_fields = model_field_names - selected_fields + new_attr_names = new_attr_names or set() + new_attr_names.update(selected_fields - model_field_names) + is_conflicting_attr_value = bool( - # 1. Conflict with another symbol on the model. + # 1. Conflict with another symbol on the model (If not de-selected via a prior .values/.values_list call). # Ex: # User.objects.prefetch_related(Prefetch(..., to_attr="id")) model.typ.type.get(attr_name) + and (deselected_fields is None or attr_name not in deselected_fields) # 2. Conflict with a previous annotation. # Ex: # User.objects.annotate(foo=...).prefetch_related(Prefetch(...,to_attr="foo")) @@ -453,7 +514,7 @@ def check_valid_attr_value( # Prefetch("groups", Group.objects.filter(name="test"), to_attr="new_attr"), # Prefetch("groups", Group.objects.all(), to_attr="new_attr"), # E: Not OK! # ) - or (new_attrs is not None and attr_name in new_attrs) + or (new_attr_names is not None and attr_name in new_attr_names) ) if is_conflicting_attr_value: ctx.api.fail( @@ -585,7 +646,9 @@ def extract_prefetch_related_annotations(ctx: MethodContext, django_context: Dja except (FieldError, LookupsAreUnsupported): pass - if to_attr and check_valid_attr_value(ctx, qs_model, to_attr, new_attrs): + if to_attr and check_valid_attr_value( + ctx, django_context, qs_model, to_attr, new_attr_names=set(new_attrs.keys()) + ): new_attrs[to_attr] = api.named_generic_type( "builtins.list", [elem_model if elem_model is not None else AnyType(TypeOfAny.special_form)], diff --git a/tests/typecheck/managers/querysets/test_annotate.yml b/tests/typecheck/managers/querysets/test_annotate.yml index eee286265..ae11a8df2 100644 --- a/tests/typecheck/managers/querysets/test_annotate.yml +++ b/tests/typecheck/managers/querysets/test_annotate.yml @@ -425,3 +425,72 @@ content: | from django.db import models class Blog(models.Model): pass + +- case: test_annotate_existing_field + installed_apps: + - django.contrib.auth + main: | + from typing import Any + from typing_extensions import TypedDict + from django.db import models + from django.db.models import Prefetch, F, QuerySet + from django.contrib.auth.models import User, Group + from django_stubs_ext.annotations import WithAnnotations + + # Error on existing field / remapped field / previous annotation + User.objects.annotate(username=F("username")) # E: Attribute "username" already defined on "django.contrib.auth.models.User" [no-redef] + User.objects.values(foo=F("id")).annotate(foo=F("username")) # E: Attribute "foo" already defined on "django.contrib.auth.models.User" [no-redef] + User.objects.annotate(computed=F('id')).annotate(computed=F('username')) # E: Attribute "computed" already defined on "django.contrib.auth.models.User@AnnotatedWith[TypedDict({'computed': Any})]" [no-redef] + + # Should be ok if filtered with `values` / `values_list` + User.objects.values_list("id").annotate(username=F("username")) + User.objects.values("id").annotate(username=F("username")) + User.objects.values_list("id", named=True).annotate(username=F("username")) + + def get_with_values_list() -> QuerySet[User, tuple[int]]: + return User.objects.values_list("id") + + get_with_values_list().annotate(username=F("username")) + + class OnlyIdDict(TypedDict): + id: int + + def get_with_values() -> QuerySet[User, OnlyIdDict]: + return User.objects.values("id") + + get_with_values().annotate(username=F("username")) + + # But still cause issue if overlapping with other symbols (methods, ...) + User.objects.values_list("id").annotate(get_full_name=F("username")) # E: Attribute "get_full_name" already defined on "django.contrib.auth.models.User" [no-redef] + User.objects.values("id").annotate(get_full_name=F("username")) # E: Attribute "get_full_name" already defined on "django.contrib.auth.models.User" [no-redef] + + # No false positive on approximative row types + tuple_any_row: models.QuerySet[User, tuple[Any, ...]] + tuple_any_row.annotate(username=F("username")) + + any_row: models.QuerySet[User, Any] + any_row.annotate(username=F("username")) + + dict_row: models.QuerySet[User, dict] + dict_row.annotate(username=F("username")) + + # Ensure collisions with methods are still errors in approximate contexts + get_with_values_list().annotate(get_full_name=F("username")) # E: Attribute "get_full_name" already defined on "django.contrib.auth.models.User" [no-redef] + tuple_any_row.annotate(get_full_name=F("username")) # E: Attribute "get_full_name" already defined on "django.contrib.auth.models.User" [no-redef] + any_row.annotate(get_full_name=F("username")) # E: Attribute "get_full_name" already defined on "django.contrib.auth.models.User" [no-redef] + dict_row.annotate(get_full_name=F("username")) # E: Attribute "get_full_name" already defined on "django.contrib.auth.models.User" [no-redef] + + # Named values_list + + # Test name collision with model methods in named values_list - should still error + User.objects.values_list("id", named=True).annotate(get_full_name=F("username")) # E: Attribute "get_full_name" already defined on "django.contrib.auth.models.User" [no-redef] + User.objects.values_list("username", named=True).annotate(get_full_name=F("first_name")) # E: Attribute "get_full_name" already defined on "django.contrib.auth.models.User" [no-redef] + User.objects.values_list("id", named=True).annotate(is_anonymous=F("username")) # E: Attribute "is_anonymous" already defined on "django.contrib.auth.models.User" [no-redef] + + # Test name collision with model fields when field is NOT in values_list - should be OK + User.objects.values_list("id", named=True).annotate(username=F("first_name")) + User.objects.values_list("first_name", named=True).annotate(username=F("last_name")) + + # Test name collision with model fields when field IS in values_list - should error + User.objects.values_list("username", named=True).annotate(username=F("first_name")) # E: Attribute "username" already defined on "django.contrib.auth.models.User" [no-redef] + User.objects.values_list("id", "username", named=True).annotate(username=F("first_name")) # E: Attribute "username" already defined on "django.contrib.auth.models.User" [no-redef] From 08375b36bc1284e75bbe289b41a3e83d5edf0c64 Mon Sep 17 00:00:00 2001 From: Thibaut Decombe Date: Sat, 4 Oct 2025 19:35:43 +0200 Subject: [PATCH 3/5] Add a `merge_extra_attrs` helper --- mypy_django_plugin/lib/helpers.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/mypy_django_plugin/lib/helpers.py b/mypy_django_plugin/lib/helpers.py index 07103162a..adb2ba80e 100644 --- a/mypy_django_plugin/lib/helpers.py +++ b/mypy_django_plugin/lib/helpers.py @@ -43,6 +43,7 @@ from mypy.types import ( AnyType, CallableType, + ExtraAttrs, Instance, LiteralType, NoneTyp, @@ -694,3 +695,32 @@ def get_model_from_expression( def fill_manager(manager: TypeInfo, typ: MypyType) -> Instance: return Instance(manager, [typ] if manager.is_generic() else []) + + +def merge_extra_attrs( + base_extra_attrs: ExtraAttrs | None, + *, + new_attrs: dict[str, MypyType] | None = None, + new_immutable: set[str] | None = None, +) -> ExtraAttrs: + """ + Create a new `ExtraAttrs` by merging new attributes/immutable fields into a base. + + If base_extra_attrs is None, creates a fresh ExtraAttrs with only the new values. + """ + if base_extra_attrs: + return ExtraAttrs( + attrs={**base_extra_attrs.attrs, **new_attrs} if new_attrs is not None else base_extra_attrs.attrs.copy(), + immutable=( + base_extra_attrs.immutable | new_immutable + if new_immutable is not None + else base_extra_attrs.immutable.copy() + ), + mod_name=None, + ) + else: + return ExtraAttrs( + attrs=new_attrs or {}, + immutable=new_immutable, + mod_name=None, + ) From a6424b23fe4669072ef255d2e814f49cf294abc6 Mon Sep 17 00:00:00 2001 From: Thibaut Decombe Date: Sat, 4 Oct 2025 19:41:33 +0200 Subject: [PATCH 4/5] Use new helpers in places where we temper with extra_attrs --- mypy_django_plugin/transformers/models.py | 25 ++------------------ mypy_django_plugin/transformers/querysets.py | 14 ++--------- 2 files changed, 4 insertions(+), 35 deletions(-) diff --git a/mypy_django_plugin/transformers/models.py b/mypy_django_plugin/transformers/models.py index 17b68e271..4f42dd903 100644 --- a/mypy_django_plugin/transformers/models.py +++ b/mypy_django_plugin/transformers/models.py @@ -26,17 +26,7 @@ from mypy.plugins import common from mypy.semanal import SemanticAnalyzer from mypy.typeanal import TypeAnalyser -from mypy.types import ( - AnyType, - ExtraAttrs, - Instance, - ProperType, - TypedDictType, - TypeOfAny, - TypeType, - TypeVarType, - get_proper_type, -) +from mypy.types import AnyType, Instance, ProperType, TypedDictType, TypeOfAny, TypeType, TypeVarType, get_proper_type from mypy.types import Type as MypyType from mypy.typevars import fill_typevars, fill_typevars_with_any @@ -1166,18 +1156,7 @@ def get_annotated_type( """ Get a model type that can be used to represent an annotated model """ - if model_type.extra_attrs: - extra_attrs = ExtraAttrs( - attrs={**model_type.extra_attrs.attrs, **fields_dict.items}, - immutable=model_type.extra_attrs.immutable.copy(), - mod_name=None, - ) - else: - extra_attrs = ExtraAttrs( - attrs=fields_dict.items, - immutable=None, - mod_name=None, - ) + extra_attrs = helpers.merge_extra_attrs(model_type.extra_attrs, new_attrs=fields_dict.items) annotated_model: TypeInfo | None if helpers.is_annotated_model(model_type.type): diff --git a/mypy_django_plugin/transformers/querysets.py b/mypy_django_plugin/transformers/querysets.py index 387b38962..3c0c8cf5f 100644 --- a/mypy_django_plugin/transformers/querysets.py +++ b/mypy_django_plugin/transformers/querysets.py @@ -14,17 +14,7 @@ from mypy.errorcodes import NO_REDEF from mypy.nodes import ARG_NAMED, ARG_NAMED_OPT, ARG_STAR, CallExpr, Expression from mypy.plugin import FunctionContext, MethodContext -from mypy.types import ( - AnyType, - ExtraAttrs, - Instance, - LiteralType, - ProperType, - TupleType, - TypedDictType, - TypeOfAny, - get_proper_type, -) +from mypy.types import AnyType, Instance, LiteralType, ProperType, TupleType, TypedDictType, TypeOfAny, get_proper_type from mypy.types import Type as MypyType from mypy_django_plugin.django.context import DjangoContext, LookupsAreUnsupported @@ -183,7 +173,7 @@ def extract_proper_type_queryset_values_list(ctx: MethodContext, django_context: # For non-named values_list, the row type does not encode column names. # Attach selected field names to the returned QuerySet instance so that # subsequent annotate() can make an informed decision about name conflicts. - ret.extra_attrs = ExtraAttrs(attrs={}, immutable=set(field_lookups), mod_name=None) + ret.extra_attrs = helpers.merge_extra_attrs(ret.extra_attrs, new_immutable=set(field_lookups)) return ret From 7ebd3dd049cd14caa6b60f8bb2fa223324a4ce3a Mon Sep 17 00:00:00 2001 From: Thibaut Decombe Date: Sat, 4 Oct 2025 21:27:20 +0200 Subject: [PATCH 5/5] Cr change --- mypy_django_plugin/lib/helpers.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/mypy_django_plugin/lib/helpers.py b/mypy_django_plugin/lib/helpers.py index adb2ba80e..e4c3f3d2a 100644 --- a/mypy_django_plugin/lib/helpers.py +++ b/mypy_django_plugin/lib/helpers.py @@ -718,9 +718,8 @@ def merge_extra_attrs( ), mod_name=None, ) - else: - return ExtraAttrs( - attrs=new_attrs or {}, - immutable=new_immutable, - mod_name=None, - ) + return ExtraAttrs( + attrs=new_attrs or {}, + immutable=new_immutable, + mod_name=None, + )