Edit report at https://bugs.php.net/bug.php?id=55554&edit=1
ID: 55554
Updated by: gron@php.net
Reported by: ryan at zuttonet dot com
Summary: Trait methods overriding legacy constructors
-Status: Assigned
+Status: To be documented
Type: Bug
Package: Scripting Engine problem
Operating System: Ubuntu
PHP Version: 5.4.0alpha3
Assigned To: gron
Block user comment: N
Private report: N
New Comment:
This bug has been fixed in SVN.
Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
Thank you for the report, and for helping us make PHP better.
Previous Comments:
------------------------------------------------------------------------
[2011-10-09 11:16:06] gron@php.net
This bug has been fixed in SVN.
Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
Thank you for the report, and for helping us make PHP better.
------------------------------------------------------------------------
[2011-10-09 11:13:03] gron@php.net
Automatic comment from SVN on behalf of gron
Revision: http://svn.php.net/viewvc/?view=revision&revision=317935
Log: Fixed Bug #55554 (Legacy constructors not handled properly) [TRAITS] [DOC]
# The handling of legacy constructors defined by traits was corrected.
# They are now properly registered and used on instantiation.
# The situation for conflicting legacy and __construct constructors is
# mostly identical. If they are defined in the class, they override conflicts
# and do not collide. However, in case different styles are mixed, between
# class and trait definition, we assume a programmer's mistake and report
# a collision.
#
# BTW: +1 for all the fixed tests! make test is fun again.
------------------------------------------------------------------------
[2011-09-14 06:17:01] ryan at zuttonet dot com
Thanks for the clarification; while that answer isn't very satisfying, it's sort
of hard to argue when the problem is so deeply rooted in the language.
Is this inconsistency in constructor handling a design decision or a fundamental
error in the engine? Are there any plans in place to enforce consistency?
If you're going to add those tests, then I guess there's not much left to do here.
------------------------------------------------------------------------
[2011-09-12 19:27:40] gron@php.net
Hi Ryan:
(I wonder whether I could answer directly to those emails sent be the tracker,
this here manually is rather messy ...)
My main point is, that PHP does handle classes that define both kinds of
constructors in a very specific way. And that is consistent with how it is handled
when traits are used.
The answer to your question, whether a trait should be able to override the
constructor of a class in which it is used, is no. The class definition should
always take precedence. However, the problem here is deeper. Since legacy
constructor and __construct are not considered equal, we got a problem.
The inequality allows you to define both in a class, as my previous example
showed. And well, for the same reason it also works with traits, of course with
unintended side effects.
I will see what pops up when I fix the issue that you can not define legacy
constructors with a trait. I will probably add a test that constructors are
handled more carefully in traits then. The WTF factor is much higher and not worth
keeping the consistency, I think.
Best regards
Stefan
------------------------------------------------------------------------
[2011-09-12 17:38:08] ryan at zuttonet dot com
Thanks, Stefan (the defect directions explicitly say not to include examples
over 20 lines; that's why I provided the github link)
The real issue is this: should we be allowed to define, in a trait, a method
that will override the constructor of the class into which we're importing the
trait?
I think not, but currently it can be done, although not consistently.
Example 1:
<?php
/**
* ============
* Defect: can alias-override class constructor, but only when using legacy
constructor naming convention
* ============
*/
trait SomeTrait {
private function constructor() {
echo "this is abuse\n";
}
}
class SomeClass {
use SomeTrait {
constructor as __construct;
}
public function SomeClass() {
echo "this is a constructor";
}
}
$c = new SomeClass(); // Fatal error: Call to private SomeClass::__construct()
In this example, I can override SomeClass::SomeClass() as the constructor by
importing SomeTrait::constructor() as SomeClass::__construct(). However, this
works only because I'm using the legacy constructor SomeClass::SomeClass(). If I
had defined SomeClass::__construct() in SomeClass' definition, PHP would not
allow this trait abuse.
Example 2:
<?php
/**
* ============
* Defect: can override class constructor, but only when using legacy
constructor naming convention
* ============
*/
trait SomeTrait {
private function __construct() {
echo "this is abuse\n";
}
}
class SomeClass {
use SomeTrait;
public function SomeClass() {
echo "this is a constructor";
}
}
$c = new SomeClass(); // Fatal error: Call to private SomeClass::__construct()
In this case, I can override the legacy constructor SomeClass::SomeClass() with
a method name SomeTrait::__construct(). However, if I had a method named
SomeClass::__construct(), it would not be overridden by the trait method.
Do my concerns make more sense now? It's not about what _should_ be allowed,
it's about the inconsistency in what currently is allowed.
In order to be consistent, either both types of constructors should be
overridable, or neither should (probably the latter).
Again, it's about the overriding of an existing constructor, not about the
mixing in of one into a class with no constructor.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=55554
--
Edit this bug report at https://bugs.php.net/bug.php?id=55554&edit=1