On Tue, 19 Apr 2022 at 14:17, Rowan Tommins <rowan.coll...@gmail.com> wrote:
> On 19/04/2022 12:34, Craig Francis wrote: > > The developers I work with would assume the last definition > > > I think you've somewhat missed my point. I wasn't talking about people's > habits or preferences, I was talking about different *scenarios* where > null is used to mean different things. > Yep, and I'm thankful you listed them... I'm just trying to focus on how PHP has worked (implicitly based on requirements), and how changing that causes problems (and will be an even bigger problem when it becomes a Fatal Type Error). Yes, some people prefer languages that "fails early" and some are more > interested in "do what I mean", but not everything is about that. > 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). 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 also cannot explain why NULL should be rejected ... does it avoid > > any bugs? > > > Yes, sometimes. Imagine an array of values provided by the user; during > validation, those which were optional and not provided get set to null. > You then loop through and display those which were provided: > > foreach ( $fields as $name => $value ) { > if ( $value !== null ) { > echo "<strong>$name:</strong> $value <br>\n"; > } > } > > Then, you realise you forgot about escaping, and decide to run > everything through htmlspecialchars(): > > $htmlFields = array_map('htmlspecialchars', $fields); > foreach ( $htmlFields as $name => $value ) { > if ( $value !== null ) { > echo "<strong>$name:</strong> $value <br>\n"; > } > } > > Spot the bug? $value will now never be null, because htmlspecialchars() > will silently turn the nulls into empty strings. > 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). This is where I'd note the value of unit tests, as they are much better placed to check this feature. 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. I've also seen the opposite problem: a string function was removed > because it was no longer needed, and the code broke because values, > including nulls, were no longer being cast to string. > > > Is protecting against this worth the backwards compatibility cost of > changing the behaviour, and requiring extra code in other scenarios? > Possibly not. But that's different from not having any benefit. That's fair, adding in an extra check might give some benefit in some cases, but I don't think it's reliable, or worth the cost in updating existing code (with no tooling to help) and the additional code that will be needed to cast these possible NULL values to the relevant type (which isn't needed for the other scalar types). That said, if you (or anyone) have any better ideas on how to address this problem, please let me know. Craig