Add mypy/static type annotations

[warsaw]

It seems reasonable to add type annotations since aiosmtpd is a Python 3 application/library and type annotations can provide better documentation to users. With the use of a tool like mypy, those assertions can even be tested.

We'll pretty quickly run into problems though since our Handler classes are duck typed, and mypy does not yet support structural typing. Meaning, examples such as Controller.__init__()'s handler argument does not have to be a subclass of anything; it simply needs to support some methods. Handlers are even weirder than straight up duck types too, because handle_*() methods are optional.

Similarly, SMTP methods smtp_*() are optional too.

Some things that might help include:

• PEP 544 defines "Protocols" that can be used to structurally type arguments;
• Techniques such as those described in this blog post;
• Just using an Any type for the duck typed arguments;
• Require that handlers derive from a common base class. I don't like this solution much as it was essentially rejected for an earlier PR, and it would be an API change requiring a major version number bump

[emmatyping]

Protocols just landed in Mypy, so you could use Mypy from git (Zulip does this), you can take advantage of Protocols today once they are in typing extensions or mypy extensions! Most of the protocol related docs are in https://mypy.readthedocs.io/en/latest/class_basics.html. You will also want to track https://github.com/python/mypy/issues/3838

[pepoluan]

• PEP 544 defines "Protocols" that can be used to structurally type arguments;

This sounds good. Since we (1) indirectly pulled typing-extensions courtesy of atpublic, and (2) drop 3.5 because of, again, atpublic, we can start implementing Protocols.

Edit: However, the fact that handle_* methods and smtp_* methods are optional and PEP 544 explicitly rejected support for annotating optional members, means that annotating the handler will be ... challenging.

[pepoluan]

Just for kicks, I tried running mypy, and the result is ... not beautiful.

There are lots of instances where mypy couldn't infer the type of a variable, I had to pepper the code with lots of asserts.

[pepoluan]

It seems mypy does not do inference, it's 100% static.

In the code, there are lots of occasions where a name is reused but the content changes.

If we want to implement type checking, I suggest we use a type checker that does inference.

[emmatyping]

Mypy does do static type inference. For example:

a = {}
....
print(a['test'])

We don't know the best way to safely type a, so mypy asks the user what to do.

But if you do something like:

a = {}
a['test'] = 1
reveal_type(a) # main.py:3: note: Revealed type is 'builtins.dict[builtins.str, builtins.int]'

We can statically infer types. Also keep in mind that runtime and static types are not always 1-to-1.

Also for redefinition of types, there is --allow-redefinition.

I went ahead and ran mypy and it looks to me like most of the errors have to do with code paths potentially being None.

[pepoluan]

I went ahead and ran mypy and it looks to me like most of the errors have to do with code paths potentially being None.

That is actually my problem with mypy; it does not understand the context.

Take, for instance, line 665 of smtp.py

mypy will complain:

aiosmtpd/smtp.py:665: error: Item "_Missing" of "Union[_Missing, bytes]" has no attribute "split"

it does not understand that at that point, it is not possible for the loginpassword to be of type _Missing.

And a lot of mypy's complaints about "potentially being None", again is due to mypy not having the ability to do contextual inference. To make the code pass mypy I have to pepper the code with lots of assert isinstance()

pytype, on the other hand, analyzes the context and does not complain about that line. (It does complain about other things, but of totally different nature, and not as finicky as mypy).

Neither are perfect, but I find mypy's over-fastidiousness to be irritating and not worth the effort to fix.

[pepoluan]

Okay so fixed a bunch of annotations thanks to pytype. Still on the fence w.r.t. mypy, but at least we're now on our way towards implementing proper type hinting in #202 (part of 1.3 milestone, btw)

[pepoluan]

We'll pretty quickly run into problems though since our Handler classes are duck typed

In 2.0, if Proposal No. 13 in #229 is agreed to, we can kinda enforce this per method.

[waynew]

I'm definitely more of a fan of duck typing, so the structural/protocol typing would be :+1:

[pepoluan]

In an ongoing attempt to fully annotate this package, I've submitted PR #259

[pepoluan]

Huh. rebasing + force pushing apparently causes LOTS of "referenced this issue" 😆