Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116758
Message-ID: <1174b73a-9e79-15f1-7fd3-497ad4f5ef31@allenjb.me.uk>
Date: Sun, 2 Jan 2022 11:26:59 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.1
To: internals@lists.php.net
References: <CAFv4g+Efz-V5V1uUedXpusNw6z-on=TT3kdWgC66SptSd8kB4g@mail.gmail.com>
In-Reply-To: <CAFv4g+Efz-V5V1uUedXpusNw6z-on=TT3kdWgC66SptSd8kB4g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Allowing NULL for some internal functions
From: php.lists@allenjb.me.uk (AllenJB)

On 01/01/2022 23:17, Craig Francis wrote:
> I've spent a few days coming up with a short(ish) list of parameters that
> are likely to receive `NULL`.
>
> The focus is on making it easier for developers to upgrade to newer
> versions of PHP. I'm thinking of the typical developer, probably using
> WordPress, isn't using Psalm (at levels 1 to 3, with no baseline), and
> isn't using `strict_types`; where they don't really have the time to add
> strval() to everything that could be NULL.
>
> Draft RFC:
> https://wiki.php.net/rfc/allow_null
>
> And the list, where I'm proposing we only relax this requirement for the
> *bold* parameters:
> https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-change.md
>
> I'll give it a few weeks to see if there are any parameter suggestions
> (welcome via email, pull request, carrier pigeon, etc), then I'll start
> looking for the best way to implement this.
>
> Craig
>
>
> PS; starting new thread, as I have gone a bit off topic, original one at:
> https://externals.io/message/116519#116556

This RFC has come out of a similar discussion around deprecating dynamic 
properties (and this isn't the first time "upgrading is hard" / "the 
language is moving too fast" has come up), so it's not just this one 
change that's affected.

I think this RFC is trying to solve the wrong problem / only fixing one 
small portion of the "immediate" problem without paying attention to the 
bigger picture.


Alternative solutions:

I feel that effort would be better spent on improving and encouraging 
use of tooling such as the PHPCompatibility CodeSniffer ruleset (which 
has unfortunately fallen behind for changes in PHP 8+) and Rector to 
improve the upgrading process for everyone. I recognize that there's 
arguments against the project being seen to endorse / contribute to 
specific tools / libraries / frameworks, but I feel that in this area 
the benefits outweigh the costs.

The project could provide guides (or link to selected existing articles) 
for upgrading older code. Whilst significant documentation does already 
exist on this it's not always easy to find or contains errors (a lot of 
- probably copy-paste - blog articles that I've seen focus on 
PHPCompatibility for upgrading to 8.0 / 8.1 even tho its coverage of the 
changes in these versions is poor). This could be done via something 
like a user-contributed wiki similar to what Python and other projects 
do: https://wiki.python.org/moin/FrontPage - I recognize that there's 
significant work in this suggestion, but I feel it also offers good 
opportunities for further improvements to the PHP ecosystem / community.

The migration guides could include documentation on updating existing 
code for each change (could / should more be done to encourage this 
being done at the time the change is made rather than trying to compile 
migration docs just before release?) - they do this sometimes but I feel 
this is an area with room for improvement.

The above mentioned alternatives solve the entire class of problem 
whilst also maintaining PHP's work towards being a stricter language 
that's that much harder to create bugs with and needs less "boilerplate" 
for more defensive programming. Which leads to the argument that in 
trying to solve a one-time problem for some developers, this RFC creates 
work for developers both in debugging code, and for developers with 
defensive programming styles to (re-)add checks to detect the exact 
class of problem these deprecations / warnings highlight.

Developers also already have tools to selectively silence deprecation 
notices using custom error handlers.


On the proposed changes / RFC content:

The RFC uses retrieving request parameters (get/post/cookie) values 
using various frameworks as a (primary) example. With the exception of 
CodeIgniter (it may be possible, but wasn't immediately obvious from the 
quickest of glances at their docs) all the frameworks used in the 
example have an additional default value parameter. This makes all of 
these (and the null coalesce alternative) relatively trivial to update 
using a regular expression find and replace, that most text editors can do.

The RFC focuses on changing string parameters, but uses 
mysqli_fetch_row(), json_decode() and error_get_last() in its arguments 
- none of which are usually used to return a string (or anything you'd 
want to treat as a string). Accessing properties of non-objects (since 
PHP 5.0) and accessing non-existant array indexes (since PHP 7.4) have 
been warnings / notices for a signifcant time already.

Some of the suggested functions (parameters) to change are strange 
selections in my opinion - gz functions, bcmath parameters, 
password_hash() and mail() (subject / message) are not usually values 
you want to pass either null or an empty value to (and are almost 
certainly bugs if the code does that). (And similarly from the maybe 
list, I would apply this to most of the date_create functions, dns 
functions hostname and logging functions message parameter)

If this change were to be made, I think it would be better to severely 
restrict the list to only the most commonly used functions. How many 
codebases affected by the null parameter deprecation are using sodium or 
MessageFormatter? While this might leave some work for developers 
upgrading codebases, I feel the long list of functions affected by this 
RFC (even without considering the maybe list) goes too far.

How would code that declares strict_types be affected by this RFC? This 
isn't clearly defined in the RFC. I would prefer it if these function 
continue to not accept null (and throw a TypeError) if strict_types is 
declared.