django-stubs icon indicating copy to clipboard operation
django-stubs copied to clipboard

Feat/builtin models types

Open FallenDeity opened this issue 7 months ago • 25 comments

I have made things!

This PR aims to add type hints to builtin model fields, i.e for example models in contrib, admin, auth etc.

Base generic fields are modified to use the default= from PEP-696 to allow the following behaviour for models

_ST_IntegerField = TypeVar("_ST_IntegerField", default=float | int | str | Combinable)
_GT_IntegerField = TypeVar("_GT_IntegerField", default=int)

class IntegerField(Field[_ST_IntegerField, _GT_IntegerField]):
    _pyi_private_set_type: float | int | str | Combinable
    _pyi_private_get_type: int
    _pyi_lookup_exact_type: str | int
class Redirect(models.Model):
    id: models.AutoField
    pk: models.AutoField
    site: models.ForeignKey[Site | Combinable, Site]
    site_id: int
    old_path: models.CharField
    new_path: models.CharField

It eliminates the need to add generic arguments explicitly at each step whenever defining models, and ensures all the models being used internally have type hints.

This PR is not complete yet as I have a few questions to ask as I am not entirely clear about the whole process here are a few points I had doubts on, and wanted some feedback before proceeding

  1. ~~Is there any particular reason models for contrib/gis/db/backends are typed Any and not included in allowlist_todo.txt, since they seem to have types here, should I type out the model fields as such~~ edit: models typed in line with source
class OracleGeometryColumns(models.Model):
    table_name: models.CharField
    column_name: models.CharField
    srid: models.IntegerField
    objects: ClassVar[Manager[Self]]
  1. ~~I noticed the presence of most of the inbuilt model fields and methods in allowlist_todo.txt so how would that be handled do I write tests under the assert_type and typecheck/contrib folder for these models and move them to allowlist.txt manually if they are type hinted and tested?~~ edit: Resolved created a category

  2. ~~Final question, there is one case where the current model field typing system might need some explicit type hinting from the user in case of where fields become optional or with null=True, because with current system I don't think there is a way to infer types from the field parameters passed, Here is an example~~

edit: Resolved using __new__ overloads

text: models.CharField # get or return type is str, as expected
# But when params like blank=True, null=True are passed no way to autoinfer that and change _GT, _ST
# i.e without explicit typehints or generic params it would still show `str`, if we were to do `reveal_type`
# Redundant verbose example
text_nullable = models.CharField[Optional[Union[str, int, Combinable]], Optional[str]](max_length=100, null=True)
# A more generic user might have to do this
text_nullable = models.CharField[str | None, str | None](max_length=100, null=True)

This is fixable if we modify the django-mypy-plugin code but not sure if thats the best option, here is how one might go about it

def set_descriptor_types_for_field(
    ctx: FunctionContext, *, is_set_nullable: bool = False, is_get_nullable: bool = False
) -> Instance:
    default_return_type = cast(Instance, ctx.default_return_type)

    # Check for null_expr and primary key stuff
    ...

+  # We get expected nullable types here
    set_type, get_type = get_field_descriptor_types(
        default_return_type.type,
        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 == fullnames.FIELD_FULLNAME)
    mapped_instance = map_instance_to_supertype(default_return_type, base_field_type)
+  # But mapped types give use the generic types we have in our fields without None
    mapped_set_type, mapped_get_type = tuple(get_proper_type(arg) for arg in 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)):
        # always replace set_type and get_type with (non-Any) mapped types
        set_type = helpers.convert_any_to_type(mapped_set_type, set_type)
        get_type = get_proper_type(helpers.convert_any_to_type(mapped_get_type, get_type))

-       # the get_type must be optional if the field is nullable
+      # Instead of nullable expression is set to True we make out mapped type optional
        if (is_get_nullable or is_nullable) and not (
            isinstance(get_type, NoneType) or helpers.is_optional(get_type) or isinstance(get_type, AnyType)
        ):
+            get_type = helpers.make_optional_type(get_type)
+           set_type = helpers.make_optional_type(set_type)
-            ctx.api.fail(
-               f"{default_return_type.type.name} is nullable but its generic get type parameter is not optional",
-                ctx.context,
-           )

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

This was the only way I could think of going about it, and user needs to have the plugin installed.

Related issues

  • Refs: #2214

TODO

  • [x] Add more tests
  • [x] Work on the contrib/gis/backends models (most probably)
  • [x] Add overloads per field for __new__ with null

FallenDeity avatar Mar 30 '25 00:03 FallenDeity