imaplib: Avoid an unnecessary Any in CommandResults (#3654)

[dkg]

This addresses #3654

[JelleZijlstra]

Could you fix the CI failure? Also, this change is problematic because it introduces a Union in a return type (as mentioned under https://github.com/python/typeshed/blob/master/CONTRIBUTING.md#conventions), so it may be better to stick with Any.

[dkg]

I've just pushed a fix for the CI, if i'm reading it right.

I'm not convinced from reading the discussion in python/mypy#1693 that Any is a significant improvement over Union as a return type -- i actually found useful improvements in some code i'm writing that uses imaplib because of trying to be more strict about the return type here.

But i'm happy to leave this decision up to the typeshed project -- if you think it's not an improvement, feel free to close this ticket. But if you do, i'd appreciate a clearer explanation of why Union is actively worse. If there are tools that treat Unions in the "unsafe" way, surely they'll treat Any in a way that's just as unsafe, no?

[srittau]

If I understand the documentation and the quick looks at the code correctly, each command always returns either bytes or Tuple[bytes, bytes], not one in some cases and the other in other cases, correct? In that case we should just have two different CommandResults types, for these cases. Using an union forces users to always use isinstance checks, even if they are perfectly aware that, e.g. list() always returns a list of simple bytes.

[dkg]

@srittau, i like the idea that you're suggesting, and it might work for some functions, but in practice that's not the behavior that i'm seeing from imaplib today.

For example, in response to uid('FETCH', '1376108', '(UID BODY.PEEK[])') (fetching 1 message via uid), i see two objects returned:

• (b'* 1 FETCH (UID 1376108 BODY[] {3531}', xxx) (this tuple contains the requested object itself as the second member, xxx)
• b')' (this bytes object represents the close parenthesis after the body object)

So maybe we can mark some functions without the union, but i'm not sure that all of them will work that way.

[srittau]

I looked into it a bit more. For some commands the return value is well known. For example, starttls() will always return ("OK", [None]) or raise an exception, and login() will always return ("OK", [...]), where ... is the byte string returned by the server. Other commands rely on the data returned by the server, e.g. fetch(), which either returns a List[None] with one element or a List[Union[bytes, Tuple[bytes, bytes]]]. In the latter case I think it makes sense to return the list as an union, since the caller can not rely on the data being any one type and must make a check to be safe. But I don't think we can apply this to CommandResults generally, because this will break existing type-safe code.

I opened #3670 to make a few example changes based on this.

[srittau]

I am going to close this, based on the discussion above. Thank you for the contribution!