[RFC] Interface Default Methods

Unnamed Person

3 years ago

On 15-6-2023 5:47, Levi Morrison via internals wrote:

> Hello, PHP Internals,
>
> I am moving my RFC for interface default methods to discussion:
> https://wiki.php.net/rfc/interface-default-methods.
>
> This can be a useful tool for a few reasons:
>   1. It can make implementing an interface easier when certain methods
> can be implemented by other methods in the interface. For example, if
> `Countable` had an `isEmpty(): bool` method, it could be implemented
> by doing `$this->count() > 0`. Of course, if an implementation can be
> more efficient, they are still free to implement it how they want.
>   2. It can mitigate BC breaks in some cases. It's somewhat common for
> authors to want to expand new methods onto existing interfaces over
> time. Although this would still be a BC break, it moves it from a
> massive change (every single implementor must add something, even if
> it's a stub, or it will fail to compile) to a naming collision issue
> only (only classes which already had a method of the same name will
> fail to compile).
>
> There is prior art for this feature in both Java and C#. There may be
> other languages, but I was aware of at least these.
>
> Note that the RFC links to a partial implementation. If there are two
> or more interfaces with default methods of the same shape (name, args,
> etc) and a class implements both interfaces and doesn't provide a
> concrete implementation, which default implementation should be
> chosen? There is a proposal for resolving this in some cases which is
> modelled after Java's implementation, but it isn't implemented.
>
> Thank you for your time. I look forward to productive feedback.
>
> Levi Morrison
>

Hi Levi,

Thanks for the RFC. The usecase seems clear.

There are two things I'm missing on an initial read of the RFC.

> Adding a default implementation to an existing interface method will

not break existing code, because every existing usage has a higher
priority than the default.

A: How would this work if the existing method in the implementing class
has a different function signature than the newly introduced method in
the interface ?
There are two aspects to this:
1. What if the existing method has different parameters ?
2. What if the existing method has the same parameters, but
different/incompatible parameter/return types ?

B: How does the ability to add default method implementations to an
interface compare to providing an abstract class implementing the
interface and providing the default method implementations ?

Smile,
Juliette

Unnamed Person

3 years ago

On Thu, Jun 15, 2023 at 12:22 AM Juliette Reinders Folmer
<php-internals_nospam@adviesenzo.nl> wrote:

>
> On 15-6-2023 5:47, Levi Morrison via internals wrote:
> > I am moving my RFC for interface default methods to discussion:
> > https://wiki.php.net/rfc/interface-default-methods.
>
> There are two things I'm missing on an initial read of the RFC.
>
>  > Adding a default implementation to an existing interface method will
> not break existing code, because every existing usage has a higher
> priority than the default.
>
> A: How would this work if the existing method in the implementing class
> has a different function signature than the newly introduced method in
> the interface ?
> There are two aspects to this:
> 1. What if the existing method has different parameters ?
> 2. What if the existing method has the same parameters, but
> different/incompatible parameter/return types ?

You mis-interpreted what I was trying to say, so I'll try to clarify
that first, and then secondly address your question.

If there an existing interface, say:

interface Interface1 { function method1(); }

The RFC is saying that if you added a default implementation to
`method1` without changing its signature at all, then that would not
be a compatibility issue. All existing implementers have somehow
implemented this method already, so adding a default will do nothing
(it will be unused).

As to your questions about incompatible signatures, it's not any
different from any other interface method. If you add a new method to
the interface, and it's not compatible with some existing method on an
implementer, then it's a compatibility issue. That's not changing with
default methods. What is changing is that if a new method is added to
an interface, and it has a default implementation, it's way less
likely to be a compatibility break, because we only have to worry
about methods that share the same name. Without default methods,
essentially all additions would be compatibility break.

> B: How does the ability to add default method implementations to an
> interface compare to providing an abstract class implementing the
> interface and providing the default method implementations ?

A key difference is that you can implement many interfaces, but can
only extend a single class (abstract or not). For this reason, many
contracts are formed with interfaces and relatively fewer with
abstract classes. Well, that's my experience, anyway.

Unnamed Person

3 years ago

On 15-6-2023 16:52, Levi Morrison via internals wrote:

> On Thu, Jun 15, 2023 at 12:22 AM Juliette Reinders Folmer
> <php-internals_nospam@adviesenzo.nl> wrote:
>> On 15-6-2023 5:47, Levi Morrison via internals wrote:
>>> I am moving my RFC for interface default methods to discussion:
>>> https://wiki.php.net/rfc/interface-default-methods.
>> There are two things I'm missing on an initial read of the RFC.
>>
>>   > Adding a default implementation to an existing interface method will
>> not break existing code, because every existing usage has a higher
>> priority than the default.
>>
>> A: How would this work if the existing method in the implementing class
>> has a different function signature than the newly introduced method in
>> the interface ?
>> There are two aspects to this:
>> 1. What if the existing method has different parameters ?
>> 2. What if the existing method has the same parameters, but
>> different/incompatible parameter/return types ?
> You mis-interpreted what I was trying to say, so I'll try to clarify
> that first, and then secondly address your question.
>
> If there an existing interface, say:
>
>      interface Interface1 { function method1(); }
>
> The RFC is saying that if you added a default implementation to
> `method1` without changing its signature at all, then that would not
> be a compatibility issue. All existing implementers have somehow
> implemented this method already, so adding a default will do nothing
> (it will be unused).
>
> As to your questions about incompatible signatures, it's not any
> different from any other interface method. If you add a new method to
> the interface, and it's not compatible with some existing method on an
> implementer, then it's a compatibility issue. That's not changing with
> default methods. What is changing is that if a new method is added to
> an interface, and it has a default implementation, it's way less
> likely to be a compatibility break, because we only have to worry
> about methods that share the same name. Without default methods,
> essentially all additions would be compatibility break.
>
>> B: How does the ability to add default method implementations to an
>> interface compare to providing an abstract class implementing the
>> interface and providing the default method implementations ?
> A key difference is that you can implement many interfaces, but can
> only extend a single class (abstract or not). For this reason, many
> contracts are formed with interfaces and relatively fewer with
> abstract classes. Well, that's my experience, anyway.
>

Fair enough and thank you for your response. You were right, I
copy/pasted the wrong sentence to quote.

I still believe this information should be added to the RFC as the risk
of adding new methods to an interface which collide with existing
methods in implementations of that interface is very real, though I
agree with Deleu that this BC impact is not so much of the RFC, but of
what can happen once the RFC would be accepted.

While I can understand the tendency to defer this question to "that's an
issue for userland when userland adds new methods to existing
interfaces" (or PHP itself if new methods would be added to PHP native
interfaces), I don't think that's fair as this RFC is the one which
opens the door to that possibility.

