On Wed, 20 Apr 2022 at 18:02, Craig Francis <cr...@craigfrancis.co.uk> wrote: > I'm just trying to focus on how PHP has worked
You keep repeating this mantra, but user-defined functions with declared parameter types have never accepted nulls, in any mode, unless explicitly marked with "?" or "= null". The fact that internal functions have parameter parsing behaviour that is almost impossible to implement in userland, and often not even consistent between functions, is a wart of engine internals, not a design decision. All of that, and the "consistency" in the title of your RFC, is a complete distraction from the real questions: 1) given a null input, and a non-nullable parameter, what should the run-time do? 2) what is the best way to help users update their code to be compatible with that new behaviour? > Agreed, the only thing I'd add... failing early with NULL might help debug some problems (assuming there is a problem), but I believe static analysis and unit tests are much better at doing this (e.g. static analysis is able to see if a variable can be NULL, and apply those stricter checks, as noted by George with Girgias RFC). I think we should institute a "swear jar" - whenever someone says "static analysis could do this better", they have to donate €1 to the PHP Foundation. :P More seriously, 1) your own RFC claims that fewer than 33% of developers use static analysis; and 2) if PHP had a compile step with mandatory static analysis, we would still have to decide whether passing NULL to these functions should be rejected as an error by the static analyser. > In contrast, failing early at runtime, on something that is not actually a > problem, like the examples in my RFC, creates 2 problems (primarily > upgrading; but also adding unnecessary code to explicitly cast those > possible NULL values to the correct type, which isn't needed for the other > scalar types). > I've been trying not to nit-pick, because I think over-all you do have a valid point, but several of your examples do not need any extra code to handle nulls; and those that do need maybe a handful of characters. For instance, $search = ($_GET['q'] ?? ''); is both shorter and clearer than $search = ($_GET['q'] ?? NULL); > Thank you... but I will add, while `htmlspecialchars()` rejecting NULL > would get you to look at the code again, I wouldn't say it's directly > picking up the problem, and it relies on there being a NULL value > in $fields for this to be noticed (if that doesen't happen, you're now in a > position where random errors will start happening in production). > There are always going to be errors that depend on the value of input, and unless you have god-like skill at writing tests, some of those will only happen in production. Saying "this shouldn't happen" doesn't answer the question of what to do when it does. > Bit of a tangent, I'm uncomfortable that `$name` is not being HTML > encoded, which takes us to context aware templating engines, and how you > can identify these mistakes via the `is_literal` RFC or the > `literal-string` type. > This isn't real code, it's an illustrative example; but if it makes you feel more comfortable, imagine $name has some deliberate HTML markup in it, so needs to be echo'd raw. > That said, if you (or anyone) have any better ideas on how to address this > problem, please let me know. > I honestly would be more interested in having some string functions return null for null input than changing existing behaviour to accept null for non-nullable parameters (interestingly, until PHP 8.0, htmlspecialchars() could return null, e.g. if given an array). Unfortunately, that would be a different kind of compatibility break, so I'm not sure it fully solves the problem. Regards, -- Rowan Tommins [IMSoP]
