Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117511
MIME-Version: 1.0
References: <CAFPFaM+3gxkRbNdza0T_69Nma7d3Y5G5pTG-Ow_8grW_cbHJTw@mail.gmail.com>
 <fd9ffa13-20dd-8a50-96c6-a092cf146179@bastelstu.be>
In-Reply-To: <fd9ffa13-20dd-8a50-96c6-a092cf146179@bastelstu.be>
Date: Mon, 11 Apr 2022 14:32:58 +0100
Message-ID: <CAFPFaMJKK6PHzqp6ZPXp_Ykgfj28fG3V=8w1uNo+wu96JauNjQ@mail.gmail.com>
To: =?UTF-8?Q?Tim_D=C3=BCsterhus?= <tim@bastelstu.be>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000032690805dc60fc13"
Subject: Re: [PHP-DEV] [RFC] Add true as type
From: george.banyard@gmail.com ("G. P. B.")

--00000000000032690805dc60fc13
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, 9 Apr 2022 at 12:05, Tim D=C3=BCsterhus <tim@bastelstu.be> wrote:

> Hi
>
> On 4/8/22 18:47, G. P. B. wrote:
> > Now that null and false can be used as standalone types, I think it
> makes a
> > lot of sense to add true as a value type for the sake of completeness
> with
> > false.
>
> I agree that it would make sense to have 'true' as the counterpart of
> `false` for completeness, but =E2=80=A6
>
> > It also has many use cases within internals and userland to declare
> proper
> > type signatures to functions and methods.
> >
> =E2=80=A6 seeing the examples of functions that might return `true`, but =
won't
> return false, I feel like the correct solution is not adding `true` as a
> standalone type, but instead making those function return something more
> obvious.
>
> - The `sort()` family should rather be be ': void' which would make it
> more obvious to static analysis that '$sorted =3D sort($unsorted)' is not
> a correct usage.
> - `DiagnoseCommand::checkComposerSchema()` in composer would benefit
> from having support for tagged unions, as currently both the error case
> (`string`) and the success case (`true`) are truthy. With the current
> PHP 8.1 feature set I can imagine that returning a proper bool and
> putting the reason in an `string &$reason` reference parameter would
> make it more obvious to use this function in an `if ()`.
> - `JsonFile::validateSchema()` in composer should be ': void' as errors
> appear to be communicated using Exceptions.
> - The closure in `ComposerRepository::asyncFetchFile()` would benefit
> from having support for tagged unions, but even with the current PHP 8.1
> feature set I can imagine that:
>
>      enum CacheStatus { case Hit; }
>
> and then making the function `mixed[]|CacheStatus` instead of
> `mixed[]|true` would make it more obvious what the meaning of the second
> option in the union is.
>
> Best regards
> Tim D=C3=BCsterhus
>

There are many many many more internal functions in PHP which only return
true, but only since PHP 8.0.0, and this is due to the huge amount of
E_WARNING to ValueError/TypeError promotion which has happened.
These functions previously did return false in certain circumstances, and
although I agree that changing these to void *would* be the most ideal,
being able to do this communicating that these functions only return true
(which means one can ignore the return value) is the first step.
Moreover, it is even more of a BC break compared to changing something
which only returns false to void as code like:
if (always_true(...)){ ... }
would stop executing this code path.

Not adding true as a type prevents extending methods which return bool to
always return true to clearly document this within the typesystem.

Best regards,

George P. Banyard

--00000000000032690805dc60fc13--