Bug: false positive with --warn-unreachable

[jcugat]

Please provide more information to help us understand the issue:

• Are you reporting a bug, or opening a feature request?
  Bug

• Please insert below the code you are checking with mypy,
  or a mock-up repro if the source is private. We would appreciate
  if you try to simplify your case to a minimal repro.

from contextlib import suppress
from typing import Dict


def foo(blah: Dict[str, str]):
    with suppress(KeyError):
        return blah["blah"]
    return blah["default"]


print(foo({"default": "default"}))

• What is the actual behavior/output?

$ mypy --warn-unreachable test.py
test.py:8: error: Statement is unreachable

• What is the behavior/output you expect?
  No error, since line 8 is reachable.

• What are the versions of mypy and Python you are using?

$ pip freeze | grep mypy
mypy==0.720
mypy-extensions==0.4.1
$ python -V
Python 3.6.8

• What are the mypy flags you are using? (For example --strict-optional)
  --warn-unreachable

Maybe related to #7204 and #7207

[JukkaL]

This seems like a bug in how mypy calculates reachability of with statements.

cc @Michael0x2a

[Michael0x2a]

@JukkaL -- I'm not sure if there's a simple solution for this one either. Basically, we could try adjusting mypy so it (correctly) treats all with statements as if they were the body of a try-finally block, which would mean statements after the with would start to be considered reachable.

But that ends up causing "Missing return statement" errors for code like the following:

def foo() -> int:
    with open("foobar.txt", "r") as f:
        return 3

More generally, there doesn't seem to be a clean way of distinguishing between context managers that suppress exceptions vs ones that don't.

It'd be nice to find some way of encoding this distinction at the type-system level (for example, by taking advantage of the proposed Annotated type?), but maybe for now we could get by just hard-coding a list of these exception-swallowing context managers within mypy?

[JelleZijlstra]

You could in theory get this out of the return type of the context manager's __exit__: if it returns None, you know that it doesn't swallow exceptions, and if it returns bool or Optional[bool], you know that it may. Of course that's not sufficient, because you don't in general know the concrete contextmanager class involved (typing.ContextManager.__exit__ is annotated as returning Optional[bool]).

One possible pragmatic solution is to assume that contextmanagers don't swallow exceptions, unless they are annotated as returning bool (but not Optional[bool], because that's what typing.ContextManager returns and most general context managers don't swallow exceptions). We'd then add an explicit -> bool return type to contextlib.suppress.__exit__ in typeshed.

[JukkaL]

@JelleZijlstra I like your idea! The implementation sounds pretty simple and it seems pretty logical (though it might be a little surprising). We'd probably need to document this in typeshed at least, and perhaps also in mypy docs.

@Michael0x2a What do you think about Jelle's proposal?

[The-Compiler]

I hope you don't mind me digging up an old issue, but it seems like this fits in nicely here - unless I'm missing something, there's no way to annotate this if using @contextlib.contextmanager, correct?

As an example:

import contextlib
from typing import Iterator

@contextlib.contextmanager
def swallow_exceptions() -> Iterator[None]:
    try:
        yield
    except Exception:
        # ...
        pass

def test() -> int:
    with swallow_exceptions():
        return 1
    return 2

results in:

unr.py:15: error: Statement is unreachable  [unreachable]
        return 2

[tjstum]

A workaround that you could consider:

import contextlib
from typing import TYPE_CHECKING, Iterator

if TYPE_CHECKING:
    class swallow_exceptions:
        def __init__(self) -> None: ...
        def __enter__(self) -> None: ...
        def __exit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> bool: ...
else:
    @contextlib.contextmanager
    def swallow_exceptions() -> Iterator[None]:
        try:
            yield
        except Exception:
            # ...
            pass