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

Code should not be marked as unreachable in certain situations #473

Closed
cperezabo opened this issue Oct 10, 2020 · 4 comments
Closed

Code should not be marked as unreachable in certain situations #473

cperezabo opened this issue Oct 10, 2020 · 4 comments

Comments

@cperezabo
Copy link

Environment data

  • VS Code version: 1.50.0
  • Extension version (available under the Extensions sidebar): v2020.9.114305
  • OS and version: macOS 10.15.7
  • Python version (& distribution if applicable, e.g. Anaconda): CPython 3.8.2
  • Type of virtual environment used (N/A | venv | virtualenv | conda | ...): Remote Container, no venv.
  • Value of the python.languageServer setting: Pylance

Expected behaviour

The code should not be marked as unreachable when it is perfectly reachable

Actual behaviour

CleanShot 2020-10-09 at 12 15 53@2x

As you can see, the above code is marked as unreachable just because dangerous raises an exception.
I am justly testing that stack.capture() captures the exception. I think is not correct to mark the code as unreachable when there is a context manager that can handle the exception.

@karthiknadig karthiknadig changed the title Code should be marked as unreachable in certain situations Code should not be marked as unreachable in certain situations Oct 11, 2020
@karthiknadig karthiknadig transferred this issue from microsoft/vscode-python Oct 11, 2020
@erictraut
Copy link
Contributor

A type checker needs to be told that your context manager can handle exceptions. Most of them do not.

Type checkers (including mypy and pyright/pylance) have adopted a convention whereby if the __enter__ method is annotated to return a bool, it is assumed that the context manager handles exceptions. Refer to this thread for details.

@cperezabo
Copy link
Author

cperezabo commented Oct 11, 2020

Annotate __enter__ to return a bool? Doesn't make sense but I have tried it anyways and doesn't work.
And if it were, what about @contextmanager decorator? you can't annotate __enter__

Moreover, the same code has no issues in PyCharm

@erictraut
Copy link
Contributor

Sorry, the thread I referenced said __exit__, not __enter__. You're correct that this technique wouldn't work with the @contextmanager decorator. There's really no good solution here. If a static analyzer assumes that all context managers catch all exceptions, that will cause a different set of problems. Since most context managers do not catch all exceptions, it's better to assume that they don't unless informed otherwise.

Here's another simple workaround for your particular case. You can provide a None return type annotation for the dangerous method. Because your code doesn't provide a return type declaration, pylance is inferring the return type. And in this case, it's inferring a NoReturn because all of the code paths raise an exception.

def dangerous() -> None:
    raise FakeException

@cperezabo
Copy link
Author

You are right, annotating it with None does the job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants