From a77a05e7f06a65d85dae9840037a939a9d39506c Mon Sep 17 00:00:00 2001 From: Marti Raudsepp Date: Thu, 30 Mar 2023 18:35:12 +0300 Subject: [PATCH] Improve APIException input and detail types APIException and its subclasses are more liberal in what they accept as input, but their `detail` attribute is already converted to a stricter type: translation strings are resolved, strings are conveted to ErrorDetail, etc This uses the recursive types feature from mypy and thus bumps minimum mypy version to 0.991: https://mypy-lang.blogspot.com/2022/11/mypy-0990-released.html --- rest_framework-stubs/exceptions.pyi | 48 +++++++++++++++++++---------- scripts/typecheck_tests.py | 9 ++++++ setup.py | 2 +- tests/typecheck/test_exceptions.yml | 24 +++++++++++++++ 4 files changed, 66 insertions(+), 17 deletions(-) diff --git a/rest_framework-stubs/exceptions.pyi b/rest_framework-stubs/exceptions.pyi index 942d4f35c..ecf59b811 100644 --- a/rest_framework-stubs/exceptions.pyi +++ b/rest_framework-stubs/exceptions.pyi @@ -1,33 +1,49 @@ -from collections.abc import Sequence +from collections.abc import Mapping, Sequence from typing import Any from django.http import HttpRequest, JsonResponse from django_stubs_ext import StrOrPromise from rest_framework.renderers import BaseRenderer from rest_framework.request import Request -from typing_extensions import TypeAlias - -def _get_error_details(data: Any, default_code: str | None = ...) -> Any: ... -def _get_codes(detail: Any) -> Any: ... -def _get_full_details(detail: Any) -> Any: ... +from typing_extensions import TypeAlias, TypedDict class ErrorDetail(str): code: str | None def __new__(cls, string: str, code: str | None = ...): ... -_Detail: TypeAlias = StrOrPromise | list[Any] | dict[str, Any] +_Detail: TypeAlias = ErrorDetail | list[ErrorDetail] | dict[str, ErrorDetail] +# NB! _APIExceptionInput doesn't technically handle Sequence/Mapping, but only list/tuple/dict. +# But since list/tuple are non-covariant types, we run into issues with union type compatibility for input params. +# So use the more relaxed Sequence/Mapping for now. +_APIExceptionInput: TypeAlias = ( + _Detail | StrOrPromise | Sequence[_APIExceptionInput] | Mapping[str, _APIExceptionInput] | None +) +_ErrorCodes: TypeAlias = str | None | list[_ErrorCodes] | dict[str, _ErrorCodes] + +class _FullDetailDict(TypedDict): + message: ErrorDetail + code: str | None + +_ErrorFullDetails: TypeAlias = _FullDetailDict | list[_FullDetailDict] | dict[str, _FullDetailDict] + +def _get_error_details(data: _APIExceptionInput, default_code: str | None = ...) -> _Detail: ... +def _get_codes(detail: _Detail) -> _ErrorCodes: ... +def _get_full_details(detail: _Detail) -> _ErrorFullDetails: ... class APIException(Exception): status_code: int - default_detail: _Detail + default_detail: _APIExceptionInput default_code: str detail: _Detail - def __init__(self, detail: _Detail | None = ..., code: str | None = ...) -> None: ... - def get_codes(self) -> Any: ... - def get_full_details(self) -> Any: ... + def __init__(self, detail: _APIExceptionInput = ..., code: str | None = ...) -> None: ... + def get_codes(self) -> _ErrorCodes: ... + def get_full_details(self) -> _ErrorFullDetails: ... + +class ValidationError(APIException): + # ValidationError always wraps `detail` in a list. + detail: list[ErrorDetail] | dict[str, ErrorDetail] -class ValidationError(APIException): ... class ParseError(APIException): ... class AuthenticationFailed(APIException): ... class NotAuthenticated(APIException): ... @@ -35,24 +51,24 @@ class PermissionDenied(APIException): ... class NotFound(APIException): ... class MethodNotAllowed(APIException): - def __init__(self, method: str, detail: _Detail | None = ..., code: str | None = ...) -> None: ... + def __init__(self, method: str, detail: _APIExceptionInput = ..., code: str | None = ...) -> None: ... class NotAcceptable(APIException): available_renderers: Sequence[BaseRenderer] | None def __init__( self, - detail: _Detail | None = ..., + detail: _APIExceptionInput = ..., code: str | None = ..., available_renderers: Sequence[BaseRenderer] | None = ..., ) -> None: ... class UnsupportedMediaType(APIException): - def __init__(self, media_type: str, detail: _Detail | None = ..., code: str | None = ...) -> None: ... + def __init__(self, media_type: str, detail: _APIExceptionInput = ..., code: str | None = ...) -> None: ... class Throttled(APIException): extra_detail_singular: str extra_detail_plural: str - def __init__(self, wait: float | None = ..., detail: _Detail | None = ..., code: str | None = ...): ... + def __init__(self, wait: float | None = ..., detail: _APIExceptionInput = ..., code: str | None = ...): ... def server_error(request: HttpRequest | Request, *args: Any, **kwargs: Any) -> JsonResponse: ... def bad_request(request: HttpRequest | Request, exception: Exception, *args: Any, **kwargs: Any) -> JsonResponse: ... diff --git a/scripts/typecheck_tests.py b/scripts/typecheck_tests.py index cffc1e235..2d482759c 100644 --- a/scripts/typecheck_tests.py +++ b/scripts/typecheck_tests.py @@ -57,6 +57,9 @@ '"_MonkeyPatchedWSGIResponse" has no attribute "data"', '" defined here', '" has no attribute "id"', + 'Invalid index type "int" for "Union[List[ErrorDetail], Dict[str, ErrorDetail]]"; expected type "str"', + 'Invalid index type "str" for "Union[ErrorDetail, List[ErrorDetail], Dict[str, ErrorDetail]]"; expected type "Union[SupportsIndex, slice]"', # noqa: E501 + 'Invalid index type "int" for "Union[ErrorDetail, List[ErrorDetail], Dict[str, ErrorDetail]]"; expected type "str"', # noqa: E501 ], "authentication": [ 'Argument 1 to "post" of "APIClient" has incompatible type "None"; expected "str', @@ -103,6 +106,12 @@ 'Argument 1 to "api_view" has incompatible type "Callable[[Any], Any]"; expected "Optional[Sequence[str]]"', ], "test_encoders.py": ['Argument "serializer" to "ReturnList" has incompatible type "None'], + "test_exceptions.py": [ + 'error: No overload variant of "__getitem__" of "list" matches argument type "str"', + "note: Possible overload variants:", + "note: def __getitem__(self, SupportsIndex, /) -> ErrorDetail", + "note: def __getitem__(self, slice, /) -> List[ErrorDetail]", + ], "test_fields.py": [ '"ChoiceModel"', 'Argument "validators" to "CharField" has incompatible type', diff --git a/setup.py b/setup.py index d23f492df..e773a4a73 100644 --- a/setup.py +++ b/setup.py @@ -20,7 +20,7 @@ def find_stub_files(name: str) -> List[str]: readme = f.read() dependencies = [ - "mypy>=0.980", + "mypy>=0.991", "django-stubs>=1.13.0", "typing-extensions>=3.10.0", "requests>=2.0.0", diff --git a/tests/typecheck/test_exceptions.yml b/tests/typecheck/test_exceptions.yml index c17849318..0d54e14b1 100644 --- a/tests/typecheck/test_exceptions.yml +++ b/tests/typecheck/test_exceptions.yml @@ -15,6 +15,7 @@ status_code = 200 default_detail = {"ok": "everything"} default_code = "ok" + - case: test_exception_declaration_lazystr main: | from django.utils.translation import gettext_lazy as _ @@ -24,3 +25,26 @@ status_code = 200 default_detail = _("Está tudo bem") default_code = "ok" + +- case: test_exception_input + main: | + from django.utils.translation import gettext_lazy as _ + from rest_framework.exceptions import APIException, ErrorDetail + + test_exception = APIException({ + 'a': [ + 'value', + _('translated'), + ErrorDetail('with code', code='brown'), + {'b': 'test', 'c': _('translated')}, + ('also', 'tuple', ErrorDetail('value')), + ] + }) + APIException(detail=test_exception.detail, code='123') + APIException(detail=[test_exception.detail], code='123') + APIException('I am just a message', code='msg') + APIException() + APIException(None, None) + APIException(...) # E: Argument 1 to "APIException" has incompatible type "ellipsis"; expected "_APIExceptionInput" + APIException({'a': ...}) # E: Dict entry 0 has incompatible type "str": "ellipsis"; expected "str": "Union[Sequence[_APIExceptionInput], Mapping[str, _APIExceptionInput], None]" + APIException({'a': ['test', ...]}) # E: List item 1 has incompatible type "ellipsis"; expected "Union[Sequence[_APIExceptionInput], Mapping[str, _APIExceptionInput], None]"