microsoft / pylance-release

Documentation and issues for Pylance
Creative Commons Attribution 4.0 International
1.72k stars 767 forks source link

Django stubs are out of date and cause erroneous type errors #6029

Open simonwhitaker opened 5 months ago

simonwhitaker commented 5 months ago

The Django stubs that ship with the Pylance VSCode extension are out of date, and take precedence over stubs installed in the local environment.

Screenshot 2024-06-18 at 5 06 14 PM

Environment data

Code Snippet

n/a

Repro Steps

  1. In a Python 3.12 (virtual) environment:
    git clone git@github.com:simonwhitaker/pyright-stubs-demo.git
    cd pyright-stubs-demo
    pip install -r requirements.txt
  2. Open the cloned repo in VS Code
  3. Open manage.py
  4. Observe that RemovedInDjango51Warning on L6 is highlighted
  5. Observe the error: "RemovedInDjango51Warning" is unknown import symbol

Expected behavior

Actual behavior

See screenshot above.

This appears to be because the django stubs shipped with the Pylance extension are out of date:

$ $ cat ~/.vscode/extensions/ms-python.vscode-pylance-2024.6.1/dist/bundled/stubs/django-stubs/utils/deprecation.pyi
from collections.abc import Callable
from typing import Any

from django.http.request import HttpRequest
from django.http.response import HttpResponse

class RemovedInDjango30Warning(PendingDeprecationWarning): ...
class RemovedInDjango31Warning(PendingDeprecationWarning): ...
class RemovedInDjango40Warning(PendingDeprecationWarning): ...
class RemovedInNextVersionWarning(DeprecationWarning): ...

class warn_about_renamed_method:
    class_name: str = ...
    old_method_name: str = ...
    new_method_name: str = ...
    deprecation_warning: type[DeprecationWarning] = ...
    def __init__(
        self,
        class_name: str,
        old_method_name: str,
        new_method_name: str,
        deprecation_warning: type[DeprecationWarning],
    ) -> None: ...
    def __call__(self, f: Callable[..., Any]) -> Callable[..., Any]: ...

class RenameMethodsBase(type):
    renamed_methods: Any = ...
    def __new__(cls, name: Any, bases: Any, attrs: Any) -> Any: ...

class DeprecationInstanceCheck(type):
    alternative: str
    deprecation_warning: type[Warning]
    def __instancecheck__(self, instance: Any) -> Any: ...

GetResponseCallable = Callable[[HttpRequest], HttpResponse]

class MiddlewareMixin:
    get_response: GetResponseCallable | None = ...
    def __init__(self, get_response: GetResponseCallable | None = ...) -> None: ...
    def __call__(self, request: HttpRequest) -> HttpResponse: ...

Logs

XXX
rchiodo commented 5 months ago

Thanks for the issue. We actually pull django stubs from here: https://github.com/sbdchd/django-types

That's because the other django stubs (https://github.com/typeddjango/django-stubs) are mypy only and don't work with other type checkers.

It's likely the types you're talking about need to be added to https://github.com/sbdchd/django-types

simonwhitaker commented 5 months ago

Thanks for the issue. We actually pull django stubs from here: https://github.com/sbdchd/django-types

That's because the other django stubs (https://github.com/typeddjango/django-stubs) are mypy only and don't work with other type checkers.

It's likely the types you're talking about need to be added to https://github.com/sbdchd/django-types

Good to know! Thanks, I'll chase on that repo.

I do question however whether bundling a specific version of Django types in the extension is a good idea. My proposed "fix" will help me (working on Django 5.0), but will presumably cause problems for anyone working on an older version of Django. It would be great if the extension either:

a) didn't bundle a specific version of the django-stubs library at all, or; b) gave an option to disable the bundled version somehow

What do you think?

rchiodo commented 5 months ago

a) didn't bundle a specific version of the django-stubs library at all, or;

What do you think?

I'm not sure if you meant we ship no type stubs or not, but the reason we bundle stubs is so we have type information for things which are not typed. Without type stubs you'd get very different intellisense from Django.

Here's an example:

def view(request, question_id):
    question = get_object_or_404(Question, pk=question_id)
    question. # Completions here

In that code above, if we remove type stubs, the type of question is unknown. So intellisense returns no members.

