Message 357551 - Python tracker (original) (raw)

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.