Skip to content

Incorrect Union typing of decorators in django-stubs\utils\decorators.pyi #2824

@Myselfito

Description

@Myselfito

Bug report

What's wrong

In django-stubs\utils\decorators.pyi, the typing of decorators is as follow:

# django-stubs\utils\decorators.pyi
_DECORATOR: TypeAlias = Callable[..., Callable[..., HttpResponseBase] | Callable[..., Callable[..., HttpResponseBase]]]

# This hint is used for the decorator_from_middleware methods
def decorator_from_middleware_with_args(middleware_class: type) -> _DECORATOR: ...
def decorator_from_middleware(middleware_class: type) -> _DECORATOR: ...

That creates several issues when attempting to build custom middleware decorators. We show below a minimal example, and two simplified use cases where type errors are incorrectly generated because of the Union type.

import functools
from typing import Any, Callable, ParamSpec

from django.http.response import HttpResponseBase
from django.utils.decorators import (
    decorator_from_middleware_with_args,
    decorator_from_middleware,
)
from django.middleware.cache import CacheMiddleware

P = ParamSpec("P")


class CustomCacheMiddleWare(CacheMiddleware): ...


# Minimal issue
# We provide the expected base example of the decoration
# The example works as expected
@decorator_from_middleware_with_args(CustomCacheMiddleWare)(page_timeout=1)
def example_function(*args: Any, **kwargs: Any) -> HttpResponseBase:
    return HttpResponseBase()


# The below is problematic. That should not be allowed
value = decorator_from_middleware_with_args(CustomCacheMiddleWare)(page_timeout=1)(1) # <--- no error
if not callable(value):
    # If called with anything, a HttpResponse is a valid result!
    # problematic for a decorator
    reveal_type(value)  # Revealed type is "django.http.response.HttpResponseBase" !


# Minimal "real life" simplified application issue: building a custom cache page
def custom_cache_page(
    test_parameter: str,
) -> Callable[[Callable[P, HttpResponseBase]], Callable[P, HttpResponseBase]]:
    print(test_parameter)

    def main_decorator(
        func: Callable[P, HttpResponseBase],
    ) -> Callable[P, HttpResponseBase]:
        decorator = decorator_from_middleware_with_args(CustomCacheMiddleWare)()
        return functools.wraps(func)(decorator(func))  # mypy error:
        # Argument 1 to "__call__" of "_Wrapper" has incompatible type
        # "HttpResponseBase | Callable[..., HttpResponseBase]";
        # expected "Callable[[VarArg(Any), KwArg(Any)], HttpResponseBase]"  [arg-type]

    return main_decorator

# Minimal "real life" simplified application issue 2: building a custom cache page with no parameter
def custom_cache_page_2(
    test_parameter: str,
) -> Callable[[Callable[P, HttpResponseBase]], Callable[P, HttpResponseBase]]:
    print(test_parameter)

    return decorator_from_middleware(CustomCacheMiddleWare)  # mypy error:
    # error: Incompatible return value type (got
    # "Callable[..., Callable[..., HttpResponseBase] | Callable[..., Callable[..., HttpResponseBase]]]",
    # expected "Callable[[Callable[P, HttpResponseBase]], Callable[P, HttpResponseBase]]")  [return-value]

How is that should be

We should have a different hint for the decorators and the decorators with parameters.
In the example below, we use TypeVar and ParamSpec, but ellipsis and HttpResponseBase for the return type
also work if needed.
With those amendments, the problematic line from above does raise a mypy error, and the "real life" applications do not.

# django-stubs\utils\decorators.pyi

from typing import Callable, ParamSpec, TypeVar

from django.http.response import HttpResponseBase
from typing_extensions import TypeAlias

_HttpResponseType = TypeVar("_HttpResponseType", bound=HttpResponseBase)
_ViewParamSpec = ParamSpec("_ViewParamSpec")

# The decorator return value needs to be a function and not a decorator
# Before: _DECORATOR: TypeAlias = Callable[..., Callable[..., HttpResponseBase] | Callable[..., Callable[..., HttpResponseBase]]]
# Correction with previous notations: _DECORATOR: TypeAlias = Callable[[Callable[..., HttpResponseBase]], Callable[..., HttpResponseBase]]
_DECORATOR: TypeAlias = Callable[
    [Callable[_ViewParamSpec, _HttpResponseType]],
    Callable[_ViewParamSpec, _HttpResponseType],
]  # <--- Standard decorator definition, with return value bound to HttpResponseBase

# We introduce a _DECORATOR_WITH_PARAMETERS type, this one's return type is a decorator
_DECORATOR_WITH_PARAMETERS: TypeAlias = Callable[..., _DECORATOR]

# decorator_from_middleware_with_args is actually returning a decorator with parameters
def decorator_from_middleware_with_args(
    middleware_class: type,
) -> _DECORATOR_WITH_PARAMETERS: ...


# decorator_from_middleware typing doesn't change
def decorator_from_middleware(middleware_class: type) -> _DECORATOR: ...

System information

  • OS: Windows
  • python version: 3.10
  • django version: 4.2
  • mypy version: 3.10
  • django-stubs version: 5.0.0
  • django-stubs-ext version: 5.0.0

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions