Skip to content

Commit

Permalink
fix contextmanager decorator treating all context managers as if th…
Browse files Browse the repository at this point in the history
…ey can suppress exceptions when that's no longer the case since python 3.7
  • Loading branch information
DetachHead committed Feb 25, 2024
1 parent 75f969a commit 7272176
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 9 deletions.
25 changes: 18 additions & 7 deletions packages/pyright-internal/src/tests/samples/withBased.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import contextlib
from contextlib import AbstractContextManager, contextmanager
from types import TracebackType
from typing import Iterator, Literal
from typing import Iterator, Literal, Iterator

from typing_extensions import assert_never

class BoolOrNone(contextlib.AbstractContextManager[None]):
class BoolOrNone(AbstractContextManager[None]):
def __exit__(
self,
__exc_type: type[BaseException] | None,
Expand All @@ -18,7 +18,7 @@ def _():
raise Exception
print(1) # reachable

class TrueOrNone(contextlib.AbstractContextManager[None]):
class TrueOrNone(AbstractContextManager[None]):
def __exit__(
self,
__exc_type: type[BaseException] | None,
Expand All @@ -33,7 +33,7 @@ def _():
print(1) # reachable


class FalseOrNone(contextlib.AbstractContextManager[None]):
class FalseOrNone(AbstractContextManager[None]):
def __exit__(
self,
__exc_type: type[BaseException] | None,
Expand All @@ -48,7 +48,7 @@ def _():
print(1) # unreachable


class OnlyNone(contextlib.AbstractContextManager[None]):
class OnlyNone(AbstractContextManager[None]):
def __exit__(
self,
__exc_type: type[BaseException] | None,
Expand All @@ -60,4 +60,15 @@ def __exit__(
def _():
with OnlyNone():
raise Exception
print(1) # unreachable
print(1) # unreachable


@contextmanager
def foo() -> Iterator[None]: ...


with foo():
a = 1

# no reportPossiblyUnboundVariable since _GeneratorContextManager cannot suppress exceptions anymore as of python 3.7
print(a)
19 changes: 17 additions & 2 deletions packages/pyright-internal/typeshed-fallback/stdlib/contextlib.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ from _typeshed import FileDescriptorOrPath, Unused
from abc import abstractmethod
from collections.abc import AsyncGenerator, AsyncIterator, Awaitable, Callable, Generator, Iterator
from types import TracebackType
from typing import IO, Any, Generic, Protocol, TypeVar, overload, runtime_checkable
from typing import IO, Any, Generic, Literal, Protocol, TypeVar, overload, runtime_checkable
from typing_extensions import ParamSpec, Self, TypeAlias

__all__ = [
Expand Down Expand Up @@ -67,8 +67,23 @@ class _GeneratorContextManager(AbstractContextManager[_T_co], ContextDecorator):
if sys.version_info >= (3, 9):
def __exit__(
self, typ: type[BaseException] | None, value: BaseException | None, traceback: TracebackType | None
) -> bool | None: ...
) -> Literal[False]: ...
elif sys.version_info >= (3, 7):
def __exit__(
self, type: type[BaseException] | None, value: BaseException | None, traceback: TracebackType | None
) -> Literal[False]: ...
else:
# python 3.7 fixes an issue where generators could incorrectly suppress StopIteration exceptions
# (see https://peps.python.org/pep-0479/). while it's still technically possible to craft a generator that
# does, it's an extreme edge case that you need to go out of your way to cause. so we change the return type
# to `Literal[False]` to prevent it from assuming every context manager can suppress exceptions. the only
# reason this isn't an issue in upstream pyright/mypy is because they both "fix" this problem in the
# stupidest way imaginable, by special-casing `bool | None` to mean `Literal[False]` instead.

# the tradeoff with my fix is that python <=3.7 users will experience plenty of annoying errors caused by
# basedpyright being overly strict and assuming variables set inside context managers can be unbound in case
# the exception gets suppressed. i could do some special casing to address this, but python 3.6 is deprecated
# anyway and you should just update to a supported version of python instead.
def __exit__(
self, type: type[BaseException] | None, value: BaseException | None, traceback: TracebackType | None
) -> bool | None: ...
Expand Down

0 comments on commit 7272176

Please sign in to comment.