`__exit__` signature detected incorrectly for IOBase

[viraptor]

Bug Report

When using a class inheriting from io.IOBase, the signature for __exit__ is detected incorrectly and prevents annotation of correct code.

To Reproduce

class Foo(io.IOBase):
  def __exit__(self) -> Literal[False]:
    return False

Expected Behavior

Code checks successfully. (__exit__ is expected to return a bool)

Actual Behavior

arpy.py:202: error: Return type "Literal[False]" of "__exit__" incompatible with return type "None" in supertype "IOBase"

Your Environment

• Mypy version used: mypy 0.961 (compiled: yes)
• Mypy command-line flags: --strict
• Mypy configuration options from mypy.ini (and other config files):
• Python version used: Python 3.9.9
• Operating system and version: Fedora 35

[AlexWaygood]

__exit__ is expected to return a bool

I think this is a slight misreading of the documentation that you link to here (the docs are admittedly quite subtle here, and could perhaps be improved). The docs say:

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.

I.e., the docs are quite careful here to talk about returning "a true value/a false value"; they quite carefully avoid saying "__exit__ must return either True or False".

"True value" is often used as shorthand in the CPython documentation to mean "any obj where bool(obj) evaluates to True". (Again, we can debate whether that results in clear documentation, but that's a CPython issue rather than a typeshed or mypy issue.)

---

I think the stubs here are probably correct at the moment. We did change the return type of IOBase.__exit__ relatively recently, though, in #7471. Could you take a look at the rationale given there, and see if it makes sense to you?

[viraptor]

Yeah, I should've been more precise here, it's true that the returned values can be truthy/falsey instead of bool. To be honest, I don't agree with making it an explicit None though. IOBase happens to return None, but I see the context contract as more important. Otherwise you're preventing anything implementing iobase from using this functionality with correct typing. For example if I wanted to implement something in a sub-sub-class of iobase that creates the context with ignoreBrokenPipe, or ignoreDroppedConnection parameter, I couldn't do that with __exit__.

In other words, I think in this case the signature limits what you can implement more than the language ref does, and that doesn't seem right.

[AlexWaygood]

For example if I wanted to implement something in a sub-sub-class of iobase that creates the context with ignoreBrokenPipe, or ignoreDroppedConnection parameter, I couldn't do that with __exit__.

That sounds reasonable, but it's not something I've ever felt the need to do — can you give a minimal example of some code where you'd like to suppress an exception, but mypy currently raises a false-positive error?

Cc. @JelleZijlstra also, given his thoughts in https://github.com/python/typeshed/pull/7471#issuecomment-1064135267

[JelleZijlstra]

I still believe that a subclass of IOBase that suppresses exceptions is enough of a contract violation that it's reasonable for type checkers to give an error.

[viraptor]

It's a big base class, so I won't write out a whole runnable example, but one would be to have a custom pipe reader which expects the pipe to disappear and exits successfully ignoring that exception. Either way, I'm purely after type checking from mypy, not other code correctness checks. The contract for context manager's exit is a falsy/truthy return value, and IOBase is documented as being a context manager. Even though IOBase only returns None, I expect the wider type rather than guessing what the subclasses will / won't do - even if it's completely misguided.

[srittau]

Shouldn't we at least add Literal[False] to the return type of IOBase.__exit__()?

[AlexWaygood]

The contract for context manager's exit is a falsy/truthy return value, and IOBase is documented as being a context manager.

But it is true that the vast majority of file-descriptor context managers have a much more specific contract than context managers in the abstract: namely, the standard contract for file descriptors is that they will always close the file, and then let any exceptions that occurred bubble up after the file has been closed.

[AlexWaygood]

I still believe that a subclass of IOBase that suppresses exceptions is enough of a contract violation that it's reasonable for type checkers to give an error.

I agree with @JelleZijlstra here; I don't think we should make any change