Uh oh!
There was an error while loading. Please reload this page.
Add ZPP checks to count() - #4940
Conversation
1df1a42 to
26d2e35Comparea2e1f6f to
7b6dc7bCompareI am not sure if it is really an argument or a counter-argument, but I'd like to share this with you. So the other day I found a piece of code in our codebase which says: I am sure that there are tons of such code in the wild... So on one hand, all these code would result in an exception all of the sudden by this PR, but on the other hand, such issues would be almost impossible to commit in the first place. |
Isn't this already producing a "type error" warning (as of 7.2)? The rationale for this PR (and the previous one) is to align its behaviour as per the Consistent Type Error RFC. [1] However it is always good to know that there is surprising code in the wild, because I have no clue what are the expected result of The previous on was closed as using |
I haven't seen any warning (but we are not yet on PHP 7.2).
Having forgotten about your PR, I also tried to promote this warning to an exception in October. ^^ All in all, I am also in favour of promoting this warning, I just wanted to highlight this aspect of the change. |
Girgias
commented
Jan 21, 2020
For reference: https://www.php.net/manual/en/migration72.incompatible.php#migration72.incompatible.warn-on-non-countable-types
Makes sense :) |
kocsismate
commented
Jan 21, 2020
What I really wanted to say is that I hadn't seen any warnings in our logs 😅 But the reason is obvious then ^^ |
Girgias
commented
Jan 21, 2020
I know just pointing out the relevant section of the upgrading document (because I wasn't sure myself if it was 7.2) :) |
Uh oh!
There was an error while loading. Please reload this page.
Add ZPP checks for the first argument to ensure it is an array or a Countable object. Due to earlier discussion NULL is special cased and deprecated.
A new take on #4572 as it was declined due to the prevalence of using
countonNULL.This allows to use
NULLwith count but deprecates its usage (which I consider valid as it was a warning in PHP 7.2+).It also adds some checks on the $mode argument.
However I needed to change the test runner again because apparently there is a weird edge case which I cannot reproduce in a test file.