Adding return types to internal methods (PHP 8)

  106539
August 11, 2019 08:43 nikita.ppv@gmail.com (Nikita Popov)
Hi internals,

Something that came up in the arginfo thread: We can now add type
annotations for everything, apart from return types on methods of non-final
classes.

The reason is that adding these return types would require inheriting
classes to specify them as well. The same problem does not exist for
argument types thanks to https://wiki.php.net/rfc/parameter-no-type-variance
(which is exactly why we wanted that RFC).

For example, if we add a "string" return type to DateTimeZone::getName(),
then any userland child of DateTimeZone would also have to specify a string
return type.

The good news: Userland classes can already specify those types *now*,
because adding a return type in a child class is legal. This means that if
we add those return types, userland extensions do not have to bump their
minimum requirement to PHP 8 when adding the return type: They can still be
compatible all the way down to PHP 7 (or 7.1, depending on the type).

What do you think about this? As we are currently annotating everything
with types, and we're at a major version, this would be the ideal time to
make this change. But there's certainly a BC break here. (And, for the
record, this is not the type of BC break where P++ or editions help,
without creating a larger mess.)

Regards,
Nikita
  106542
August 11, 2019 12:37 pollita@php.net (Sara Golemon)
On Sun, Aug 11, 2019 at 3:44 AM Nikita Popov ppv@gmail.com> wrote:

> What do you think about this? As we are currently annotating everything > with types, and we're at a major version, this would be the ideal time to > make this change. But there's certainly a BC break here. (And, for the > record, this is not the type of BC break where P++ or editions help, > without creating a larger mess.) > > I think your original proposal was spot on. It added to the language without creating any BC breaks. The ideal outcome.
This modification adds an admittedly small BC break for an equally small benefit. -1 from me. -Sara P.S. - Perhaps a way to the middle might be to introduce implicit return type hints. If a child method is implemented with no return type specified on a parent method which has one, then the parent's type is assumed at bind time, then we provide a better runtime error if that assumption is violated in a strict_types=1 context which is the only place where strict covariance of return types matters to PHP.
  106543
August 11, 2019 12:50 nikita.ppv@gmail.com (Nikita Popov)
On Sun, Aug 11, 2019 at 2:37 PM Sara Golemon <pollita@php.net> wrote:

> On Sun, Aug 11, 2019 at 3:44 AM Nikita Popov ppv@gmail.com> wrote: > >> What do you think about this? As we are currently annotating everything >> with types, and we're at a major version, this would be the ideal time to >> make this change. But there's certainly a BC break here. (And, for the >> record, this is not the type of BC break where P++ or editions help, >> without creating a larger mess.) >> >> I think your original proposal was spot on. It added to the language > without creating any BC breaks. The ideal outcome. > This modification adds an admittedly small BC break for an equally small > benefit. > > -1 from me. > > -Sara > > P.S. - Perhaps a way to the middle might be to introduce implicit return > type hints. If a child method is implemented with no return type specified > on a parent method which has one, then the parent's type is assumed at bind > time, then we provide a better runtime error if that assumption is violated > in a strict_types=1 context which is the only place where strict covariance > of return types matters to PHP. >
This is an interesting idea. There's still a BC break here, but it's more limited and only applies to code that is actively violating type contracts (that were previously implicit). Imho this should not be bound to strict_types though: strict_types controls coercion behavior of scalar type annotations, and nothing else. It should not be overloaded with this additional meaning (and I don't think the BC break is nowhere near large enough to justify this overloading). Do you have this in mind as something for use by internal classes only, or as a general language feature? I can see the use as a general language feature (see for example the debacle where PHPUnit added "void" annotations to some methods). The migration problem of adding return types is certainly not limited to internal classes. On the other hand it does seem somewhat weird that an (inherited) return type will be enforced that was not explicitly given in the source. Regards, Nikita
  106545
August 11, 2019 13:01 pollita@php.net (Sara Golemon)
On Sun, Aug 11, 2019 at 7:51 AM Nikita Popov ppv@gmail.com> wrote:

> On Sun, Aug 11, 2019 at 2:37 PM Sara Golemon <pollita@php.net> wrote: > > P.S. - Perhaps a way to the middle might be to introduce implicit return > > type hints. If a child method is implemented with no return type > specified > > on a parent method which has one, then the parent's type is assumed at > bind > > time, then we provide a better runtime error if that assumption is > violated > > in a strict_types=1 context which is the only place where strict > covariance > > of return types matters to PHP. > > > > This is an interesting idea. There's still a BC break here, but it's more > limited and only applies to code that is actively violating type contracts > (that were previously implicit). Imho this should not be bound to > strict_types though: strict_types controls coercion behavior of scalar type > annotations, and nothing else. It should not be overloaded with this > additional meaning (and I don't think the BC break is nowhere near large > enough to justify this overloading). > > Fair. Just trying to tread as light as possible, but there is a vanishing point.
> Do you have this in mind as something for use by internal classes only, or > as a general language feature? I can see the use as a general language > feature (see for example the debacle where PHPUnit added "void" annotations > to some methods). The migration problem of adding return types is certainly > not limited to internal classes. On the other hand it does seem somewhat > weird that an (inherited) return type will be enforced that was not > explicitly given in the source. > > I agree that it would be as useful to userspace classes as internal ones. It would allow library authors to embrace strong typing without breaking
consumers who are already conforming to docblock types. It's probably worth proof-of-concepting this and making an RFC irrespective of the push to annotate all the things. If feasible and accepted, I'd be much more on board with including return types for internal methods in that plan however. -Sara
  106546