That's because the actual function definition isn't typed at all. We can't actually figure out the type that might be returned:

def get_object_or_404(klass, *args, **kwargs):

Whereas the type stubs provide types:

def get_object_or_404(
    klass: type[_T] | Manager[_T] | QuerySet[_T], *args: Any, **kwargs: Any
) -> _T: ...

If those type stubs aren't installed, Django is a lot harder to write (well in my opinion). That's why we ship them for some very common packages.

We have to pick a version to ship, and sometimes that version is out of date. Django is a tough one because the type stubs we use are not the most commonly maintained version. We have some ideas on how to address that, but it's a bit far down our backlog.

The best option for most people to get actual intellisense is to ship the latest type stubs. Most public APIs don't change all that often and when they do they're mostly additive (meaning newer type stubs work for the most part with older installs)

b) gave an option to disable the bundled version somehow

What do you think?

This might be the right idea for some people (people using older versions I'd think). Right now you can do this yourself by deleting our shipping stubs. That doesn't work so well though because you have to keep deleting them every time you upgrade.

So maybe an option to disable type stubs?

simonwhitaker commented 4 months ago

If those type stubs aren't installed, Django is a lot harder to write (well in my opinion). That's why we ship them for some very common packages.

Honestly, having type stubs that are a couple of major versions out of date bundled with the extension makes Django pretty hard to write, too.

This might be the right idea for some people (people using older versions I'd think). Right now you can do this yourself by deleting our shipping stubs. That doesn't work so well though because you have to keep deleting them every time you upgrade.

Yeah, that's the problem. This just bit me again. My Pylance extension obviously updated itself, and I spent half an hour trying to remember how I fixed the erroneous type errors in my Django code the last time I saw them.

An option to disable type stubs would be really useful please.

simonwhitaker commented 4 months ago

Incidentally, I did chase this on the source repo (https://github.com/sbdchd/django-types/issues/255), but the issue hasn't been acknowledged.

simonwhitaker commented 4 months ago

@rchiodo Any update on this one? It keeps biting our development team. A way to disable built-in type stubs would be very helpful here.

rchiodo commented 4 months ago

Sorry it's not currently planned. It's on our backlog but will likely take some time to get to. I'd recommend just submitting the new types to the https://github.com/sbdchd/django-types/blob/main/django-stubs/utils/deprecation.pyi as a PR.

rik commented 3 months ago

https://github.com/typeddjango/django-stubs (which is more active than the https://github.com/sbdchd/django-types fork) has worked on pyright support (release notes, pull request). I don't know if it's ready but it would be nice if Pylance swapped eventually.

debonte commented 3 months ago

@rik, that PR only started running Pyright as part of the django-stubs test suite.

I believe https://github.com/typeddjango/django-stubs/issues/579 is the issue to watch for progress.

rik commented 3 months ago

The run-pyright jobs are green though (latest commit).

rchiodo commented 3 months ago

We should definitely test them out to see if we get better results than the stubs we were using.

rchiodo commented 3 months ago

It does look like even though the pyright run is green, it is flagging a lot of errors: https://github.com/typeddjango/django-stubs/actions/runs/10299251186/job/28506153092

rik commented 3 months ago

Ah right, when you expand the "Run pyright on the stubs" step.

anentropic commented 1 month ago

Is there some way I can disable the VS Code bundled django-types stubs in the meantime and instead use the django-stubs installed and configured in my project?

debonte commented 1 month ago

Is there some way I can disable the VS Code bundled django-types stubs in the meantime and instead use the django-stubs installed and configured in my project?

@anentropic, the stubs in your python.analysis.stubPath should override the bundled stubs.

anentropic commented 1 month ago

Ugh, my problem was an old version of django-stubs in my own venv...

I thought I had latest 5.1.0 and that type errors with ValuesQuerySet must come from django-types ...but instead I was still on 5.0.0, lacking the fix from ~5.0.1 that I wanted

apologies

Samuel-Therrien-Beslogic commented 3 weeks ago

It seems the bundled stubs are prioritized over those found in the venv.

See: Image

Then if I delete %USER%\.vscode\extensions\ms-python.vscode-pylance-2024.10.1\dist\bundled\stubs\django-stubs and restart the interpreter:

Image