Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114071
MIME-Version: 1.0
References: <CAH5C8xUqzFJ4V0uo7_FviMq3Sy9Da3MWNMhh+neMO8ZjEnyQWg@mail.gmail.com>
 <CAF+90c8s0+GvL378hhxnah315ae69_56LO=pMXnQisA5-E-5Lw@mail.gmail.com> <CAH5C8xUOBPhe3Z-TugkaQCSQjx41gM_c8P7t-WudSr9FaW6MMQ@mail.gmail.com>
In-Reply-To: <CAH5C8xUOBPhe3Z-TugkaQCSQjx41gM_c8P7t-WudSr9FaW6MMQ@mail.gmail.com>
Date: Mon, 19 Apr 2021 16:43:35 +0200
Message-ID: <CAOWwgp=hHyPMmeOoFAVW9Ja7O2GsT3xSxkEwpte=LOas2p_4gg@mail.gmail.com>
To: =?UTF-8?B?TcOhdMOpIEtvY3Npcw==?= <kocsismate90@gmail.com>
Cc: Nikita Popov <nikita.ppv@gmail.com>, PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000006b648a05c0545b84"
Subject: Re: [PHP-DEV] [RFC] [Discussion] Adding return types to internal methods
From: nicolas.grekas+php@gmail.com (Nicolas Grekas)

--0000000000006b648a05c0545b84
Content-Type: text/plain; charset="UTF-8"

> 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
> would
> > 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.
> And
> > 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,
> there
> > 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).
>

I strongly prefer either TentativeReturnType or ReturnTypeWillChange yes
(ReturnTypeWillChange would work for me.)
They're more declarative and can be re-used by userland without a need for
any RFC from internals.
Actually, I plan to use them in Symfony to help us add return types asap.
Once this is changed in the RFC, I'll be +1 on it.

Thanks for doing the hard work,
Nicolas

--0000000000006b648a05c0545b84--