August 11, 2019 13:23 nikita.ppv@gmail.com (Nikita Popov)
On Sun, Aug 11, 2019 at 3:01 PM Sara Golemon <pollita@php.net> wrote:

> On Sun, Aug 11, 2019 at 7:51 AM Nikita Popov ppv@gmail.com> wrote: > >> On Sun, Aug 11, 2019 at 2:37 PM Sara Golemon <pollita@php.net> wrote: >> > P.S. - Perhaps a way to the middle might be to introduce implicit return >> > type hints. If a child method is implemented with no return type >> specified >> > on a parent method which has one, then the parent's type is assumed at >> bind >> > time, then we provide a better runtime error if that assumption is >> violated >> > in a strict_types=1 context which is the only place where strict >> covariance >> > of return types matters to PHP. >> > >> >> This is an interesting idea. There's still a BC break here, but it's more >> limited and only applies to code that is actively violating type contracts >> (that were previously implicit). Imho this should not be bound to >> strict_types though: strict_types controls coercion behavior of scalar >> type >> annotations, and nothing else. It should not be overloaded with this >> additional meaning (and I don't think the BC break is nowhere near large >> enough to justify this overloading). >> >> Fair. Just trying to tread as light as possible, but there is a > vanishing point. > > >> Do you have this in mind as something for use by internal classes only, or >> as a general language feature? I can see the use as a general language >> feature (see for example the debacle where PHPUnit added "void" >> annotations >> to some methods). The migration problem of adding return types is >> certainly >> not limited to internal classes. On the other hand it does seem somewhat >> weird that an (inherited) return type will be enforced that was not >> explicitly given in the source. >> >> I agree that it would be as useful to userspace classes as internal > ones. It would allow library authors to embrace strong typing without > breaking consumers who are already conforming to docblock types. > > It's probably worth proof-of-concepting this and making an RFC > irrespective of the push to annotate all the things. If feasible and > accepted, I'd be much more on board with including return types for > internal methods in that plan however. >
After thinking about this a bit, this is going to be pretty hard to implement. Currently return type checks are performed using a separate opcode, which means that we need to know the return type at compile-time. At the point where inheritance happens and we know the parent return type, we are no longer able to modify the opcodes of the function. We'd have to integrate the return type check into the main return logic, with a possible negative impact on performance... Nikita
  106547
August 11, 2019 14:29 nicolas.grekas+php@gmail.com (Nicolas Grekas)
Le dim. 11 août 2019 à 15:24, Nikita Popov ppv@gmail.com> a écrit :