Smile,
Juliette

Unnamed Person

3 years ago

> I still believe this information should be added to the RFC as the risk
> of adding new methods to an interface which collide with existing
> methods in implementations of that interface is very real, though I
> agree with Deleu that this BC impact is not so much of the RFC, but of
> what can happen once the RFC would be accepted.

Juliette, it's in the RFC under the second bullet point here:
https://wiki.php.net/rfc/interface-default-methods#backward_incompatible_changes.
Is there something specific you'd like added there?

Deleu

3 years ago

>
> I still believe this information should be added to the RFC as the risk
> of adding new methods to an interface which collide with existing
> methods in implementations of that interface is very real, though I
> agree with Deleu that this BC impact is not so much of the RFC, but of
> what can happen once the RFC would be accepted.
>
> While I can understand the tendency to defer this question to "that's an
> issue for userland when userland adds new methods to existing
> interfaces" (or PHP itself if new methods would be added to PHP native
> interfaces), I don't think that's fair as this RFC is the one which
> opens the door to that possibility.
>
> Smile,
> Juliette
>

Adding a new method to an existing interface is already a BC break today
and requires a major version bump, otherwise implementers of the interface
will get a fatal error because they're missing implementing all methods of
an interface. We can call this a "Userland BC Path" (how a userland can
make a BC to other users). This RFC doesn't open this door, it actually
creates a path which makes this Userland BC Path less "damaging". If an
interface provider adds a new method to an interface (already a BC Break),
but does so by adding a default implementation, then such a change will
break less code than just adding a method without a default implementation
because only code that already had the exact same method name would now
break.

One can argue that this change might make it so that users start
considering adding methods with default implementation as
not-so-much-a-bc-break and do so without bumping a major version, in which
case this RFC could be said to "open the door for some users to start
introducing BC-breaks without bumping major version" because they consider
it a much smaller BC break, but it can't be said to open the possibility.

Does this make sense?

--
Marco Deleu

Michael Babker

3 years ago

> On Thursday, Jun 15, 2023 at 12:01 PM, Deleu <deleugyn@gmail.com (mailto:deleugyn@gmail.com)> wrote:
>
> One can argue that this change might make it so that users start
> considering adding methods with default implementation as
> not-so-much-a-bc-break and do so without bumping a major version, in which
> case this RFC could be said to "open the door for some users to start
> introducing BC-breaks without bumping major version" because they consider
> it a much smaller BC break, but it can't be said to open the possibility.

At this point, though, how different is the impact from this type of B/C break from the B/C break that already occurs when new methods are added to non-final classes where a subclass used a different signature and isn’t compatible with the new addition?

- Michael

Deleu

3 years ago

I loved the proposal and I think this has the potential to be a huge
developer experience improvement, perhaps at least as great as property
constructor promotion and match().

Just a side note though, in the Backward Compatibility section, I would
think that only "None" should be declared. From the PHP binary perspective,
running code before and after this change has absolutely 0 impact/breaking
changes.
The feature may introduce a new way for *Users of PHP* to break BC with
*Other Users of PHP*. The language change itself has no impact on PHP code
written prior to the feature. The additional note about how users may break
BC by using the feature would be a description of the feature itself, thus
might be best declared as part of the Proposal instead.

I wish us all the best of luck with this proposal.

On Thu, Jun 15, 2023 at 12:48 AM Levi Morrison via internals <
internals@lists.php.net> wrote:

