Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:113170
MIME-Version: 1.0
References: <CAH5C8xUhozPLq=q6RE0D7pF-qB-2D+pCwyazGRmyzJ1KRVU2cw@mail.gmail.com>
 <CAF+90c-Q9EfS=kpTkRdkApD8Dk6_VqXXL5zbL19sCtKVntKY6w@mail.gmail.com> <CAOWwgpn5My3XCwa5pQcf-96tgmd4XEmud3d+Ryhs8RZycVm-SQ@mail.gmail.com>
In-Reply-To: <CAOWwgpn5My3XCwa5pQcf-96tgmd4XEmud3d+Ryhs8RZycVm-SQ@mail.gmail.com>
Date: Sat, 13 Feb 2021 22:54:46 +0100
Message-ID: <CAH5C8xVdJ+D0a8wWE-Fvn326fr80qk=5UqRVGGgiuWnkj9NP_w@mail.gmail.com>
To: Nicolas Grekas <nicolas.grekas+php@gmail.com>
Cc: Nikita Popov <nikita.ppv@gmail.com>, PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000be11ad05bb3ecd33"
Subject: Re: [PHP-DEV] [RFC] Adding return types to internal methods (PHP 8)
From: kocsismate90@gmail.com (=?UTF-8?B?TcOhdMOpIEtvY3Npcw==?=)

--000000000000be11ad05bb3ecd33
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Nikita and Nicolas,

After a month of pause, I came back to this topic.

A concern I have is that for methods with union return types, there
> wouldn't be any way to avoid a deprecation warning without having PHP 8 a=
s
> the minimum version requirement. And given PHP's general error handling
> story, deprecation warnings are not exactly graceful...
>

Thanks for pointing out this possible consequence. I do agree that it's a
valid concern, so we should try to do something about it. I could come up
with a few possible mitigations so far:
- instead of E_DEPRECATE, we could emit E_STRICT warnings so that people
can separately tackle this class of errors from the rest
- we could add support for a SuppressReturnTypeDeclaration annotation that
could suppress these notices altogether
- we could postpone the tentative declaration of those method return types
which can't be expressed on PHP 7.4 to a later version (e.g. PHP 8.4) when
the 7.x series will have become EOL for a while

Actually, I counted how many of the return types in question can't be
expressed on PHP 7.4. I found ~270 results out of all the ~1440 methods
which don't yet have a return type declared.

Do you think these ideas would sufficiently answer your concerns?

Is there any way we could make this mechanism more broadly available than
> just for internal methods? This is a problem that also shows up when addi=
ng
> return types in userland libraries.


Inspired by Symfony's strategy, a #[ReturnTypeDeclaration("int|string",
"Library Foo v2.0 will declare int|string as return type")] attribute could
be added to methods in order to gradually
let any child classes know that a future version of library Foo will
declare the int|string return type for the method in question. Then
overriding methods should either declare some kind of
return type (unfortunately, even a wrong one would suffice, since it would
be cumbersome to store exact type information in an attribute, if I'm not
mistaken) or add the very same
attribute - but at least the first argument should be exactly the same,
from character to character - if people want to avoid the deprecation
notice.

It has just come to my mind that in case of internal functions, we could
combine the annotation based approach described above together with the
behavior I originally proposed:
- if an overriding method declares a return type then the regular
compatibility checks would kick in, and a notice would be emitted in case
of failure
- if an overriding method doesn't have a return type, but
a ReturnTypeDeclaration attribute, then a notice would be emitted if the
attribute doesn't have the exact same type as the
one we added via PHPDoc in the stubs
- otherwise a notice is emitted

To be honest, this migration path seems to be slightly overcomplicated for
me. So personally, I would be just fine with the ideas I collected in the
beginning of my message for reducing impact.
However, if you think that it makes some sense, I'm ok with going with
something along the lines of it.

Regards:
M=C3=A1t=C3=A9

--000000000000be11ad05bb3ecd33--