> On Sun, Aug 11, 2019 at 3:01 PM Sara Golemon <pollita@php.net> wrote: > > > On Sun, Aug 11, 2019 at 7:51 AM Nikita Popov ppv@gmail.com> > wrote: > > > >> On Sun, Aug 11, 2019 at 2:37 PM Sara Golemon <pollita@php.net> wrote: > >> > P.S. - Perhaps a way to the middle might be to introduce implicit > return > >> > type hints. If a child method is implemented with no return type > >> specified > >> > on a parent method which has one, then the parent's type is assumed at > >> bind > >> > time, then we provide a better runtime error if that assumption is > >> violated > >> > in a strict_types=1 context which is the only place where strict > >> covariance > >> > of return types matters to PHP. > >> > > >> > >> This is an interesting idea. There's still a BC break here, but it's > more > >> limited and only applies to code that is actively violating type > contracts > >> (that were previously implicit). Imho this should not be bound to > >> strict_types though: strict_types controls coercion behavior of scalar > >> type > >> annotations, and nothing else. It should not be overloaded with this > >> additional meaning (and I don't think the BC break is nowhere near large > >> enough to justify this overloading). > >> > >> Fair. Just trying to tread as light as possible, but there is a > > vanishing point. > > > > > >> Do you have this in mind as something for use by internal classes only, > or > >> as a general language feature? I can see the use as a general language > >> feature (see for example the debacle where PHPUnit added "void" > >> annotations > >> to some methods). The migration problem of adding return types is > >> certainly > >> not limited to internal classes. On the other hand it does seem somewhat > >> weird that an (inherited) return type will be enforced that was not > >> explicitly given in the source. > >> > >> I agree that it would be as useful to userspace classes as internal > > ones. It would allow library authors to embrace strong typing without > > breaking consumers who are already conforming to docblock types. > > > > It's probably worth proof-of-concepting this and making an RFC > > irrespective of the push to annotate all the things. If feasible and > > accepted, I'd be much more on board with including return types for > > internal methods in that plan however. > > > > After thinking about this a bit, this is going to be pretty hard to > implement. Currently return type checks are performed using a separate > opcode, which means that we need to know the return type at compile-time. > At the point where inheritance happens and we know the parent return type, > we are no longer able to modify the opcodes of the function. We'd have to > integrate the return type check into the main return logic, with a possible > negative impact on performance... >
Coincidentally, we're working on exactly this in SYmfony: we want to add return-type declarations in v5 (due in November) and we want to provide a continuous upgrade path so that apps that are using v4.4 are warned with a deprecation asking them to add the return type. We're doing this using `@return` annotation in the docblocks: when one is found in base Symfony class, child classes that don't declare their return type get the notice. Because we don't want Symfony to trigger the deprecation when extending itself, we silence it when a child class has an explicit `@return` annotation. This is the way to opt-out from the notice, while telling our userland what they should do, before we do. The related PR is here if you want to have a look: https://github.com/symfony/symfony/pull/30323 I don't know if you'll find applicable, but an idea for PHP itself could be that internal classes expose the planned return-type via a docblock, accessible by `ReflectionMethod::getDocComment()`. This way we could build userland tools to leverage it and trigger deprecations. In general on this thread, I agree with Nikita it would be desireable to add return types, and I also agree with Sara that it should not be a hard BC break. I hope this helped a bit :) NIcolas
  106548
August 11, 2019 16:33 pollita@php.net (Sara Golemon)
On Sun, Aug 11, 2019 at 8:24 AM Nikita Popov ppv@gmail.com> wrote:

