Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117801
Message-ID: <24d35dc8-bd1d-3d90-99ac-ebdcc3b2a9e4@gmail.com>
Date: Fri, 27 May 2022 10:11:18 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Content-Language: en-GB
To: internals@lists.php.net
References: <CAFv4g+Ej4JbM9SzQ0iU-d8C4T+2XNo=8jnJ1uwfQ=skZZXx9Tg@mail.gmail.com>
 <1755E8B5-229B-47B2-BBAF-B5E014F5473D@craigfrancis.co.uk>
 <1180af01-080f-ee0a-3159-74bf7e0a8aea@gmail.com>
 <73F563E2-7C31-4B5E-A6B9-AE1BD05ADD1C@craigfrancis.co.uk>
In-Reply-To: <73F563E2-7C31-4B5E-A6B9-AE1BD05ADD1C@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 26/05/2022 13:20, Craig Francis wrote:
> First, the Docblock originally said this function did not accept NULL, but at runtime it accepted/coerced NULL to an empty string. This is exactly how `htmlspecialchars()` worked pre 8.1. Where developers using static analysis tools can choose to treat NULL as an invalid value, and those tools could report nullable variables as an error (via strict type checking).


The same events can be given a very different narrative:

Originally, this function was not intended to accept null. The 
documentation clearly stated the accepted types, and any static analysis 
tool would reject any value not in that list. The author of the function 
chose to treat NULL as an invalid value.

Because the list of valid types wasn't enforced at run-time, it was 
accidentally possible to pass null. Many developers who weren't using 
additional tooling came to rely on this bug, so as a compromise, the 
authors decided to change the documented behaviour of nulls.

At no point did anybody look at the function and say "I can safely pass 
null to this, as long as I remember never to use static analysis tools". 
They accidentally passed null, and by luck got a useful result.


>> I also note that the commit message says "On PHP >= 8.1, an error is thrown if `null` is passed to `htmlspecialchars`." which is of course not true for native PHP, only if you make the highly dubious decision to promote deprecations to errors.
> While I'd put the word "error" down as a typo, the intention is for 9.0 to throw a type error.


My point is that there is no action required on PHP 8.1. The commit 
message should have said, "On PHP >= 9.0, an error will be thrown if 
`null` is passed to `htmlspecialchars`."



> And while user-defined functions are part of the conversation (for consistency reasons), I'm trying to find the benefits of breaking NULL coercion for internal functions (because, if there is an overall benefit, that Laravel Blade patch should be reverted).


This is a complete non sequitur. It's perfectly possible for different 
scenarios to support different decisions, and what Laravel decides that 
particular function should accept is based on their own assessment of 
the trade-offs. PHP is free to make an entirely different decision based 
on entirely different trade-offs.

Meanwhile, the benefits have been explained repeatedly in this thread. 
You may not agree that they are worth the cost, and as I've repeatedly 
said, I have some sympathy for that. But please stop trying to take the 
conversation back to the very beginning by implying that you've asked a 
question and not received an answer.


Regards,

-- 
Rowan Tommins
[IMSoP]