php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Doc Bug #77294 Parent methods can "fulfill requirements" of child trait abstract methods
Submitted: 2018-12-13 16:04 UTC Modified: 2018-12-13 19:44 UTC
Votes:1
Avg. Score:3.0 ± 0.0
Reproduced:0 of 0 (0.0%)
From: chasepeeler at gmail dot com Assigned:
Status: Open Package: Class/Object related
PHP Version: 7.0.33 OS: Windows
Private report: No CVE-ID: None
Have you experienced this issue?
Rate the importance of this bug to you:

 [2018-12-13 16:04 UTC] chasepeeler at gmail dot com
Description:
------------
If a child class uses a trait with an abstract method, while it's parent class uses a trait with a defined method of the same name, the method from the parent classes trait will act as the implementation of the abstract method.

The issue also exists in cases where a class uses two traits. One trait contains an abstract method while the other trait contains a defined method of the same name. When executed, the defined method will act as the implementation of the abstract method.

I don't know if this is a bug or an undocumented feature. If it isn't a bug, then it breaks the "compiler assisted copy/paste rule" since copy/pasting the traits into the classes would cause an error. It also appears to violate the documented behavior in relation to conflict resolution: http://php.net/manual/en/language.oop5.traits.php#language.oop5.traits.conflict. If it isn't a bug, then the documentation needs to be updated to better explain this situation.




Test script:
---------------
https://3v4l.org/OKoTY


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2018-12-13 17:22 UTC] requinix@php.net
-Summary: Abstract trait method implementation +Summary: Parent methods can "fulfill requirements" of child trait abstract methods -Type: Bug +Type: Documentation Problem
 [2018-12-13 17:22 UTC] requinix@php.net
Calling traits "copy/paste" is a simplification. They're more like mixins.

As my edit to the summary suggests, what's happening is that the parent class is defining a method (via a trait, which is unimportant), the child's trait is defining a *requirement* of a method, and the parent is meeting the requirement. Surely this is more useful than the compiler raising an error, right?

The "Abstract Trait Members" section already addresses this idea but could gain a sentence saying that the fulfilling can be done by a parent. And while someone's in there they can fix the "fullfill" spelling.
 [2018-12-13 18:12 UTC] cmb@php.net
> And while someone's in there they can fix the "fullfill" spelling.

Oops, my bad.  Fixed[1].

> Calling traits "copy/paste" is a simplification. They're more
> like mixins.

The RFC[2], however, says:

| As already mentioned, multiple inheritance and Mixins are
| complex mechanisms. Traits are an alternative which have been
| designed to impose no additional semantics on classes. […] It is
| almost like a language supported and failsafe copy'n'paste
| mechanism to build classes.

> Surely this is more useful than the compiler raising an error, right?

However, the compiler raises an error:

  Fatal error: Declaration of parentImplementedTH::bar(array $a) must be compatible with abstractBarTH::bar(int $a) in /in/OKoTY on line 57

This contradicts the statement:

| A concrete class fulfills this requirement by defining a
| concrete method with the same name; its signature may be
| different.

which was supposed to fix bug #75449.

[1] <http://svn.php.net/viewvc?view=revision&revision=346323>
[2] <https://wiki.php.net/rfc/horizontalreuse>
 [2018-12-13 18:47 UTC] chasepeeler at gmail dot com
For the record, I don't necessarily think the way it currently operates is a bad thing - but maybe it is. That's why I said that if the current behavior is correct, then the documentation should be updated to better explain the behavior.

> Calling traits "copy/paste" is a simplification. They're more like mixins.

I agree. However, most developers think of them that way, and, for the most part, it's an accurate description. It's possible this behavior is the only thing that breaks that convention. As the other comment shows, even the RFC referred to it as copy/paste.

> However, the compiler raises an error: <snip>

Whether that's another bug or not is outside the scope of this report. I implemented it that way just to show that one method was the implementation of the other. In my first example, where the signatures are the same, no error is given.

> ... what's happening is that the parent class is defining a method <snip>

What about the 3rd example where a single class uses two traits? In this example, the trait with the defined method still implements the abstract method from the other class. No parent/child relationship at all.

> The "Abstract Trait Members" section already addresses this idea

Not really, it says "A concrete class fullfills (sic) this requirement by defining a concrete method with the same name"

It doesn't mention anything about how abstract and concrete methods in different traits would interact with each other. The fact that you can implement an abstract method before the abstract method is defined (child/parent) or in the same class it's defined (two traits in a single class) isn't intuitive at all - especially in light of the section on conflict resolution: "If two Traits insert a method with the same name, a fatal error is produced, if the conflict is not explicitly resolved." - there is no qualification in there for instances where one of the methods is abstract. 

On another note, I first discovered this because PhpStorm was showing a method as being protected. It ended up that it was defined as protected abstract in a a trait used by a class. That classes parent, however, used a trait that defined a  concrete method of the same name that was public. Since it isn't very intuitive to look at an ancestor to see where an abstract method is defined, PhpStorm thought it was a protected method. Everything executed fine, though, showing that it wasn't protected.
 [2018-12-13 19:44 UTC] requinix@php.net
> The RFC[2], however, says:
> However, most developers think of them that way,
Yeah, I was trying to go for a short version of "traits are more limited than mixins, whose functionality obviously depends on the language you look at, but are more powerful than copy/paste because there are definite features that can be useful for a trait that don't match up with what would happen if a programmer were to actually copy and paste code".

> What about the 3rd example where a single class uses two traits?
It falls under the "defining a concrete method" rule. Even if the implementation is coming from another trait.

> The fact that you can implement an abstract method before the abstract method is defined
But that's not what it is. A method in a trait that uses the "abstract" keyword functions works in one of two ways: either as a requirement or as an abstract method. The using class has the option of somehow implementing the requirement, in which case the trait is happy because the method it needs is there. That implementation could come from a parent or from another trait, as long as it's there. But if the using class does not implement the requirement, the requirement still needs to be there of course, so it must get baked into the class and therefore as a regular abstract method. The class then must be abstract too and child classes are subject to the usual rules.

Which is why the example is how it is:
1. childAbstract meets the abstractBar::bar requirement by inheriting from parentImplemented::bar
2. childAbstractTH (trait version) is not valid because of LSP restrictions between the abstractBarTH::bar requirement and parentImplementedTH::bar implementation
3. childAbstractTH (copy/paste version) is not valid because bar was made abstract (and LSP restrictions)
4. childBothTH (trait version) meets the abstractBarTH::bar requirement by getting its implementation from implementedBarTH::bar
5. childBothTH (copy/paste version) is obviously not valid

#2 is the weird one. It does seem to me to be a bug that an implementing class gets away with just the name requirement but a parent implementing class has all the usual LSP restrictions.

But the rest makes sense to me. Sure, abstract methods in traits could function like copy/paste abstract methods and force a fatal error (like #5), but the way they are now provides a feature much like interfaces: for a class to use the trait, it must implement the abstract requirements or declare itself abstract too.
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Fri Apr 26 10:01:25 2019 UTC