> After thinking about this a bit, this is going to be pretty hard to > implement. Currently return type checks are performed using a separate > opcode, which means that we need to know the return type at compile-time. > At the point where inheritance happens and we know the parent return type, > we are no longer able to modify the opcodes of the function. We'd have to > integrate the return type check into the main return logic, with a possible > negative impact on performance... > > Well, we certainly can't do it without some perf impact. 100% agree there. The naïve and simple solution is expensive enough that I'd agree
it's not worth doing. I can see a path to get fairly close to zero, but at the cost of a non-trivial amount of complexity. Hrmmm Yeah, ultimately I'm not sure that gives us enough to justify the cost. Okay, forget that tack pending someone being able to come up with an implementation that doesn't suck. -Sara On Sun, Aug 11, 2019 at 8:24 AM Nikita Popov ppv@gmail.com> wrote:
> On Sun, Aug 11, 2019 at 3:01 PM Sara Golemon <pollita@php.net> wrote: > >> On Sun, Aug 11, 2019 at 7:51 AM Nikita Popov ppv@gmail.com> >> wrote: >> >>> On Sun, Aug 11, 2019 at 2:37 PM Sara Golemon <pollita@php.net> wrote: >>> > P.S. - Perhaps a way to the middle might be to introduce implicit >>> return >>> > type hints. If a child method is implemented with no return type >>> specified >>> > on a parent method which has one, then the parent's type is assumed at >>> bind >>> > time, then we provide a better runtime error if that assumption is >>> violated >>> > in a strict_types=1 context which is the only place where strict >>> covariance >>> > of return types matters to PHP. >>> > >>> >>> This is an interesting idea. There's still a BC break here, but it's more >>> limited and only applies to code that is actively violating type >>> contracts >>> (that were previously implicit). Imho this should not be bound to >>> strict_types though: strict_types controls coercion behavior of scalar >>> type >>> annotations, and nothing else. It should not be overloaded with this >>> additional meaning (and I don't think the BC break is nowhere near large >>> enough to justify this overloading). >>> >>> Fair. Just trying to tread as light as possible, but there is a >> vanishing point. >> >> >>> Do you have this in mind as something for use by internal classes only, >>> or >>> as a general language feature? I can see the use as a general language >>> feature (see for example the debacle where PHPUnit added "void" >>> annotations >>> to some methods). The migration problem of adding return types is >>> certainly >>> not limited to internal classes. On the other hand it does seem somewhat >>> weird that an (inherited) return type will be enforced that was not >>> explicitly given in the source. >>> >>> I agree that it would be as useful to userspace classes as internal >> ones. It would allow library authors to embrace strong typing without >> breaking consumers who are already conforming to docblock types. >> >> It's probably worth proof-of-concepting this and making an RFC >> irrespective of the push to annotate all the things. If feasible and >> accepted, I'd be much more on board with including return types for >> internal methods in that plan however. >> > > After thinking about this a bit, this is going to be pretty hard to > implement. Currently return type checks are performed using a separate > opcode, which means that we need to know the return type at compile-time. > At the point where inheritance happens and we know the parent return type, > we are no longer able to modify the opcodes of the function. We'd have to > integrate the return type check into the main return logic, with a possible > negative impact on performance... > > Nikita > >
  107199
September 18, 2019 08:59 jmquarck@gmail.com
Hi internals,

