Return false for hash_hmac/hash_hmac_file on invalid algorithm

[pascalheidmann]

This PR adds a dynamic function return type extension for both hash_hmac and hash_hmac_file as both functions can return false in certain circumstances. For both functions this means providing an invalid algorithm as first parameter will statically return false, otherwise string for hash_hmac and string|false for hash_hmac_file (we can't check for file readability in static analysis, and you should always check for file errors anyway).

There has been a PR more than 1.5 years ago that tried to fix this within static functions map but that would lead to far more false positives.

[ondrejmirtes]

Otherwise this looks great 👍

[ondrejmirtes]

This array can be here statically, it's not referenced anywhere else in the extension.

[pascalheidmann]

Have inlined the value, hope this is correct?

I had orientated on other instances like ArrayPointerFunctionsDynamicReturnTypeExtension which also uses the private array $functions = [] notation.

[ondrejmirtes]

There needs to be a fallback for PHP 7.1 using https://www.php.net/manual/en/function.hash-algos.php

[pascalheidmann]

I have added function_exists('hash_hmac_algos') ? hash_hmac_algos() : hash_algos() check but sadly PHPStan doesn't care. Any suggestion?

[ondrejmirtes]

It's fine, just make the tests pass and I'll fix the PHPStan issue just before merging :) Thanks.

[dktapps]

Just a 2 cents: Maybe it might make more sense to specify the algo parameter as a union of all available hash functions as UnionType(ConstantStringType) so that this isn't relying on the existence of false return to make errors show up in user code.
Context: in PHP 8, this triggers a ValueError instead of returning false: https://github.com/php/php-src/blob/efe6006ced4f91d0df1fdb0aef7f0079273ac0d0/ext/hash/hash.c#L496

[pascalheidmann]

@ondrejmirtes I have replaced the conditional declaration of those supported algorithms with a static one like @dktapps suggested.

As the behaviour seems to change from return false to a more convenient Exception driven approach: How should this be handled?

[ondrejmirtes]

Thank you!

[ondrejmirtes]

I'll handle that case when I'll be adding support for PHP 8.