Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117515
MIME-Version: 1.0
References: <CAFv4g+Ej4JbM9SzQ0iU-d8C4T+2XNo=8jnJ1uwfQ=skZZXx9Tg@mail.gmail.com>
In-Reply-To: <CAFv4g+Ej4JbM9SzQ0iU-d8C4T+2XNo=8jnJ1uwfQ=skZZXx9Tg@mail.gmail.com>
Date: Mon, 11 Apr 2022 16:14:32 +0100
Message-ID: <CAFPFaML-dia9d7qhpqsVjaQ1vruF+eODXYtieow8BhZ69HL4YQ@mail.gmail.com>
To: Craig Francis <craig@craigfrancis.co.uk>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000006fb93305dc6267e4"
Subject: Re: [PHP-DEV] NULL Coercion Consistency
From: george.banyard@gmail.com ("G. P. B.")

--0000000000006fb93305dc6267e4
Content-Type: text/plain; charset="UTF-8"

On Fri, 8 Apr 2022 at 18:35, Craig Francis <craig@craigfrancis.co.uk> wrote:

> Hi,
>
> I've written a new draft RFC to address the NULL coercion problems:
>
> https://wiki.php.net/rfc/null_coercion_consistency


The statement that:

> But the implementation caused a Type Error when coercing NULL for everyone
> (even when not using *strict_types=1*), this seems more of an over-sight
>
is utterly wrong and was a conscious design choice based on the widely
accepted view that nullable by default like Java is a mistake and defeats
the purpose.

Even the competing RFC from Zend et al [1] proposed this behaviour

> A new set of coercion rules will apply to both user-land type hints and
> internal type hints. The guiding principals behind these new rules are:
>
>    1. If the type of the value is an exact match to the type requested by
>    the hint - allow.
>    2. If the value can be coerced to the type requested by the hint
>    without data loss and without creation of likely unintended data - allow.
>    3. In all other cases - reject.
>
> And later to describe the internal function changes:

> This RFC proposes to bring the rule-set described in the last section to
> internal functions as well, through updates to the zend_parse_parameters()
> function.
>

There was obviously the caveat about NULL being coerced for internals
functions, but other than usage, another reasons stated:

> However, since this requires substantial auditing of internal functions -
> especially ones that have default values but don't explicitly declare
> themselves as accepting NULLs - it's outside the scope of this RFC and
> will be revisited for 7.1. Note that if the
> https://wiki.php.net/rfc/nullable_types
> <https://wiki.php.net/rfc/nullable_types_rfc> is accepted it will further
> reduce this discrepancy, by allowing user-land functions and internal
> functions the same level of granularity in terms of accepting or rejecting
> NULL values for function arguments.
>

This auditing was to a large extent performed in PHP 8.0 with the
introduction of the stubs.
You can disagree with some of it, but this again brings us back to amending
functions to have nullable arguments, not a blanket change.


I appreciate some want to force strict type checking on everyone, but I
> want to make sure we have this properly documented, with names, and
> explanations.
>
> Breaking changes should be justified - if they aren't, they only
> make upgrading difficult and frustrating (bad for security).
>

The breaking change was justified and explained, and voted unanimously in
favour with 46 votes.
The burden of justification is now on your end.

Moreover, the breaking change has also been announced with ample time to
fix the issues until the next major version.

As with the previous one, this RFC is getting a strong no from my end,
changing the type system into a broken and inconsistent way, and reversing
a unanimously voted RFC needs strong justification.

Best,

George Peter Banyard

[1] https://wiki.php.net/rfc/coercive_sth

--0000000000006fb93305dc6267e4--