Annotations for `nacl.bindings.crypto_box`

[DMRobertson]

This introduced a few errors in nacl.public which I wasn't sure how to suppress. AFAICS both will raise a TypeError at run-time, so I suppressed mypy's errors. I wasn't completely satisfied with this; I very much welcome input here! (cc @Dreamsorcerer)

Box._shared_key

The first error mypy reported was

src/nacl/public.py:193: error: Incompatible types in assignment (expression has type "None", variable has type "bytes")  [assignment]

referring to self._shared_key. I resolved this by marking _shared_key: Optional[bytes] to cover both cases in the constructor. But doing this introduced two new errors:

src/nacl/public.py:236: error: Argument 3 to "crypto_box_afternm" has incompatible type "Optional[bytes]"; expected "bytes"  [arg-type]
src/nacl/public.py:278: error: Argument 3 to "crypto_box_open_afternm" has incompatible type "Optional[bytes]"; expected "bytes"  [arg-type]

Which correspond to the annotations I added to crypto_box_afternm and crypto_box_open_afternm's k argument. Both docstrings say that k should be a bytes instance, and go on to inspect len(k). Therefore, calling this with k=None will raise a TypeError.

SealedBox._private_key

The second of the original two errors was

src/nacl/public.py:382: error: Argument 3 to "crypto_box_seal_open" has incompatible type "Optional[bytes]"; expected "bytes"  [arg-type

because self._private_key may be None. In this instance there is an explicit ensure check in crypto_box_seal_open which will raise a TypeError.

I also adjusted the SealedBox docstring to match its __init__ method, which appears to be the convention (judging by the other classes in this file).

[DMRobertson]

I also noticed that bytes(Box(None, None)) is None, which surprised me and might cause further typechecking pain down the line. (See https://github.com/DMRobertson/pynacl/blob/8a0cf857af989cd4b25019542da5cfafe945419f/src/nacl/public.py#L180-L196)

I think the intention is that one either creates a box by providing both keys to __init__; or else one uses decode. If everyone plays by those rules, then _shared_key is never None. But I'm not quite sure how to convince mypy of this!

[Dreamsorcerer]

Box._shared_key

Am I right in thinking that None is only supported for the classmethod at line 201 (looks like you have the same conclusion in your last comment)? Which then sets the shared_key separately. So, the best solution would be to remove the support for None and refactor that classmethod. I can't see any other possible use for it, it just looks like a hack to make that classmethod work.

Not sure I have a good suggestion on how to make that work correctly. Maybe adding support for a shared_key argument to __init__()?. But, at worst, we could add annotations to __init__() (without None), remove the else (knowing that we will set it in the classmethod), and add a type: ignore in the classmethod (where the actual hack is happening).