Re: NULL Coercion Consistency

On 26 May 2022, at 15:01, Michael Babker <michael.babker@gmail.com> wrote: > On Thu, May 26, 2022 at 7:21 AM Craig Francis <craig@craigfrancis.co.uk> wrote: >> That said, I would still like to know about the benefits of rejecting NULL for >> htmlspecialchars(), in the context of that Laravel Blade patch; >> because, if it is beneficial (vs the BC break), they should revert that patch... shouldn't >> they? > > No, they should not. Laravel made an explicit choice to handle null values in its escaping > function for its templating framework. That is their choice to make. They should not be forced to > implicitly handle null values because the language decides to add implicit type coercion to user > defined functions (because consistency seems to be so important to some), and they should not be > forced to reject null values because underlying language functions reject them as well. Erm, I'm simply asking - If there is a good reason for throwing an exception when NULL is passed to htmlspecialchars(), then that reason would also apply to Laravel Blade for it's HTML encoding. If you know what that reasons is, you should be able to make that case to them, and get that patch reverted. I believe the only time it's helpful is when working in a strict type environment, which is best checked with static analysis, as those extra type checks can highlight unexpected values. But most (I'm assuming ~85%) developers simply do not use strict type checking, and don't seem to consider the sources of NULL (see the "Common sources of NULL" I've listed in the RFC), and for those developers, spending days editing their code to manually convert those NULL values, that does not seem justified. To respond to your comment, Laravel doesn't provide a native type declaration (it's in a DocBlock), so they don't have any type coercion for their e() function: https://github.com/laravel/framework/blob/c113768dbd47de7466d703108eaf8155916d5666/src/Illuminate/Support/helpers.php#L109 > Every implicit type coercion is a bug in hiding IMO. That's fine, but strict static analysis is so much better at finding these issues (i.e. can this variable be NULL, vs a runtime check to see if the value is NULL this time)... and if you want runtime checks as well, that's where strict_types=1 comes in. > If a function, by explicit design, can safely work with null values then that parameter should > be properly declared nullable. htmlspecialchars() can and always has safely worked with NULL, the same with the other 335 parameters I've listed. But when I suggested changing these parameters to be nullable, it was not well received. > Yes, that means that there is a lot of work involved for folks between now and the release of > PHP 9.0 to either refactor code to avoid type errors or to get the language to expand the supported > types in appropriate functions, but IMO that effort from all parties is worth it in the long run to > ensure language consistency (I really don’t think userland PHP and internal/extension PHP should > have vastly different behaviors, and type coercion support based on where a function is designed is > one of those holes that needs filled) and to ensure all APIs explicitly declare what types of data > they support. I don't think userland and internal functions should have different behaviours either (my RFC does say "keep user-defined and internal functions consistent"). To achieve that, for those not using strict_types=1, I'm simply suggesting that NULL coercion continues to work for internal functions, and we update user-defined functions to allow NULL coercion. This would also help developers add native types to their functions. Then we have a setup which is basically the same as how the string "6" can be coerced to the integer 6, and how string concatenation with NULL works, how '' == $nullable works, how sprintf('Hi %s', $nullable) works, how echo $nullable` works, etc. Craig