search

webmozarts / assert

Assertions to validate method input/output with nice error messages.
MIT License
7.56k stars 145 forks source link

`Assert::isList` does not work for a list containing `NAN` #246

Closed boesing closed 2 years ago

boesing commented 3 years ago

I've tried to optimize the polyfill of symfony/polyfill-php81 which provides array_is_list function.

Since I've seen how Assert::isList is doing the assertion, I've thought thats a smart way of doing so. Sadly, PHP is a bit ... special when working with the NAN value.

https://3v4l.org/8I0Du

Thus said, the Assertion from this library might not be accurate when it comes to arrays with NAN.

I've worked around it by using:

array_keys($array) === range(0, count($array) -1);

Sadly, this will slow down the check a bit: https://3v4l.org/QGiu0

Not sure if its worth to use the new implementation as having arrays containing NAN might be rare. But at least I wanted to share my todays WTF moment.

zerkms commented 3 years ago

Given the current implementation gives false negatives - one optimisation could be only perform a slow recheck if the main fast check returned false.

So, if $array !== \array_values($array) is false -> do extra array_keys($array) === range(0, count($array) -1).

Given negative branch incurs throwing an exception, extra slowness does not really matter then.

And the positive case would be as fast.

BackEndTea commented 2 years ago

This has been fixed, and will be released in 1.11.0