Re: Allowing NULL for some internal functions

On Sat, 1 Jan 2022 at 23:17, Craig Francis <craig@craigfrancis.co.uk> wrote: > Draft RFC: > https://wiki.php.net/rfc/allow_null > I know, I'm a glutton for punishment, but I've started writing a patch for this: https://github.com/craigfrancis/php-src/compare/master...allow-null It does not affect any Fatal Error exceptions for scripts using 'strict_types'. For now it simply defines Z_PARAM_STR_ALLOW_NULL; it's not used by any functions yet (that's the next step). It works a bit like Z_PARAM_STR_OR_NULL (but it will return an empty string instead of NULL). It should be a fairly easy drop in replacement for Z_PARAM_STR. e.g. To update htmlspecialchars(), and related functions, I'd simply update this line: https://github.com/php/php-src/blob/7b90ebeb3f954123915f6d62fb7b2cd3fdf3c6ec/ext/standard/html.c#L1324 Please note I'm not a C programmer, and I barely understand the code base, so constructive/friendly feedback is welcome. Thanks, Craig Extra notes... No-one has given a single reason why any one of the listed functions should not allow NULL; that said, I appreciate that developers using strict_types do prefer this (and they will not be affected by this). I've not seen any suggestion on how Rector, PHPCompatibility, or anything else could provide a way to fix this issue automatically. Only very strict static analysis (e.g. Psalm at levels 1 to 3, with no baseline) currently picks up this issue. I am simply trying to "relax certain type requirements on a case by case basis" (which will help upgrading to PHP 9); ref the original discussion: https://externals.io/message/112327 I know one person simply said this was a "terrible idea", but I'm still waiting to hear any details on why. In response to the "it's a bit late" position; we only have a deprecation at the moment (can be ignored), it will be too late when PHP 9 starts raising Fatal Error exceptions for anyone committing the heinous crime of passing NULL to these functions (for reference, WordPress aren't exactly rushing to fix this one). The change in behaviour is done in zend_parse_arg_str_weak(), so I don't think it will make much of a performance difference; but considering how much effort went in to testing is_literal(), and how people still managed to mis-read that, and how other RFCs don't mention performance *details* at all, I'm skipping that bit of pain. And yes, I have seen the -4 on externals.io... thank you for that </sarcasm>.