djangorestframework-simplejwt icon indicating copy to clipboard operation
djangorestframework-simplejwt copied to clipboard

Incorrect type hints for several classes/methods

Open mlazar-endear opened this issue 11 months ago • 4 comments

Hi!

I just updated my version to djangorestframework-simplejwt==5.3.1 and I'm running into several mypy errors that were introduced with the recent typehints PR here: https://github.com/jazzband/djangorestframework-simplejwt/pull/683

I've looked into it, and there are several incorrect types in this library. Many of them have are arguments/return values which are annotated as Token, but in reality should be str or bytes. Some of them are easy to spot (and should have been caught by mypy when adding the types in the first place). Some of them are more difficult because of the dynamic nature of the library (e.g. AUTH_TOKEN_CLASSES), but they raise errors when you start writing subclasses for the tokens and backends.

For example, this line passes a Token object into the underlying jwt library, which is incorrect (should be str or bytes).

https://github.com/jazzband/djangorestframework-simplejwt/blob/c791e987332ed5e22a86428160d6372b1d85ffae/rest_framework_simplejwt/backends.py#L139-L140

Same thing here, passes a Token object but it should be str or bytes.

https://github.com/jazzband/djangorestframework-simplejwt/blob/c791e987332ed5e22a86428160d6372b1d85ffae/rest_framework_simplejwt/backends.py#L100-L102

Here, the raw_token is correctly annotated, but then it's being passed into an AuthToken initializer which is expecting the argument to be Token.

https://github.com/jazzband/djangorestframework-simplejwt/blob/c791e987332ed5e22a86428160d6372b1d85ffae/rest_framework_simplejwt/authentication.py#L95-L103

This initializer is wrong (Why would the token class be initialized with an instance of itself?)

https://github.com/jazzband/djangorestframework-simplejwt/blob/c791e987332ed5e22a86428160d6372b1d85ffae/rest_framework_simplejwt/tokens.py#L30-L39

Unfortunately, I think fixing these type hints will require significant effort to go through and untangle everything.

mlazar-endear avatar Feb 29 '24 22:02 mlazar-endear

For anyone else who runs into this, you can configure mypy to ignore the all of the typehints provided by this library.

Here's what I added in my pyproject.toml file:

[[tool.mypy.overrides]]
# https://github.com/jazzband/djangorestframework-simplejwt/issues/787
module = "rest_framework_simplejwt.*"
follow_imports = "skip"

mlazar-endear avatar Mar 01 '24 15:03 mlazar-endear

@mlazar-endear do you have django-stubs enabled? I believe that's a requirement for type checking simplejwt

Andrew-Chen-Wang avatar Mar 19 '24 07:03 Andrew-Chen-Wang

@Andrew-Chen-Wang Yes I do, but I don't see any reason why that would matter.

django-stubs==4.2.7
django-stubs-ext==4.2.7
djangorestframework-stubs==3.14.5

django==4.2.10
djangorestframework-simplejwt==5.3.1

mlazar-endear avatar Mar 19 '24 13:03 mlazar-endear

just for diagnosing. yes i see the issues too thanks!

Andrew-Chen-Wang avatar Mar 19 '24 15:03 Andrew-Chen-Wang