> Hello, PHP Internals,
>
> I am moving my RFC for interface default methods to discussion:
> https://wiki.php.net/rfc/interface-default-methods.
>
> This can be a useful tool for a few reasons:
>  1. It can make implementing an interface easier when certain methods
> can be implemented by other methods in the interface. For example, if
> `Countable` had an `isEmpty(): bool` method, it could be implemented
> by doing `$this->count() > 0`. Of course, if an implementation can be
> more efficient, they are still free to implement it how they want.
>  2. It can mitigate BC breaks in some cases. It's somewhat common for
> authors to want to expand new methods onto existing interfaces over
> time. Although this would still be a BC break, it moves it from a
> massive change (every single implementor must add something, even if
> it's a stub, or it will fail to compile) to a naming collision issue
> only (only classes which already had a method of the same name will
> fail to compile).
>
> There is prior art for this feature in both Java and C#. There may be
> other languages, but I was aware of at least these.
>
> Note that the RFC links to a partial implementation. If there are two
> or more interfaces with default methods of the same shape (name, args,
> etc) and a class implements both interfaces and doesn't provide a
> concrete implementation, which default implementation should be
> chosen? There is a proposal for resolving this in some cases which is
> modelled after Java's implementation, but it isn't implemented.
>
> Thank you for your time. I look forward to productive feedback.
>
> Levi Morrison
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>

--
Marco Deleu

Tim Düsterhus

3 years ago

Hi

On 6/15/23 17:08, Deleu wrote:

> The feature may introduce a new way for *Users of PHP* to break BC with
> *Other Users of PHP*. The language change itself has no impact on PHP code
> written prior to the feature. The additional note about how users may break
> BC by using the feature would be a description of the feature itself, thus
> might be best declared as part of the Proposal instead.
>

There's also the possible impact with regard to static analysis tools,
IDEs and formatters that do not expect a method body to exist within
interfaces and thus might erroneously report such an interface as
invalid. This might be considered an impact on code written prior to the
feature existing.

Best regards
Tim Düsterhus

Unnamed Person

3 years ago

On Thu, Jun 15, 2023 at 9:16 AM Tim Düsterhus <tim@bastelstu.be> wrote:

>
> Hi
>
> On 6/15/23 17:08, Deleu wrote:
> > The feature may introduce a new way for *Users of PHP* to break BC with
> > *Other Users of PHP*. The language change itself has no impact on PHP code
> > written prior to the feature. The additional note about how users may break
> > BC by using the feature would be a description of the feature itself, thus
> > might be best declared as part of the Proposal instead.
> >
>
> There's also the possible impact with regard to static analysis tools,
> IDEs and formatters that do not expect a method body to exist within
> interfaces and thus might erroneously report such an interface as
> invalid. This might be considered an impact on code written prior to the
> feature existing.
>
> Best regards
> Tim Düsterhus

I have added a brief section in RFC impact that code analysis tools
will need to be updated. This is implicitly true for many language
features, but I see no harm in calling it out in the RFC, so I have
done so. Thank you for the feedback.

Ilija Tovilo

3 years ago

Hi Levi

> I am moving my RFC for interface default methods to discussion:
> https://wiki.php.net/rfc/interface-default-methods.

This or a similar concept makes sense to me. The proposal seems
similar to Swift protocol extensions, or Rust traits, with the
exception that function default implementations may only be defined in
the interface itself.

Note that there's a large overlap between this proposal and extending
traits to allow implementing interfaces.
https://wiki.php.net/rfc/traits-with-interfaces The main difference is
how you would use the feature from a given class, i.e. using an
interface implementation or a trait usage. Implementing interfaces
from traits would require declaring both a trait and an interface. I
do think your proposal is the more natural approach.

The redundancy of interfaces and traits after this RFC are also
somewhat unfortunate. Both interfaces and traits could inject default
behavior into classes. Both could enforce implementation of methods in
classes (traits through abstract methods). My intuition is that
interface default implementations should be used for public APIs
(because this provides an abstracted interface), while traits should
be used for protected/private ones (because non-public methods can't
be added to interfaces). The other obvious difference is that
interfaces don't allow manual conflict resolution while traits do.

The RFC doesn't mention default implementations for static methods.
I'm not sure there's a use case but it might make sense to explicitly
mention whether they are supported.

Ilija

Unnamed Person

3 years ago

On Thu, Jun 15, 2023 at 3:37 PM Ilija Tovilo <tovilo.ilija@gmail.com> wrote:

> > I am moving my RFC for interface default methods to discussion:
> > https://wiki.php.net/rfc/interface-default-methods.
>
> The RFC doesn't mention default implementations for static methods.
> I'm not sure there's a use case but it might make sense to explicitly
> mention whether they are supported.

Interesting point. I sometimes forget that `static` is allowed in
interfaces. When I rebase the branch, I'll check it out. I assume it
works at a technical level without issues. If that's true, then absent
further evidence, I would say to allow it.

Levi Morrison

3 years ago

On Thu, Jun 15, 2023 at 5:41 PM Levi Morrison via internals
<internals@lists.php.net> wrote:

>
> On Thu, Jun 15, 2023 at 3:37 PM Ilija Tovilo <tovilo.ilija@gmail.com> wrote:
> > > I am moving my RFC for interface default methods to discussion:
> > > https://wiki.php.net/rfc/interface-default-methods.
> >
> > The RFC doesn't mention default implementations for static methods.
> > I'm not sure there's a use case but it might make sense to explicitly
> > mention whether they are supported.
>
> Interesting point. I sometimes forget that `static` is allowed in
> interfaces. When I rebase the branch, I'll check it out. I assume it
> works at a technical level without issues. If that's true, then absent
> further evidence, I would say to allow it.

I have rebased my branch on the latest master, and added a test case
that default methods work with static methods on interfaces. They do.
If you can think of any interesting test cases to add in that space,
feel free.

PR: https://github.com/php/php-src/pull/11467

Larry Garfield

3 years ago

On Thu, Jun 15, 2023, at 3:47 AM, Levi Morrison via internals wrote:

> Hello, PHP Internals,
>
> I am moving my RFC for interface default methods to discussion:
> https://wiki.php.net/rfc/interface-default-methods.
>
> This can be a useful tool for a few reasons:
>  1. It can make implementing an interface easier when certain methods
> can be implemented by other methods in the interface. For example, if
> `Countable` had an `isEmpty(): bool` method, it could be implemented
> by doing `$this->count() > 0`. Of course, if an implementation can be
> more efficient, they are still free to implement it how they want.
>  2. It can mitigate BC breaks in some cases. It's somewhat common for
> authors to want to expand new methods onto existing interfaces over
> time. Although this would still be a BC break, it moves it from a
> massive change (every single implementor must add something, even if
> it's a stub, or it will fail to compile) to a naming collision issue
> only (only classes which already had a method of the same name will
> fail to compile).
>
> There is prior art for this feature in both Java and C#. There may be
> other languages, but I was aware of at least these.
>
> Note that the RFC links to a partial implementation. If there are two
> or more interfaces with default methods of the same shape (name, args,
> etc) and a class implements both interfaces and doesn't provide a
> concrete implementation, which default implementation should be
> chosen? There is a proposal for resolving this in some cases which is
> modelled after Java's implementation, but it isn't implemented.
>
> Thank you for your time. I look forward to productive feedback.

This would essentially replace the semi-common pattern of providing an interface, and a trait that implements most of the interface for you.

I have many conflicting thoughts on this RFC.

On the plus side, the flexibility it provides sounds delightful, and it would effectively render abstract classes almost entirely pointless.

On the down side, multiple inheritance is often avoided for a reason, and as Ilija noted it doesn't *completely* replace traits and base classes as it doesn't apply to non-public methods.

From a BC perspective it seems fine.  It's at worst the same as now, and potentially makes extending existing interfaces easier.

I think I am overall positive, but still a bit uneasy and having a hard time articulating why.

--Larry Garfield

Deleu

3 years ago

On Thu, Jun 15, 2023 at 12:48 AM Levi Morrison via internals <
internals@lists.php.net> wrote:

> Hello, PHP Internals,
>
> I am moving my RFC for interface default methods to discussion:
> https://wiki.php.net/rfc/interface-default-methods.
>
> This can be a useful tool for a few reasons:
>  1. It can make implementing an interface easier when certain methods
> can be implemented by other methods in the interface. For example, if
> `Countable` had an `isEmpty(): bool` method, it could be implemented
> by doing `$this->count() > 0`. Of course, if an implementation can be
> more efficient, they are still free to implement it how they want.
>  2. It can mitigate BC breaks in some cases. It's somewhat common for
> authors to want to expand new methods onto existing interfaces over
> time. Although this would still be a BC break, it moves it from a
> massive change (every single implementor must add something, even if
> it's a stub, or it will fail to compile) to a naming collision issue
> only (only classes which already had a method of the same name will
> fail to compile).
>
> There is prior art for this feature in both Java and C#. There may be
> other languages, but I was aware of at least these.
>
> Note that the RFC links to a partial implementation. If there are two
> or more interfaces with default methods of the same shape (name, args,
> etc) and a class implements both interfaces and doesn't provide a
> concrete implementation, which default implementation should be
> chosen? There is a proposal for resolving this in some cases which is
> modelled after Java's implementation, but it isn't implemented.
>
> Thank you for your time. I look forward to productive feedback.
>
> Levi Morrison
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>

A question just occurred to me. Building up on the example of the RFC, is
the following snippet valid and would it behave as expected?

```
interface Interface1 {
function method1() { echo __METHOD__ . "\n"; }
}

interface Interface2 {
function method1() { echo __METHOD__ . "\n"; }
}

class Class1 implements Interface1, Interface2 {
function method1() {
$result = Interface1::method1();

Interface2::method1();

return $result;
}
}

$result = (new Class1())->method1();
```

--
Marco Deleu

Unnamed Person

3 years ago

On Fri, Jun 16, 2023 at 11:51 AM Deleu <deleugyn@gmail.com> wrote:

>
>
> On Thu, Jun 15, 2023 at 12:48 AM Levi Morrison via internals <internals@lists.php.net> wrote:
>>
>> Hello, PHP Internals,
>>
>> I am moving my RFC for interface default methods to discussion:
>> https://wiki.php.net/rfc/interface-default-methods.
>>
>> This can be a useful tool for a few reasons:
>>  1. It can make implementing an interface easier when certain methods
>> can be implemented by other methods in the interface. For example, if
>> `Countable` had an `isEmpty(): bool` method, it could be implemented
>> by doing `$this->count() > 0`. Of course, if an implementation can be
>> more efficient, they are still free to implement it how they want.
>>  2. It can mitigate BC breaks in some cases. It's somewhat common for
>> authors to want to expand new methods onto existing interfaces over
>> time. Although this would still be a BC break, it moves it from a
>> massive change (every single implementor must add something, even if
>> it's a stub, or it will fail to compile) to a naming collision issue
>> only (only classes which already had a method of the same name will
>> fail to compile).
>>
>> There is prior art for this feature in both Java and C#. There may be
>> other languages, but I was aware of at least these.
>>
>> Note that the RFC links to a partial implementation. If there are two
>> or more interfaces with default methods of the same shape (name, args,
>> etc) and a class implements both interfaces and doesn't provide a
>> concrete implementation, which default implementation should be
>> chosen? There is a proposal for resolving this in some cases which is
>> modelled after Java's implementation, but it isn't implemented.
>>
>> Thank you for your time. I look forward to productive feedback.
>>
>> Levi Morrison
>>
>> --
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: https://www.php.net/unsub.php
>>
>
> A question just occurred to me. Building up on the example of the RFC, is the following snippet valid and would it behave as expected?
>
> ```
> interface Interface1 {
>     function method1() { echo __METHOD__ . "\n"; }
> }
>
> interface Interface2 {
>     function method1() { echo __METHOD__ . "\n"; }
> }
>
> class Class1 implements Interface1, Interface2 {
>     function method1() {
>         $result = Interface1::method1();
>
>         Interface2::method1();
>
>         return $result;
>     }
> }
>
> $result = (new Class1())->method1();
> ```
>
>
> --
> Marco Deleu

I'm not sure why you are saving `null` from `Interface1::method1()`'s
implicit return and then returning it from inside `Class1::method1`,
but yes, this is valid.

Alexandru Pătrănescu

3 years ago

On Thu, Jun 15, 2023, 06:48 Levi Morrison via internals <
internals@lists.php.net> wrote:

> Hello, PHP Internals,
>
> I am moving my RFC for interface default methods to discussion:
> https://wiki.php.net/rfc/interface-default-methods.
>
> Thank you for your time. I look forward to productive feedback.
>
>

Hi Levi,

Thank you for this!
I like the proposal and I think it fits well.
I'm used to it from Java and I don't know any reason for it to be
considered a bad practice since it was introduce, about 9-10 years ago.

1. Do we want to allow also private methods in the interface so that it
allows code reuse or better code organization in smaller methods? Java
introduced this only in the next version.
The private methods would be invisible to implementing classes and to
anything else, of course.
We can also add them at a later point but it might be better to have the
feature complete now.

Sidenote: I don't think protected method should ever exist on interfaces;
that should remain for abstract classes use case.

2. The only use case for myself for ever using traits would be if traits
would offer a default implementation for an interface. Now with a default
implementation offered by interface itself, sounds even better.
However, I can see how there could be more than one provided default
implementation that can be offered.
Would a method implemented by a trait have higher precedence over the
interface default implementation?
Would a trait offered implementation be directly usable by the interface?

Thanks,
Alex

Levi Morrison

3 years ago

On Sat, Jun 17, 2023 at 6:05 AM Alexandru Pătrănescu <drealecs@gmail.com> wrote:

>
> Thank you for this!
> I like the proposal and I think it fits well.
> I'm used to it from Java and I don't know any reason for it to be
> considered a bad practice since it was introduce, about 9-10 years ago.
>
> 1. Do we want to allow also private methods in the interface so that it
> allows code reuse or better code organization in smaller methods? Java
> introduced this only in the next version.
> The private methods would be invisible to implementing classes and to
> anything else, of course.
> We can also add them at a later point but it might be better to have the
> feature complete now.
>
> Sidenote: I don't think protected method should ever exist on interfaces;
> that should remain for abstract classes use case.

Sorry for the delay in responding to your message. I have implemented
support for private methods for people to experiment with. As far as I
can tell, it works as expected and I don't see any issues with them.
Of course, they only make sense as helper methods to the public
default methods.

I'd like to hear what others think about allowing private interface
methods that have method bodies, but I think it's easy and sensible.

> 2. The only use case for myself for ever using traits would be if traits
> would offer a default implementation for an interface. Now with a default
> implementation offered by interface itself, sounds even better.
> However, I can see how there could be more than one provided default
> implementation that can be offered.
> Would a method implemented by a trait have higher precedence over the
> interface default implementation?
> Would a trait offered implementation be directly usable by the interface?

I'm not entirely sure what you are asking about traits, so I'll try to
clarify. In a class which uses a trait to implement an interface, that
trait method will take priority over the interface's default method. I
will add a test to the PR to make this more obvious.

Alexandru Pătrănescu

3 years ago

On Tue, Jun 20, 2023 at 7:30 AM Levi Morrison <levim@php.net> wrote:

> On Sat, Jun 17, 2023 at 6:05 AM Alexandru Pătrănescu <drealecs@gmail.com>
> wrote:
> >
> > 1. Do we want to allow also private methods in the interface so that it
> > allows code reuse or better code organization in smaller methods?
>
> Sorry for the delay in responding to your message. I have implemented
> support for private methods for people to experiment with. As far as I
> can tell, it works as expected and I don't see any issues with them.
> Of course, they only make sense as helper methods to the public
> default methods.
>

> I'd like to hear what others think about allowing private interface
> methods that have method bodies, but I think it's easy and sensible.
>

Looks good to me. Thank you!

>
> > Would a method implemented by a trait have higher precedence over the
> > interface default implementation?
> > Would a trait offered implementation be directly usable by the interface?
>
> I'm not entirely sure what you are asking about traits, so I'll try to
> clarify. In a class which uses a trait to implement an interface, that
> trait method will take priority over the interface's default method. I
> will add a test to the PR to make this more obvious.
>

Yes, that looks good, this was the first question, exactly.


Sorry for the lack of clarity in my second question.
Is it if it would be allowed to have something like:

trait Trait1 {
function method1() { echo __METHOD__, "\n"; }
}

interface Interface1 {
use Trait1;
function method1();
}

So both trait and interface can be used independently without code
duplication.

A more complex scenario where this might be useful:

// external package
interface Interface1 {
function method1();
}
interface Interface2 {
function method2();
}

// internal package
trait Trait1 {
function method1() { echo __METHOD__, "\n"; }
}
trait Trait2 {
function method2() { echo __METHOD__, "\n"; }
}
interface Interface3 extends Interface1, Interface2 {
use Trait1;
use Trait2;
}

Where it will allow either usage of a class for:
- Interface1 with Trait1
- Interface2 with Trait2
- Interface3 that would already have the method implemented.

Hope it's clearer now.
I don't know if we want to support use of a trait in the interface
but I think the RFC should mention that it is or is not supported.

Thank you,
Alex

David Gebler

3 years ago

On Thu, Jun 15, 2023 at 4:48 AM Levi Morrison via internals <
internals@lists.php.net> wrote:

>
> I am moving my RFC for interface default methods to discussion:
> https://wiki.php.net/rfc/interface-default-methods.
>
>

Can I ask, the RFC doesn't say - does your implementation ensure default
implementations can only call other methods which exist on the interface in
the same manner as Java? i.e. the following would give some sort of error?

interface A {
public function foo(): void {
$this->bar();
}
}

class B implements A {
public function bar(): void {
...
}
}

But the following would be okay?

interface A {
public function foo(): void {
$this->bar();
}

public function bar(): void;
}

Unnamed Person

3 years ago

On Sat, Jun 17, 2023 at 4:10 PM David Gebler <davidgebler@gmail.com> wrote:

>
> On Thu, Jun 15, 2023 at 4:48 AM Levi Morrison via internals <internals@lists.php.net> wrote:
>>
>>
>> I am moving my RFC for interface default methods to discussion:
>> https://wiki.php.net/rfc/interface-default-methods.
>>
>
> Can I ask, the RFC doesn't say - does your implementation ensure default implementations can only call other methods which exist on the interface in the same manner as Java? i.e. the following would give some sort of error?
>
> interface A {
>     public function foo(): void {
>         $this->bar();
>     }
> }
>
> class B implements A {
>     public function bar(): void {
>         ...
>     }
> }
>
> But the following would be okay?
>
> interface A {
>     public function foo(): void {
>         $this->bar();
>     }
>
>     public function bar(): void;
> }
>

No, there's no attempt to ensure the method body adheres to calling
the public interface. Due to PHP's possible dynamic behaviors, I don't
think it's reasonable to attempt to enforce it at compile-time. I'm
not sure it's worth the effort trying to enforce it at runtime either,
but it would be nice to see lints from static analysis tools which
detect this issue.

David Gebler

3 years ago

On Mon, Jun 19, 2023 at 3:53 AM Levi Morrison <levi.morrison@datadoghq.com>
wrote:

>
> No, there's no attempt to ensure the method body adheres to calling
> the public interface. Due to PHP's possible dynamic behaviors, I don't
> think it's reasonable to attempt to enforce it at compile-time. I'm
> not sure it's worth the effort trying to enforce it at runtime either,
> but it would be nice to see lints from static analysis tools which
> detect this issue.
>

Okay, thanks. That's really quite significant, since it changes the feature
to one which could allow changes made to interfaces to adversely impact
existing clients of an interface without those clients changing a thing.

I was excited by the description of the RFC, but I was imagining something
more like how Java does it. Personally, this limitation (or rather lack of
it) is enough that I wouldn't support it and I'd urge anyone who can vote
to carefully consider the implications of the implementation being proposed.

Rowan Collins

3 years ago

On 19/06/2023 20:20, David Gebler wrote:

> Okay, thanks. That's really quite significant, since it changes the feature
> to one which could allow changes made to interfaces to adversely impact
> existing clients of an interface without those clients changing a thing.

As the RFC says, it introduces a form of multiple inheritance, and
inheritance in PHP doesn't make any such guarantee - to adapt your example:

class A {
public function foo(): void {
$this->bar();
}
}

class B extends A {
public function bar(): void {
...
}
}

The job of detecting that class A doesn't define a contract for method
bar falls to static analysers, and the same would be true for default
implementations on interfaces.


Regards,

--
Rowan Tommins
[IMSoP]

David Gebler

3 years ago

On Mon, Jun 19, 2023 at 9:53 PM Rowan Tommins <rowan.collins@gmail.com>
wrote:

> On 19/06/2023 20:20, David Gebler wrote:
> > Okay, thanks. That's really quite significant, since it changes the
> feature
> > to one which could allow changes made to interfaces to adversely impact
> > existing clients of an interface without those clients changing a thing.
>
>
> As the RFC says, it introduces a form of multiple inheritance, and
> inheritance in PHP doesn't make any such guarantee - to adapt your example:
>
> class A {
>      public function foo(): void {
>          $this->bar();
>      }
> }
>
> class B extends A {
>      public function bar(): void {
>          ...
>      }
> }
>
> The job of detecting that class A doesn't define a contract for method
> bar falls to static analysers, and the same would be true for default
> implementations on interfaces.
>
>
>
>

Sure, but in this example were B::bar() to be a private method, you would
get a scope violation error. You would not if you were to use a trait,
since any methods implemented in the trait would be executed in the same
scope as the class using the trait. And that seems to be what's being
proposed here for interface defaults. So my concern here is that when you
use a trait, you know that what you're doing is effectively
copy-and-pasting implementation into your class scope. You don't expect
that when you declare a class to implement an interface.

There's some degree of irony to me that we have another RFC, the Override
attribute, which will introduce an engine-level check that could easily
(and arguably should) be left to static analysis tools, yet we're talking
here about introducing a mechanism where private methods on a class could
be called by stealth by a mere interface. I'm not even sure static analysis
tools would in their typical configuration pick up on a situation like
this, either, since it could in this scenario sit in a vendor dependency
and not your own code.

I like the idea of this RFC - in fact it's one which has been near top of
my wishlist for PHP language features for a long time - but I think this is
an issue with the proposed implementation which at the very least warrants
highlighting and discussion.

Levi Morrison

3 years ago

> I like the idea of this RFC - in fact it's one which has been near top of
> my wishlist for PHP language features for a long time - but I think this is
> an issue with the proposed implementation which at the very least warrants
> highlighting and discussion.

I understand your concern but personally believe it's overblown. This
problem already exists with abstract classes, but doesn't seem to be
that much of an issue in practice. I hope static analysis can fill the
gap here, but don't think these checks are necessary to ship this
feature.

David Gebler

3 years ago

On Tue, 20 Jun 2023, 04:10 Levi Morrison, <levim@php.net> wrote:

> > I like the idea of this RFC - in fact it's one which has been near top of
> > my wishlist for PHP language features for a long time - but I think this
> is
> > an issue with the proposed implementation which at the very least
> warrants
> > highlighting and discussion.
>
> I understand your concern but personally believe it's overblown. This
> problem already exists with abstract classes, but doesn't seem to be
> that much of an issue in practice. I hope static analysis can fill the
> gap here, but don't think these checks are necessary to ship this
> feature.
>

Yeah I suppose I'm just saying "Interface default methods" can be
interpreted a few different ways and there's key differences between
Java-style, versus syntax sugar for mixing an interface and a trait in to
one unit, versus a user can effectively extend multiple abstract classes
but with interface keyword.

Appreciate it appears to still be a WIP and you may not have decided or
designed all the finer details yet but I think at the very least, the RFC
text needs to be updated as soon as practical to be much more explicit
about these details so voters know exactly what they're voting on when the
time comes.

Cheers.

Rowan Collins

3 years ago

On Tue, 20 Jun 2023 at 13:29, David Gebler <davidgebler@gmail.com> wrote:

> Yeah I suppose I'm just saying "Interface default methods" can be
> interpreted a few different ways and there's key differences between
> Java-style, versus syntax sugar for mixing an interface and a trait in to
> one unit, versus a user can effectively extend multiple abstract classes
> but with interface keyword.
>

As far as I can see, "Java-style", and " effectively extend multiple
abstract classes" are actually the same thing, and per my e-mail that's
what I think the RFC is proposing. The "mixing an interface and a trait"
interpretation would not lead to the method resolution shown.

Additional constraints in Java around calling *public* methods outside the
interface are just a consequence of how it performs static analysis and
compilation on *all* classes, not something specific to interface default
methods. The lack of those constraints in PHP is likewise not something
that's specific to this proposal, the language *always* allows you to
attempt any method call on any object, including $this, even if a type
checker would say the call is invalid.

Regards,

--
Rowan Tommins
[IMSoP]

Levi Morrison

3 years ago

On Tue, Jun 20, 2023 at 6:29 AM David Gebler <davidgebler@gmail.com> wrote:

>
> On Tue, 20 Jun 2023, 04:10 Levi Morrison, <levim@php.net> wrote:
>
> > > I like the idea of this RFC - in fact it's one which has been near top of
> > > my wishlist for PHP language features for a long time - but I think this
> > is
> > > an issue with the proposed implementation which at the very least
> > warrants
> > > highlighting and discussion.
> >
> > I understand your concern but personally believe it's overblown. This
> > problem already exists with abstract classes, but doesn't seem to be
> > that much of an issue in practice. I hope static analysis can fill the
> > gap here, but don't think these checks are necessary to ship this
> > feature.
> >
>
>
> Yeah I suppose I'm just saying "Interface default methods" can be
> interpreted a few different ways and there's key differences between
> Java-style, versus syntax sugar for mixing an interface and a trait in to
> one unit, versus a user can effectively extend multiple abstract classes
> but with interface keyword.

I will update the RFC soon. In my head it was so obvious that it would
be similar to regular inheritance and less like traits, to the extent
I didn't even realize I'd need to specify it. This is why discussion
periods can be valuable; the RFC doesn't always say what's in the RFC
author's head :)

Unnamed Person

3 years ago

On Tue, Jun 20, 2023 at 9:46 PM Levi Morrison <levim@php.net> wrote:

>
> On Tue, Jun 20, 2023 at 6:29 AM David Gebler <davidgebler@gmail.com> wrote:
> >
> > On Tue, 20 Jun 2023, 04:10 Levi Morrison, <levim@php.net> wrote:
> >
> > > > I like the idea of this RFC - in fact it's one which has been near top of
> > > > my wishlist for PHP language features for a long time - but I think this
> > > is
> > > > an issue with the proposed implementation which at the very least
> > > warrants
> > > > highlighting and discussion.
> > >
> > > I understand your concern but personally believe it's overblown. This
> > > problem already exists with abstract classes, but doesn't seem to be
> > > that much of an issue in practice. I hope static analysis can fill the
> > > gap here, but don't think these checks are necessary to ship this
> > > feature.
> > >
> >
> >
> > Yeah I suppose I'm just saying "Interface default methods" can be
> > interpreted a few different ways and there's key differences between
> > Java-style, versus syntax sugar for mixing an interface and a trait in to
> > one unit, versus a user can effectively extend multiple abstract classes
> > but with interface keyword.
>
> I will update the RFC soon. In my head it was so obvious that it would
> be similar to regular inheritance and less like traits, to the extent
> I didn't even realize I'd need to specify it. This is why discussion
> periods can be valuable; the RFC doesn't always say what's in the RFC
> author's head :)

I have significantly updated the RFC. There's one more topic that
needs to go in there, tentatively called 'default cancelling':

```php
interface Interface1 { function m1() { /* ... */ } }
interface Interface2 extends Interface1 {
function m1(); // may or may not change signature
}
```

For various reasons which I'll put in the RFC, this will "cancel" the
default, meaning any class which implements Interface2 instead of
Interface1 will not receive that default.

Deleu

3 years ago

On Tue, Jun 20, 2023 at 12:10 AM Levi Morrison <levim@php.net> wrote:

> > I like the idea of this RFC - in fact it's one which has been near top of
> > my wishlist for PHP language features for a long time - but I think this
> is
> > an issue with the proposed implementation which at the very least
> warrants
> > highlighting and discussion.
>
> I understand your concern but personally believe it's overblown. This
> problem already exists with abstract classes, but doesn't seem to be
> that much of an issue in practice. I hope static analysis can fill the
> gap here, but don't think these checks are necessary to ship this
> feature.
>

I agree that these checks would not be a requirement to ship this feature.
I was thinking even further just as a thought experiment. Let's pretend for
a second that these checks could easily be implemented, wouldn't it
actually make it worse for the language as a whole? The only place where a
method body would be forbidden to call `$this->foo()` ahead-of-time
(compile-time or runtime with special error) would be on an interface
default method. Everywhere else in the language would have the behavior of
resolving the scope of `$this` and then attempting to actually execute the
method. If the scope of `$this` has a `__call()` implementation, this would
never lead to a "FatalError method not found" scenario, making these checks
even weirder.

What I'm concluding is that although it could have been nice to not allow
these weird quirks, that ship has sailed decades ago and doing it on an
interface default implementation (even if it was possible) would just
create a major language inconsistency and it would always be best to
implement this RFC without it regardless of technical limitations.

--
Marco Deleu

Rowan Collins

3 years ago

On 19/06/2023 22:12, David Gebler wrote:

>
> Sure, but in this example were B::bar() to be a private method,
> you would get a scope violation error. You would not if you were to
> use a trait, since any methods implemented in the trait would be
> executed in the same scope as the class using the trait. And that
> seems to be what's being proposed here for interface defaults. So my
> concern here is that when you use a trait, you know that what
> you're doing is effectively copy-and-pasting implementation into your
> class scope. You don't expect that when you declare a class to
> implement an interface.

That's a crucially different example - it's not about whether the
interface has to declare everything it uses, but about what scope it
operates in.

The RFC should probably spell it out more clearly, but my interpretation
was that the methods are being inherited just like if they were on an
abstract class, not "pasted" like a trait - e.g. the example of
delegating to an interface method would not work with a trait. As such,
they would execute in the scope of the interface, and have access to
protected but not private members of the class that inherits them.

This is also hinted at in the exchange between Levi and Alexandru about
private methods on the interface itself, which would not be visible
anywhere outside the interface. Again, that's different from traits,
where members marked "private" are private to the target class, not the
trait.

Regards,

--
Rowan Tommins
[IMSoP]