django-stubs

django-stubs copied to clipboard

I am seeing the following error:

"_MonkeyPatchedWSGIResponse" has no attribute "content"; maybe "context"? [attr-defined]

It gets set in the constructor: https://github.com/django/django/blob/73b4f3f9b3c2c64482c8a1174b3c5ab208d11279/django/http/response.py#L355

And it is typed properly: https://github.com/typeddjango/django-stubs/blob/008662c09753ab3e472151ced743e18e6f85716e/django-stubs/http/response.pyi#L87-L88

But the responses are derived from HttpResponseBase, which might be wrong? https://github.com/typeddjango/django-stubs/blob/008662c09753ab3e472151ced743e18e6f85716e/django-stubs/test/client.pyi#L55-L59

Ref: 62263814 (/cc @sterliakov)

Example code:

def test_foo(client: "Client") -> None:
    resp = client.post("/foo")
    assert resp.status_code == 400
    assert "<title>Bad Request (400)</title>" in resp.content.decode()

System information

• OS:
• python version:
• django version:
• mypy version:
• django-stubs version: v1.11.0
• django-stubs-ext version:

Related: https://github.com/typeddjango/django-stubs/pull/968

1. other missing attributes: context_data (where I use "errors" from), url, and template_name.

2. With context["errors"] I've noticed that it results in:

No overload variant of "getitem" of "list" matches argument type "str" [call-overload]

The revealed type for response.context is builtins.list[builtins.dict[builtins.str, Any]].

Should I create a separate issue for the problem with context? I've tried fixing it quickly with the following, but it still gives the call-overload error:

diff --git i/django-stubs/test/client.pyi w/django-stubs/test/client.pyi
index 85c74ba..3e203dc 100644
--- i/django-stubs/test/client.pyi
+++ w/django-stubs/test/client.pyi
@@ -28,6 +28,7 @@ from django.http.cookie import SimpleCookie
 from django.http.request import HttpRequest
 from django.http.response import HttpResponseBase
 from django.template.base import Template
+from django.test.utils import ContextList
 from django.urls import ResolverMatch

 BOUNDARY: str = ...
@@ -125,7 +126,7 @@ class _MonkeyPatchedWSGIResponse(_WSGIResponse):
     request: Dict[str, Any]
     client: Client
     templates: List[Template]
-    context: List[Dict[str, Any]]
+    context: ContextList
     content: bytes
     resolver_match: ResolverMatch

The content attribute is missing as well

pytest-django users need this.

django-stubs 1.12.0 pytest-django 4.5.2

Workaround

Python 3.10+:

import typing

response_url = typing.cast(str | None, getattr(response, 'url', None))

Python < 3.10:

import typing

response_url = typing.cast(typing.Union[str , None], getattr(response, 'url', None))

@tony Wait, but url is not documented among other attributes. Also it doesn't seem to be added in code. Neither HttpResponseBase nor its subclasses (both HttpResponse and FileResponse) do have it (only HttpResponseRedirectBase). Where does url attribute come from?

Also, problem with content was closed (#968, finally decided not to prefer getvalue and to allow content on base responses), context is fixed as well, so this issue should be closed after url issue is resolved.

The case I'm aware of for .url is with responses that are redirects (apparently from HttpResponseRedirectBase as you mentioned), which can be returned from the client request methods, like response = client.get(...). It seems non-ideal to have a mypy violation when referring to .url if you know/expect it's a redirect response.

As long as this relates to redirect responses, and client.get cannot return specific response subclasses (since there is no way to know that statically), I suggest to assert isinstance(response, HttpResponseRedirect) in (very rare, I suppose) cases when you need to access url - the type will be narrowed to intersection. Current state is perfectly valid, IMO: we cannot know that one exact call returned redirect response, and we shouldn't add url to all responses (because most of them simply don't have it). Rare type errors (or type asserts) are absolutely fine, while missing error when it should actually arise is more dangerous. Type checkers are used to guard the code from unsafe behavior - why should we allow something that raises AttributeError in fact?

Again, the simplest possible solutions are a) type: ignore comment and b) type assert. This is a standard practice for all cases when a function returns broader type compared to required one. I'll check tomorrow whether type assert is valid here (so that mypy doesn't say something like "subclass of X and Y cannot exist: would have incompatible signatures"), but it should be, AFAIR. And finally, we're talking about typechecking of tests, where writing type-safe code often doesn't make sense, and type-ignore comment is often the best solution.

Do you have any other proposals? I'm against adding url to all responses (mainly because I can try to use it at some point, and don't want mypy to allow it on arbitrary response), and I cannot imagine a solution to detect response type (it is possible, of course, but would take enormous amount of plugin code).

On Mon, Sep 12, 2022, 20:04 Steven DeMartini @.***> wrote:

The case I'm aware of for .url is with responses that are redirects (apparently from HttpResponseRedirectBase https://github.com/django/django/blob/main/django/http/response.py#L602 as you mentioned), which can be returned from the client request methods, like response = client.get(...). It seems non-ideal to have a mypy violation when referring to .url if you know/expect it's a redirect response.

— Reply to this email directly, view it on GitHub https://github.com/typeddjango/django-stubs/issues/971#issuecomment-1243958002, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMBQIRFSABTM4BRNQOFMNG3V55IAJANCNFSM5W5FB63A . You are receiving this because you were mentioned.Message ID: @.***>

assert isinstance(response, HttpResponseRedirect) works without issue in the situation in my test code. Agreed that that's a reasonable approach here and no need to add url generally to the monkey-patched response. 👍

I just fixed response.url errors in a project I'm working on. Instead of adding assert isinstance(response, HttpResponseRedirect) would work, I switched to just using response["Location"] instead. The url property is just that lookup under the hood so it didn't seem worth it to add all the extra type-narrowing asserts.

Anyway seems like we've reasonably addressed all the problems here so let's close the issue.