Use a Protocol as the argument for io.TextIOWrapper

[JelleZijlstra]

I ran into a false positive where gzip.GzipFile couldn't be assigned
to io.TextIOWrapper.

[JelleZijlstra]

The Protocol is based on reading https://github.com/python/cpython/blob/main/Modules/_io/textio.c, specifically how it uses self->buffer.

[srittau]

Considering the (valid) primer hits: The documentation says: "A buffered text stream providing higher-level access to a BufferedIOBase buffered binary stream." Would it make sense to use that type? I didn't investigate yet.

[JelleZijlstra]

Looks like gzip.GzipFile inherits from BufferedIOBase, so that would at least fix my problem. I'll see what primer thinks.

[JelleZijlstra]

OK, that's worse for mypy-primer.

[srittau]

Looking at the primer hits:

1. pytest no 1.: Edit: This genuine problem is actually pointed out in a source comment: "TextIOWrapper doesn't expose a mode, ..." so I think an explicit cast is fine.
2. pytest no 2: A real problem. The result of tempfile.TemporaryFile(), which is an typing.IO object, is passed to TextIOWrapper. We should probably tighten our return type. (Tighten tempfile() return types #10232)
3. pytest no 3: Test code, ignoring.
4. pylint: The result of TextIOWrapper.detach() is passed in, which is not defined in our stubs. The base class's TextIOBase.detach() returns a BinaryIO. In the implementation TextIOBase is not implemented, but TextIOWrapper is, hinting that we could tighten the return type in the stubs as well. Edit: This problem has disappeared by typing detach().
5. rich: Passes in a custom _Reader class, which derives from RawIOBase. Points to a genuine problem, at least going by the Python docs.
6. urllib3: Needs to update its explicit type annotation.
7. black: Passes in sys.stdout.buffer. Would need an assertion, which can't hurt in this case.
8. cwltool no 1: See black.
9. cwltool no 2: Would need to update its cast, but potentially points to a real problem.

Overall I would say that we should fix the other two problems in typeshed first and then do this change. It's a bit disruptive, but I think overall worth it for finding more potential problems and further cleanup of the I/O types.

[srittau]

Actually, the TemporaryFile() annotation were still incorrect. #10284.

[srittau]

To recap the primer output:

Category 1: Passes in incompatible type

These seem to work at runtime, but still I think that a type checker pointing out that this could be problematic is actually an advantage. Explicitly casting away the problem would acknowledge this in the implementation.

• pytest no 2 (src/_pytest/capture.py:466)
• rich

Category 2: Passes in `sys.stdout.buffer

These have the usual "std" types problems. Using an explicit assert on sys.stdout's type would be a good idea in any case.

• black
• cwltool no 2 (cwltool/main.py)

Category 3: Acknowledged as problematic in the source code

I see adding a cast here as acceptable.

• pytest no 1 (src/_pytest/capture.py:174)

Category 4: Needs to update its explicit type annotation/cast

• urllib3
• cwltool no 1 (cwltool/cwlprov/writablebagfile.py)

As written before, despite the primer output I think this change is worthwhile as is, for the reasons outlined above.

[JelleZijlstra]

I think I'd prefer to go back to a protocol, even if the documentation explicitly calls for a BufferedIOBase. The runtime implementation doesn't care about the nominal type, so I think we should also use a protocol.

Approval was for different solution.

[srittau]

I consider this solution worse. For example, the primer hit at src/_pytest/capture.py:546 is a valid use that's now rejected. While this particular instance can be fixed by adjusting the protocol, we can't fix all valid uses without the protocol actually mimicking BufferedIOBase.

The best solution would be to make TextIOWrapper generic, bound to the protocol and defaulting to BufferedIOBase, but this can't be implemented, until we have type var defaults.

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

pytest (https://github.com/pytest-dev/pytest)
+ src/_pytest/capture.py:179: error: Returning Any from function declared to return "str"  [no-any-return]
+ src/_pytest/capture.py:179: error: "_TextIOWrapperBuffer" has no attribute "mode"  [attr-defined]
+ src/_pytest/capture.py:471: error: Argument 1 to "EncodedFile" has incompatible type "FileIO"; expected "_TextIOWrapperBuffer"  [arg-type]
+ src/_pytest/capture.py:471: note: Following member(s) of "FileIO" have conflicts:
+ src/_pytest/capture.py:471: note:     name: expected "str", got "int | str | bytes | PathLike[str] | PathLike[bytes]"
+ src/_pytest/capture.py:551: error: Too few arguments for "read" of "_TextIOWrapperBuffer"  [call-arg]
+ src/_pytest/capture.py:554: error: Incompatible return value type (got "Buffer", expected "bytes")  [return-value]

streamlit (https://github.com/streamlit/streamlit)
- lib/tests/streamlit/web/server/component_request_handler_test.py:176:38: error: Argument 1 to "TextIOWrapper" has incompatible type "str"; expected "IO[bytes]"  [arg-type]
+ lib/tests/streamlit/web/server/component_request_handler_test.py:176:38: error: Argument 1 to "TextIOWrapper" has incompatible type "str"; expected "_TextIOWrapperBuffer"  [arg-type]

[JelleZijlstra]

Just wanted to confirm the primer output was still the same, closing in favor of #11420.