Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decorated context manager tooltips don't display actual arguments #516

Closed
mivade opened this issue Oct 23, 2020 · 7 comments
Closed

Decorated context manager tooltips don't display actual arguments #516

mivade opened this issue Oct 23, 2020 · 7 comments
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@mivade
Copy link

mivade commented Oct 23, 2020

Environment data

  • Language Server version: 2020.10.2
  • OS and version: macOS 10.15.7
  • Python version (& distribution if applicable, e.g. Anaconda): 3.7.7 from a miniconda environment

Expected behaviour

Tooltips should show the actual arguments that a contextmanager-decorated function takes.

Actual behaviour

Arguments are rendered as *args: Any, **kwargs: Any (see screenshot).

Screen Shot 2020-10-23 at 3 45 43 PM

Logs

Background analysis message: getSemanticTokens
[BG(1)] parsing: /my-home/test.py (0ms)
[BG(1)] binding: /my-home/test.py (0ms)
Background analysis message: analyze
[BG(1)] analyzing: /my-home/test.py ...
[BG(1)]   checking: /my-home/test.py (1ms)
[BG(1)] analyzing: /my-home/test.py (1ms)
Background analysis message: getDiagnosticsForRange
Background analysis message: resumeAnalysis
[FG] parsing: /my-home/.vscode/extensions/ms-python.vscode-pylance-2020.10.2/dist/typeshed-fallback/stdlib/2and3/_typeshed/__init__.pyi [fs read 0ms] (10ms)
[FG] binding: /my-home/.vscode/extensions/ms-python.vscode-pylance-2020.10.2/dist/typeshed-fallback/stdlib/2and3/_typeshed/__init__.pyi (0ms)
[FG] parsing: /my-home/.vscode/extensions/ms-python.vscode-pylance-2020.10.2/dist/typeshed-fallback/third_party/2and3/typing_extensions.pyi [fs read 0ms] (0ms)
[FG] binding: /my-home/.vscode/extensions/ms-python.vscode-pylance-2020.10.2/dist/typeshed-fallback/third_party/2and3/typing_extensions.pyi (0ms)
[FG] parsing: /my-home/.vscode/extensions/ms-python.vscode-pylance-2020.10.2/dist/typeshed-fallback/stdlib/3/typing.pyi [fs read 0ms] (11ms)
[FG] binding: /my-home/.vscode/extensions/ms-python.vscode-pylance-2020.10.2/dist/typeshed-fallback/stdlib/3/typing.pyi (3ms)
[FG] parsing: /my-home/.vscode/extensions/ms-python.vscode-pylance-2020.10.2/dist/typeshed-fallback/stdlib/2and3/contextlib.pyi [fs read 0ms] (3ms)
[FG] binding: /my-home/.vscode/extensions/ms-python.vscode-pylance-2020.10.2/dist/typeshed-fallback/stdlib/2and3/contextlib.pyi (1ms)

Code Snippet / Additional information

from contextlib import contextmanager
from typing import ContextManager


@contextmanager
def foo(bar: int) -> ContextManager[int]:
    yield bar
@erictraut
Copy link
Contributor

The problem is that you're annotating the undecorated function incorrectly. The undecorated function is not a context manager. It becomes a context manager only once it is decorated.

There are a couple of workarounds:

  1. Remove the return type annotation entirely. Pylance can infer the return type in this case.
  2. If you want to include an explicit return type annotation, the correct type is either Generator[int] or Iterator[int].

@mivade
Copy link
Author

mivade commented Oct 24, 2020

You are technically correct (the best kind of correct!) that the annotation is incorrect. However neither workaround appears to work:

Screen Shot 2020-10-23 at 7 23 03 PM

Screen Shot 2020-10-23 at 7 23 42 PM

I didn't note it in the original report, but I only even tried ContextManager as the return type annotation because I originally noticed this when I didn't annotate the return type at all.

@erictraut
Copy link
Contributor

erictraut commented Oct 24, 2020

the best kind of correct!

That made me chuckle. :) Great Futurama reference.

The reason that the decorated function has this signature is due to the typeshed stub for contextlib.pyi. Here's how the decorator is declared:

def contextmanager(func: Callable[..., Iterator[_T]]) -> Callable[..., _GeneratorContextManager[_T]]: ...

Note that it returns a Callable that takes any parameters. (That's what the ... means.) This is why you're seeing a signature for foo that accepts an arbitrary collection of *args and **kwargs.

The contextmanager decorator was not not possible to annotate correctly prior to the introduction of PEP 612, which introduces a way to specify that the parameters for a decorated function should be unmodified while the return type is modified. Now that PEP 612 has been ratified, you can use the ParamSpec to properly annotate this decorator.

_PS = ParamSpec("_PS")
def contextmanager(func: Callable[_PS, Iterator[_T]]) -> Callable[_PS, _GeneratorContextManager[_T]]: ...

Pyright (which is the type checker used within Pylance) already has full support for PEP 612, but some of the other type checkers have not yet added this support. For that reason, the typeshed stubs have not yet incorporated ParamSpec into the stdlib stubs. Once they do, this will work as you would expect.

If you would like, you can file a bug or submit a PR in the typeshed repo to encourage the maintainers to make these changes. Pylance/pyright regularly update the typeshed stubs, so we will automatically pick up any improvements.

@mivade
Copy link
Author

mivade commented Oct 24, 2020

@erictraut thanks for the detailed explanation! I will note that PyCharm handles decorated context managers correctly (from a usability perspective), but I'm fairly certain that they use their own tooling which explains why the behavior is different.

Please feel free to close this if you think it's not relevant given that the source of the problem is in another project. Thanks again for your help!

@erictraut
Copy link
Contributor

The latest typeshed stubs for contextlib now make use of ParamSpec (PEP 612), which means that @contextmanager is now properly annotated! This bug will therefore be addressed with the next release of pylance.

@erictraut erictraut added bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version and removed waiting for upstream Waiting for upstream to release a fix labels Jun 16, 2021
@mivade
Copy link
Author

mivade commented Jun 16, 2021

Awesome, thanks!

@bschnurr
Copy link
Member

This issue has been fixed in version 2021.6.2, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/main/CHANGELOG.md#202162-16-june-2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version
Projects
None yet
Development

No branches or pull requests

4 participants