I post this under Typing as there doesn’t seem to be a tools-specific category.
Context managers which suppress exceptions raised within their body can lead to unexpected errors, which current tooling has few to no ways of detecting.
Given scenario:
from contextlib import suppress
def maybe_throws(throw: bool) -> None:
# not all codepaths lead to an exception, so it's not (...) -> NoReturn
if throw:
raise Exception
with suppress(Exception):
maybe_throws(True)
foo = 123
print(foo)
Result of testing in pyright playground:
A report is emitted, stating that foo at print can be unbound. However, removing the call to maybe_throws doesn’t get rid of the warning, suggesting that pyright special cases suppress and simply emits warnings for any variable definitions originating from within the context manager’s body.
Running the code in Mypy’s playground produces no warnings.
This problem caught me off-guard while working with anyio.
The linter issues no warnings for the following code:
import anyio
async def main():
with anyio.move_on_after(5):
await anyio.sleep_forever() # do something to exceed time limit
foo = 123
print(foo)
if __name__ == "__main__":
anyio.run(main)
I’d like to discuss what could be done to give linters a deterministic solution in catching this problem, in analogous way to how
I thought of a special form akin to TypeGuard, like Supresses or Supressing. But the usage leaves a lot to ask for:
class CtxManager:
def __enter__(self) -> Supresses[Exceptions?, ReturnType?]:
# should it be on enter or exit?
...
def __exit__(self, *exc_spec) -> Suppressing[what?]:
...
@contextmanager
def supress(*exceptions: type[BaseException]) -> Supressing[abc.Iterator[None]]:
# annotation only valid in conjunction with contextmanager
# what'd prevent one from using it without the decorator?
try:
yield
except exceptions:
pass
Are you asking for more or less warnings? The supress example you provide should provide a warning. Type checkers are not able to detect what exceptions could happen when, so they have to assume than an exception happens as soon as possible (if they want to care at all, mypy appears to not care). Adding exception declarations is a highly controversial topic discussed at length in other threads.
To add an extra warning, type checkers could try to figure out if __exit__ can ever return a truthy value, and if so, treat the body of the with statement as potentially not executed. Anything more granular [1] than that would require code path analysis that currently doesn’t exists and isn’t planned (although you could ofcourse start a PEP. But expect a lot of pushback).
e.g. your suggestion of listing Exception types ↩︎
No, that’s not what pyright is doing. Pyright assumes that the evaluation of any expression or statement found within a try block could potentially generate an exception, and it creates edges in the code flow graph from those locations to the end of the context manager block. Then it performs code flow analysis to determine whether each variable is assigned a value along all possible code flow paths.
Your example includes the statement foo = 123. It’s possible (although admittedly very unlikely) that the runtime evaluation of 123 could raise an exception. If that occurs, then the exception will be caught and suppressed, and foo will be unbound.
Pyright doesn’t try to distinguish which expressions or statements could possibly result in an exception because almost any expression or statement can theoretically do so.
Pyright has no special-case knowledge of suppress or any other context manager. It simply looks at the return result of the __exit__ method to determine whether the context manager suppresses exceptions. In this regard, the behavior of pyright and mypy should be the same.
The difference you’re seeing above between contextlib.suppress and anyio.move_on_after is that the form has an __exit__ method that returns bool and the latter returns bool | None.
If an exception is supplied, and the method wishes to suppress the exception (i.e., prevent it from being propagated), it should return a true value. Otherwise, the exception will be processed normally upon exit from this method.
Yup, I read it, but it doesn’t explain why going from bool to bool | None makes the typecheckers stop treating the manager as suppressing (or at least, stop emitting warnings about potential unbounds).
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)