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">
<label>
Search
<input type="search" name="name" value="' . htmlspecialchars($name) . '">
</label>
<input type="submit" value="Go">
</form>';
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.
Regards,
--
Rowan Tommins
[IMSoP]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
