Leandro Lima <lean...@limaesilva.com.br> added the comment:

In my view, this function signature changed too silently. Even using static 
type checkers, I could only find about this compatibility breaking change when 
actually running the code.

If I understand well the reason it was done this way, digestmod needed to 
become a mandatory argument, but this couldn't be done without changing the 
order between msg and digestmod in the function's signature.

In my view, the two other ways this could be solved were:
1. hmac.new(key: Union[bytes, bytearray],
            digestmod: str,
            msg: Union[bytes, bytearray, None] = None)
2. hmac.new(key: Union[bytes, bytearray],
            *,
            digestmod: str,
            msg: Union[bytes, bytearray] = None)

If the signature of the function changed to reflect digestmod becoming 
mandatory, then static code checkers could catch a misuse of the function.

Now, suppose that we're dealing with someone that doesn't use static code 
analysis and a legacy signature used in some code:

hmac.new(b"key", b"msg")

- In option (1), we'd be passing b"msg" as the digestmod argument when the 
original intention was to pass it as the msg argument. But since both have 
disjoint expected types, this mistake would be rejected because passing the 
wrong type would lead to a TypeError
- In option (2) we'd be making clear that from now on, both msg and digestmod 
would only be specifiable as keyword arguments and an inadvertent use of the 
old signature would also lead to a TypeError.

Given that it seems a rather safe signature change (that is: there's no chance 
someone would be able to use the old signature with the new definition) and 
that actually changing the signature would allow for static code analysis tools 
to actually catch the error without needing to run the code, I think that we 
should consider further changing this function and making sure that the change 
doesn't go so easily unnoticed like today.

----------
nosy: +Leandro Lima

_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue37218>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to