On 27/04/2022 16:51, Craig Francis wrote:
Forgive this primitive example, but this shows `$name` being used in three 
different ways, where an automated tool cannot simply change line 1 so it 
doesn't return a NULL, because it would break the Register link.

$name = $request->get('name');

if (trim($name) === '') { // Contains non-whitespace characters; so not "", " 
", NULL, etc
   $where_sql[] = 'name LIKE ?';
   $where_val[] = $name;

echo '
   <form action="./" method="get">
       <input type="search" name="name" value="' . htmlspecialchars($name) . '">
     <input type="submit" value="Go">

if ($name !== NULL) {
   $register_url = '/admin/accounts/add/?name=' . urlencode($name);
   echo '
     <p><a href="' . htmlspecialchars($register_url) . '">Add Account</a></p>';

In this example, why does `trim()` and `htmlspecialchars()` justify a fatal 
Type Error?

Honestly, I fail to see how the inconsistent null handling in this example is anything other than a bug:

* If strings containing only spaces are considered empty, it's probably a mistake that they're not trimmed everywhere. But of course adding $name = trim($name) would remove all distinction between null and ''. * If null is equivalent to an empty string when deciding whether to run the SQL, why is it not also equivalent to an empty string when rendering the register link? The current logic means that clicking "Go" causes a register link to appear, with an empty name on the query string, which doesn't seem like useful functionality. * On the other hand, if NULL is considered a different state, why is there no behaviour distinguishing it around the SQL logic? It seems likely that an else clause would be added there, perhaps "else { echo 'You must enter a name.'; }" In the current code, that would appear for both null and empty strings, which is probably a mistake.

I think that actually demonstrates quite nicely that most code would benefit from treating "string" and "?string" as strictly different types, and either defaulting to an empty string explicitly and early, or considering null at every step. Simultaneously relying on null values being preserved, and them being silently coerced, leads to fragile code where it's not clear where nulls are deliberately handled as empty strings, and where they've simply been forgotten about.

Telling users when they've passed null to a non-nullable parameter is precisely 
about *preserving* that distinction: if you want null to mean something 
specific, treating it as a string is a bug.

I don't think that represents a bug, are we are talking about a system that 
takes user input (so often nullable), supports coercion with other simple 
types, and supports NULL coercion in other contexts (e.g. string concat).

Read the sentence you replied to again: IF you want to treat null as distinct from '', THEN failing to do so is a bug. If, on the other hand, you just want to take nullable user input and handle it CONSISTENTLY as a string, then it's fine to explicitly default to '' AT SOURCE.

Despite all of the above, I am honestly torn on this issue. It is a disruptive 
change, and I'm not a fan of errors for errors' sake; but I can see the value 
in the decision made back in 7.0 to exclude nulls by default.

Thanks Rowan, I'll just add that I think the fatal Type Error for NULL will be 
much more disruptive, and I would rather relax that requirement for user 
defined functions, so NULL coercion works in all contexts.

I think the main difference between our positions is that I believe that if PHP's type system was designed from scratch today, null would not be silently coerced in these situations. So while I agree that the change will be disruptive, I disagree with your position that it brings no benefit.

On 27/04/2022 18:34, Craig Francis wrote:
But I'm wondering, is it only one function? and assuming it's a problem, could we 
use `Z_PARAM_LONG_OR_NULL()` and specifically throw an exception when either 
parameter is NULL, like the `max < min` check? On the basis that I'd rather 
have one extra check for this function, and keep NULL coercion working everywhere 
else (i.e. where it's fine).

Well, since it's one of three examples in Guilliam's e-mail, the answer to the first question seems rather trivially "no", unless I'm missing something? As for the second question, certainly we could add specific prohibitions to null on a case by case basis, but that's basically equivalent to your previous suggestion of explicitly allowing null on a case by case basis, and doesn't really answer the question of what the default behaviour should be - especially bearing in mind that any default should apply to both built-in and user-defined functions.


Rowan Tommins

PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Reply via email to