Re: Consistent type errors for internal functions and BC breaks

Am 16.01.2021 um 20:01 schrieb Nikita Popov <nikita.ppv@gmail.com>: > On Sat, Jan 16, 2021 at 6:07 PM Christian Schneider <cschneid@cschneid.com > <mailto:cschneid@cschneid.com>> wrote: > While testing the migration of our site to PHP 8 we noticed the BC break in internal functions > like round() for parameter type errors: > round("foo") used to (silently, without warning) cast to float and return 0 where as > now it throws a TypeError: round(): > https://3v4l.org/pU0LD > <https://3v4l.org/pU0LD> > > This might be a blocker for people to migrate to PHP 8, should there be a way to have a > warning-instead-of-exception stage added to PHP 8 even though there first versions were already > released to help people migrate? > > This is the interaction of two changes. The RFC does exactly what it says, and promotes certain > warnings to Error exceptions. At the same time, PHP 8 moved some function implementations to use > standardized parameter handling. These two changes in conjunction can, in rare cases, conspire to > produce an Error where previously no warning was thrown. round() is an example of where this > happened. Thanks for the explanation, I was wondering how this slipped through. Now I know :-) > I don't think there's a comprehensive list of edge cases where something like this > has occurred. It would certainly be useful if any cases that are found make their way into the > manual. I think this should definitely be in the Migration/Upgrade guide. But my main concern stays the same: It is an unfortunate situation that there is no way of knowing if code will abort once switching to PHP 8 for basic things like round(). Background: We have dozens of data sources where we want to ignore single invalid input values without breaking the process and/or discarding the whole data element just because one field has e.g "n/a" instead of a float. You can question this decision but the fact is that we have a lot of battle-proven code working happily with round() doing enough canonization for our needs. I currently see the following options for someone wanting to switch to PHP 8: 1) Do a full code review of all code involving round() etc. (assuming there is a list of affected functions which there currently isn't) 2) Convert all code to use typed variables for everything which could end up in round() etc. 3) Convert all round(...) to round(floatval(...)) (and doing similar stuff for other affected functions) 4) Have a copy of your production environment feeding it live data and wait some time to see if any exceptions are triggered before exposing PHP 8 to the users. I think 1) and 2) are a lot of work and 2) might be a drastic coding style change which could lead to even more work. 3) is an ugly hack and litters the code. 4) Might be the most realistic migration path but delays the PHP 8 switch and is not guaranteed to catch all code paths. That's why I'd like to suggest 5) Generate warnings instead of exceptions for the cases where previously silent code now generates an exception with 8.0.1. This could be in 7.4 (to give users a way of preparing for the transition before actually doing it) or (preferably) in 8.0.2 to speed up PHP 8 adoption while giving people more peace of mind that their code won't break if they have been running in E_ALL mode for a while. - Chris