I appologise for writing here without warning, and without being an active
member of Internals. But I did not find any other public way to fairly
contribute to the conversation: externals.io, though is claim ("opening
#internals to the outside") is actually closed, and writing a blog post or
a tweet thread looked unfair to me.

About all these RFC discussions and the PHP evolution, from the outside it
seems too petty. In my opinion, PHP lacks of the ambition to pursue a
long-term vision. Any vision would be far better than having none.
Otherwise you just write code, which is actually your job, but without
meaning. What kind of language PHP would like to be? Having a purpose to
accomplish would facilitate RFC priorization, and pull more implication
from people.

A clear mission, a narrow focus, does not necessarily mean a narrow feature
set. Python is an example for this: they decided to fight against Go for
its prominence, and they achieved spreading Python features to data, to
asynchronous, etc. Plenty of old Python contributors recently mentioned how
impossible has become for them to keep on track with all new features
Python provides now. In comparison, PHP walks like the elders.

PHP RFC's may stay as a draft, or under discussion, for years. In 2019 this
looks is demoralizing when the whole RFC process should be invigorating for
the community.

My claim is for you all to leave aside these minor disagreements and
discuss, internally as well as with the community, what that mission might
be. PHP has strengths no other general purpose language has, why not
leverage them?

 Actually, this is not the topic for my writing here, but I also suggest
spending some time, or appointing someone, to improve this whole
communication toolset: it is old-fashioned, and stops contribution.


Best.
Jordi Martinez

Missatge de Nikita Popov ppv@gmail.com> del dia dg., 11 d’ag. 2019
a les 10:44:

> Hi internals, > > Something that came up in the arginfo thread: We can now add type > annotations for everything, apart from return types on methods of non-final > classes. > > The reason is that adding these return types would require inheriting > classes to specify them as well. The same problem does not exist for > argument types thanks to > https://wiki.php.net/rfc/parameter-no-type-variance > (which is exactly why we wanted that RFC). > > For example, if we add a "string" return type to DateTimeZone::getName(), > then any userland child of DateTimeZone would also have to specify a string > return type. > > The good news: Userland classes can already specify those types *now*, > because adding a return type in a child class is legal. This means that if > we add those return types, userland extensions do not have to bump their > minimum requirement to PHP 8 when adding the return type: They can still be > compatible all the way down to PHP 7 (or 7.1, depending on the type). > > What do you think about this? As we are currently annotating everything > with types, and we're at a major version, this would be the ideal time to > make this change. But there's certainly a BC break here. (And, for the > record, this is not the type of BC break where P++ or editions help, > without creating a larger mess.) > > Regards, > Nikita >
  107200
September 18, 2019 11:11 andreas@dqxtech.net (Andreas Hennings)
I am not a voting member, but I agree with Sara Golemon.
This is just too big a BC break.

I had similar questions in my own projects, and always chose to find a
solution that does not break BC.

The strategy was:
- Keep existing classes and interfaces stable.
- Introduce new classes or interfaces if I need to change the contract
in a BC-breaking way.
- Where possible, use inheritance to bridge old vs new.

Interface Named {
  function getName();
}

interface NamedV2 extends Named {
  function getName(): string;
}

This can lead to namespace clutter, but it is the responsible thing to
do to allow sustainable evolution.

The "*V2" suffix could be replaced with namespaces, e.g.
"PHP\\8\\Named" vs "PHP\\9\\Named".
Then perhaps there could be some magic to make all symbols from
"PHP\\8" available in "PHP\\9" as well, even if they are not
overridden.

-- Andreas

On Wed, 18 Sep 2019 at 11:00, <jmquarck@gmail.com> wrote:
> > Hi internals, > > I appologise for writing here without warning, and without being an active > member of Internals. But I did not find any other public way to fairly > contribute to the conversation: externals.io, though is claim ("opening > #internals to the outside") is actually closed, and writing a blog post or > a tweet thread looked unfair to me. > > About all these RFC discussions and the PHP evolution, from the outside it > seems too petty. In my opinion, PHP lacks of the ambition to pursue a > long-term vision. Any vision would be far better than having none. > Otherwise you just write code, which is actually your job, but without > meaning. What kind of language PHP would like to be? Having a purpose to > accomplish would facilitate RFC priorization, and pull more implication > from people. > > A clear mission, a narrow focus, does not necessarily mean a narrow feature > set. Python is an example for this: they decided to fight against Go for > its prominence, and they achieved spreading Python features to data, to > asynchronous, etc. Plenty of old Python contributors recently mentioned how > impossible has become for them to keep on track with all new features > Python provides now. In comparison, PHP walks like the elders. > > PHP RFC's may stay as a draft, or under discussion, for years. In 2019 this > looks is demoralizing when the whole RFC process should be invigorating for > the community. > > My claim is for you all to leave aside these minor disagreements and > discuss, internally as well as with the community, what that mission might > be. PHP has strengths no other general purpose language has, why not > leverage them? > > Actually, this is not the topic for my writing here, but I also suggest > spending some time, or appointing someone, to improve this whole > communication toolset: it is old-fashioned, and stops contribution. > > > Best. > Jordi Martinez > > Missatge de Nikita Popov ppv@gmail.com> del dia dg., 11 d’ag. 2019 > a les 10:44: > > > Hi internals, > > > > Something that came up in the arginfo thread: We can now add type > > annotations for everything, apart from return types on methods of non-final > > classes. > > > > The reason is that adding these return types would require inheriting > > classes to specify them as well. The same problem does not exist for > > argument types thanks to > > https://wiki.php.net/rfc/parameter-no-type-variance > > (which is exactly why we wanted that RFC). > > > > For example, if we add a "string" return type to DateTimeZone::getName(), > > then any userland child of DateTimeZone would also have to specify a string > > return type. > > > > The good news: Userland classes can already specify those types *now*, > > because adding a return type in a child class is legal. This means that if > > we add those return types, userland extensions do not have to bump their > > minimum requirement to PHP 8 when adding the return type: They can still be > > compatible all the way down to PHP 7 (or 7.1, depending on the type). > > > > What do you think about this? As we are currently annotating everything > > with types, and we're at a major version, this would be the ideal time to > > make this change. But there's certainly a BC break here. (And, for the > > record, this is not the type of BC break where P++ or editions help, > > without creating a larger mess.) > > > > Regards, > > Nikita > >