| Both sides previous revisionPrevious revisionNext revision | Previous revisionLast revisionBoth sides next revision |
| rfc:allow_null [2021/12/31 12:31] – craigfrancis | rfc:allow_null [2022/02/20 20:19] – Possible options craigfrancis |
|---|
| ====== PHP RFC: Allow NULL ====== | ====== PHP RFC: Allow NULL ====== |
| |
| * Version: 1.1 | * Version: 1.3 |
| * Voting Start: ? | * Voting Start: ? |
| * Voting End: ? | * Voting End: ? |
| * RFC Started: 2021-12-23 | * RFC Started: 2021-12-23 |
| * RFC Updated: 2021-12-31 | * RFC Updated: 2022-02-20 |
| * Author: Craig Francis, craig#at#craigfrancis.co.uk | * Author: Craig Francis, craig#at#craigfrancis.co.uk |
| * Status: Draft | * Status: Draft |
| ===== Introduction ===== | ===== Introduction ===== |
| |
| PHP 8.1 introduced "Deprecate passing null to non-nullable arguments of internal functions" ([[https://externals.io/message/112327|short discussion]]), which is making it difficult for developers to upgrade. | PHP 8.1 introduced "Deprecate passing null to non-nullable arguments of internal functions" ([[https://externals.io/message/112327|short discussion]]), which is making it difficult (time consuming) for developers to upgrade. |
| |
| Often //NULL// is used for undefined //GET/////POST/////COOKIE// variables: | This has introduced an inconstancy when not using //strict_types=1//, because most variables types (e.g. integers) can be coerced to the relevant type, but //NULL// has been deprecated. |
| | |
| | In PHP //NULL// is often used to represent something; e.g. when a //GET/////POST/////COOKIE// variable is undefined: |
| |
| <code php> | <code php> |
| * //json_decode()// | * //json_decode()// |
| |
| Which makes it common for //NULL// to be passed to internal functions, e.g. | This makes it common for //NULL// to be passed to many internal functions, e.g. |
| |
| <code php> | <code php> |
| </code> | </code> |
| |
| This includes developers explicitly using //NULL// to skip certain parameters, e.g. //$additional_headers// in //mail()//. | Developers also use //NULL// to skip certain parameters, e.g. //$additional_headers// in //mail()//. |
| |
| Currently this affects those using PHP 8.1 with //E_DEPRECATED//, but it implies everyone will need to modify their code in the future. | Where //NULL// has always been coerced into an empty string, the integer 0, the boolean false, etc. |
| |
| It also applies even if the developer is not using //strict_types=1//. | Currently the deprecation notices only affect those using PHP 8.1 with //E_DEPRECATED//, but it implies everyone will need to modify their code to avoid **Fatal Errors** in PHP 9.0. |
| |
| And while the individual changes are easy - there are many of them, they are difficult to find, and often pointless (e.g. //urlencode(strval($name))//). | Developers using //strict_types=1// may find some value in this, but it's excessive and inconstant for everyone else. |
| |
| Without the changes below, developers will need to either - use these deprecation notices, or use very strict Static Analysis (one that can determine when a variable can be //NULL//; e.g. Psalm at [[https://psalm.dev/docs/running_psalm/error_levels/|level 3]], with no baseline). | And while individual changes are easy, there are many of them (time consuming), difficult to find, and often pointless, e.g. |
| |
| ===== Proposal ===== | * urlencode(strval($name)); |
| | * urlencode((string) $name); |
| | * urlencode($name ?? ''); |
| |
| Update **some** internal function parameters to accept //NULL//, to reduce the burden for developers upgrading. | To find these issues, developers need to either - use these deprecation notices, or use very strict Static Analysis (one that can determine when a variable can be //NULL//; e.g. Psalm at [[https://psalm.dev/docs/running_psalm/error_levels/|level 3]], with no baseline). |
| |
| While this is in Draft, the [[https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-change.md|list of functions are hosted on GitHub]]. | It's worth noting that some parameters, like //$separator// in //explode()//, already have a "cannot be empty" Fatal Error. So it might be useful to have a separate RFC to update some of these parameters to consistently reject NULL **or** Empty Strings, e.g. //$needle// in //strpos()// and //$json// in //json_decode()//. |
| |
| Only the parameters in **bold** would be changed. | ===== Proposal ===== |
| |
| Suggestions and pull requests welcome. | There are 3 possible approaches: |
| |
| ===== Decision Process ===== | - NULL triggers a Fatal Error with strict_types=1, otherwise allow coercion (like how integers can be coerced to a string). |
| | - NULL triggers a Fatal Error for everyone, but update some parameters to explicitly allow NULL (e.g. //?string//). |
| | - NULL triggers a Fatal Error for everyone (forget about backwards compatibility). |
| |
| Does the parameter work with //NULL//, in the same way it would with an empty string? e.g. | This needs to be done before the eventual end of the deprecation period, and //TypeError// exceptions are thrown. |
| |
| - //preg_match()// should continue to deprecate //NULL// for //$pattern//, whereas //$subject// should accept //NULL//. | If we choose to "update some parameters to explicitly allow NULL", there is a [[https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-change.md|draft list of functions]], where the **bold** are being proposed (currently only includes string parameters). [[https://github.com/craigfrancis/php-allow-null-rfc/issues|Suggestions]] and [[https://github.com/craigfrancis/php-allow-null-rfc/pulls|Pull Requests]] welcome. |
| - //hash_file()// should continue to deprecate //NULL// for the //$filename//. | |
| - //hash()// should accept //NULL// for //$data//. | |
| - //substr_count()// requires a non-empty string //$needle// (continue to deprecate //NULL//). | |
| - //mb_convert_encoding()// requires a valid encoding for //$to_encoding// (continue to deprecate //NULL//). | |
| |
| You could argue some function parameters should not accept an empty string (e.g. //strrpos()// accepting an empty string for //$needle//), but those should be addressed in a future RFC, involving a discussion on any backwards compatibility issues for every change (there is no point complaining about //NULL// now, and then going though this process again if the developer simply uses //strval()// to get an empty string). | There is also a [[https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-maybe.md|Maybe List]], where the more questionable arguments end with a "!". For example, //strrpos()// accepting an empty string for //$needle// is wired in itself, and //sodium_crypto_box_open()// should never receive a blank //$ciphertext//. And there is an [[https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-other.md|Other List]], which can be ignored. |
| |
| One set of candidates that could be removed are functions like //sodium_crypto_box_open()// where a blank //$ciphertext// will always return //false// (for failure). | ===== Decision Process ===== |
| | |
| | Does //NULL// for this parameter justify a Fatal Error? e.g. |
| | |
| | - //preg_match()// should **deprecate** //NULL// for //$pattern// ("empty regular expression" warning). |
| | - //preg_match()// should **accept** //NULL// for //$subject// (e.g. checking user input). |
| | - //hash_file()// should **deprecate** //NULL// for the //$filename//. |
| | - //hash()// should **accept** //NULL// for //$data//. |
| | - //substr_count()// should **deprecate** //NULL// for //$needle// ("$needle cannot be empty" error). |
| | - //mb_convert_encoding()// should **deprecate** //NULL// for //$to_encoding// (requires a valid encoding). |
| |
| ===== Backward Incompatible Changes ===== | ===== Backward Incompatible Changes ===== |
| ===== Future Scope ===== | ===== Future Scope ===== |
| |
| Some functions parameters could be updated to complain when an Empty String or //NULL// is provided. | Some function parameters could be updated to complain when an //NULL// **or** //Empty String// is provided; e.g. //$method// in //method_exists()//, or //$characters// in //trim()//. |
| |
| ===== Voting ===== | ===== Voting ===== |
| TODO | TODO |
| |
| ===== Patches and Tests ===== | ===== Tests ===== |
| |
| To get and **Test** the list of functions, I wrote a script to //get_defined_functions()//, then used //ReflectionFunction()// to identify parameters that accepted the 'string' type, and not //->allowsNull()//. This resulted in the [[https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-change.md|list of functions to change]], where I manually removed the [[https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-other.md|functions that shouldn't be changed]], and updated the script to test every argument (to see that it complained with //NULL//, and the output remained the same) - [[https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions.php|Source]]. | To get and **Test** the list of functions, I wrote a script to //get_defined_functions()//, then used //ReflectionFunction()// to identify parameters that accepted the 'string' type, and not //->allowsNull()//. This resulted in the [[https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-change.md|list of functions to change]], where I manually removed the [[https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-other.md|functions that shouldn't be changed]], and updated the script to test every argument (to see that it complained with //NULL//, and the output remained the same) - [[https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions.php|Source]]. |
| ===== Implementation ===== | ===== Implementation ===== |
| |
| TODO | https://github.com/craigfrancis/php-src/compare/master...allow-null |
| | |
| | This patch defines //Z_PARAM_STR_ALLOW_NULL//. |
| | |
| | It works a bit like //Z_PARAM_STR_OR_NULL//, but it will return an empty string instead of //NULL//. |
| | |
| | It's a fairly easy drop in replacement for //Z_PARAM_STR//, e.g. [[https://github.com/php/php-src/blob/7b90ebeb3f954123915f6d62fb7b2cd3fdf3c6ec/ext/standard/html.c#L1324|htmlspecialchars()]]. |
| |
| ===== Rejected Features ===== | ===== Rejected Features ===== |