Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114065
MIME-Version: 1.0
References: <CAH5C8xUqzFJ4V0uo7_FviMq3Sy9Da3MWNMhh+neMO8ZjEnyQWg@mail.gmail.com>
 <CAF+90c8s0+GvL378hhxnah315ae69_56LO=pMXnQisA5-E-5Lw@mail.gmail.com>
In-Reply-To: <CAF+90c8s0+GvL378hhxnah315ae69_56LO=pMXnQisA5-E-5Lw@mail.gmail.com>
Date: Sun, 18 Apr 2021 15:51:39 +0200
Message-ID: <CAH5C8xUOBPhe3Z-TugkaQCSQjx41gM_c8P7t-WudSr9FaW6MMQ@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000d336d605c03f832e"
Subject: Re: [PHP-DEV] [RFC] [Discussion] Adding return types to internal methods
From: kocsismate90@gmail.com (=?UTF-8?B?TcOhdMOpIEtvY3Npcw==?=)

--000000000000d336d605c03f832e
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Thank you, Nikita, for the wise wrap-up about the general problem and the
possible solutions!

I think as far as the original motivation is concerned (adding return types
> to internal methods), the RFC as proposed does well. The only thing I wou=
ld
> change is to add the ReflectionMethod::getTentativeReturnType() part
> independently of the userland aspect. I believe it's important that this =
is
> exposed in reflection, even if the more general userland functionality is
> not provided. For example, if you have a mock generator, you'll probably
> want to either add the tentative return type as a real return type to
> mocks, or at least automatically add the #[SuppressReturnTypeNotice]
> attribute.
>

Initially, the reflection-related changes were a core part of the RFC, but
since I didn't find a really good use-case
for ReflectionMethod::getTentativeReturnType(), I moved it to the secondary
part. I think your example highlights really well that it's needed indeed,
I've just put it back to the core proposal.


> I'm more ambivalent about the userland side of this proposal. As Nicolas
> has already pointed out, the RFC as proposed requires the library to have=
 a
> PHP 8.1 minimum version requirement to make use of this functionality. An=
d
> libraries requiring PHP 8.1 are likely not the target audience for this
> feature -- at least not when it comes to adding return types at all, ther=
e
> might still be a use when it comes to changing them in the future, as
> typesystem capabilities are expanded.
>

I agree here as well, as neither I am a fan of the current implementation.
Especially after you made me realize that it doesn't even solve the general
problem completely... That being said, I've just got rid of the
userland-related part of the RFC, since this is something somebody can
implement later after giving some more thought for the feature.

Now that the controversial part has been removed from the RFC, I'd like to
start the vote shortly (late next week), in case there is not any more
feedback. Finally, I didn't rename the SuppressReturnTypeNotice attribute,
but I'm open to do so if you strongly prefer TentativeReturnType of
ReturnTypeWillChange (although I like the latter one more).

M=C3=A1t=C3=A9

--000000000000d336d605c03f832e--