Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117651
Message-ID: <0484c0c8-569f-7889-343d-829fb820c64d@gmail.com>
Date: Sat, 30 Apr 2022 18:05:06 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Content-Language: en-GB
To: PHP internals <internals@lists.php.net>
References: <CAFv4g+Ej4JbM9SzQ0iU-d8C4T+2XNo=8jnJ1uwfQ=skZZXx9Tg@mail.gmail.com>
 <42D0A480-F262-4F72-9C4D-887762A8D800@gmail.com>
 <CAFv4g+ELZ5ZA6zyDFHCJCM6vSy3MFTvEGS9=zo5ZM1izHQWuFw@mail.gmail.com>
 <0b061f28-a087-efd3-8602-424ee03458e0@gmail.com>
 <CAFv4g+HROPPx4hT226ypbKZwM_7nq31jJ6+X+RNq_THZDVrJPA@mail.gmail.com>
 <CALKiJKpPri6A35_g4XqWOG4Zm3XMdz4Z3gRFyMTh9V2TxrbYmg@mail.gmail.com>
 <7DB0A01F-04FB-420D-9025-E027E5DE02F7@craigfrancis.co.uk>
 <c3197e59-3447-0ebc-094b-cb1d67e2314b@gmail.com>
 <9859B3B4-091A-4311-8F68-F6C35FBC32A1@craigfrancis.co.uk>
In-Reply-To: <9859B3B4-091A-4311-8F68-F6C35FBC32A1@craigfrancis.co.uk>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] NULL Coercion Consistency
From: rowan.collins@gmail.com (Rowan Tommins)

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]