false positive `bad-context-manager` for overloaded `__exit__`

[jorenham]

Describe the Bug

from types import TracebackType
from typing import overload

class CM:
    def __enter__(self, /) -> None: ...
    @overload
    def __exit__(self, tp: None, ex: None, tb: None) -> None: ...
    @overload
    def __exit__(self, tp: type, ex: BaseException, tb: TracebackType) -> None: ...
    def __exit__(self, tp: type | None, ex: BaseException | None, tb: TracebackType | None) -> None: ...

with CM():
    pass

ERROR sandbox.py:12:6-10: Cannot use `CM` as a context manager [[bad-context-manager](https://pyrefly.org/en/docs/error-kinds/#bad-context-manager)]
  No matching overload found for function `CM.__exit__`
  Possible overloads:
  (tp: None, ex: None, tb: None) -> None [closest match]
  (tp: type, ex: BaseException, tb: TracebackType) -> None

I realize that that this is tighter than how e.g. typeshed annotates __exit__, but I believe that it's correct like this.

Sandbox Link

https://pyrefly.org/sandbox/?project=N4IgZglgNgpgziAXKOBDAdgEwEYHsAeAdAA4CeS4ATrgLYAEALqcfHRDcbpQ3QCqWoAxjGxCA1r2YwAOujDV6TYhHQBzNhy49cANxiUouVJlmzBUVHDh0AwgFlEsus7qYYYOgH1PMdA33eABRwMFBgADR0APQAlHQAtAB8dAByuOgwiHSEOU4uAAK6%2BobGec5uHt4w%2BBAMQSFhkQzEWWkZkdWt6TBN2F0ZcUmp3Vk5hGV0hXoGRiboLq7uXj41dZ7BoRGMLYxSHfhZAEKWMACi%2BMLEDBDpvVn8QiLikiyDyW2Z2bnzLhXL1bV6psmjslDA6AAfYbtOidOjHELnS7XdKQ6E9Rh9PgCYSiQQSKRoj5vdGjb6yADutQAFrY7IEYo4fs5iJY4LIQOEQABXa5QOAkciIEAAYjoAFU%2BbVSHQwNz0IIUeh2ehZH8wFwaKg6uhuTRsPpAgc2H4SXAGJQmQtKDAGNzKPMwNIQCk9QbLXRgPgAL7OjnerlCa56ABi0BgFDQWDwRDIIG9QA&version=3.13

(Only applicable for extension issues) IDE Information

No response

[connernilsen]

Hey @jorenham, three reports in one day! Thanks for your contributions.

That does seem like a more appropriate signature, given that if there is an exception, all three of those would be filled in. I'm not sure if there are any cases where all three wouldn't be some or None.

With that said, the official typeshed spec has it as

    def __exit__(
        self,
        exc_type: type[BaseException] | None,  # noqa: PYI036 # This is the module declaring BaseException
        exc_val: BaseException | None,
        exc_tb: TracebackType | None,
        /,
    ) -> None: ...

so the overloads you mention

    @overload
    def __exit__(self, tp: None, ex: None, tb: None) -> None: ...
    @overload
    def __exit__(self, tp: type, ex: BaseException, tb: TracebackType) -> None: ...

don't actually satisfy it (technically you could call __exit__(Exception, None, None), which is a valid call, but your overloads don't allow for it).

@ndmitchell, @stroxler in this case, would we want to adjust our interpretation of it (are we doing custom patches to bundled typeshed), is this something we'll need to fix upstream in typeshed, or should we leave things as they are?

[jorenham]

With that said, the official typeshed spec has it as

That links to memoryview, but from the error message I wasn't able to tell which one of the typeshed __exit__ signatures this was referring to. I wouldn't mind submitting a typeshed PR to fix that, but I'd have to know which one pyrefly is using as "ground truth" here.

[jorenham]

three reports in one day!

I guess scipy/scipy-stubs#902 is to blame for that ;)

[yangdanny97]

@jorenham

The exception is coming from here:

pyrefly/pyrefly/lib/alt/solve.rs

Line 1153 in 101eb13

fn context_value_exit(

We try to call your __exit__ with 3 arguments of type[BaseException] | None, BaseException | None, and TracebackType | None, and get an overload matching error.

Mypy and Pyright don't seem to error for this, because they don't actually check the types of the 3 parameters. They only error if the function cannot take 3 arguments. For example, here's what Mypy gives for the following signatures:

def __exit__(self, tp: int, ex: int, tb: int) -> None: ...  # OK
def __exit__(self, tp: int, ex: int, tb: int, z: int = 0) -> None: ...  # OK
def __exit__(self, tp: int, ex: int, tb: int, z: int) -> None: ...  # Error
def __exit__(self, tp: int, ex: int) -> None: ...  # Error

Do you think we're being too strict here? It would be very easy to change our logic to only check the number of arguments by calling __exit__ with three Any arguments.