Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » Adding return types to internal methods (PHP 8) »

Adding return types to internal methods (PHP 8)

php.internals

Nikita Popov

7 years ago
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

Sara Golemon

7 years ago
On Sun, Aug 11, 2019 at 3:44 AM Nikita Popov <nikita.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.

Nikita Popov

7 years ago
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 <nikita.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

Sara Golemon

7 years ago
On Sun, Aug 11, 2019 at 7:51 AM Nikita Popov <nikita.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

Nikita Popov

7 years ago
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 <nikita.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

Nicolas Grekas

7 years ago
Le dim. 11 août 2019 à 15:24, Nikita Popov <nikita.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 <nikita.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

Sara Golemon

7 years ago
On Sun, Aug 11, 2019 at 8:24 AM Nikita Popov <nikita.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 <nikita.ppv@gmail.com> wrote:

Unnamed Person

6 years ago
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 <nikita.ppv@gmail.com> del dia dg., 11 d’ag. 2019 a les 10:44:

Andreas Hennings

6 years ago
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: