Corrected trait analysis: method ordering, abstract trait methods do not override concrete ones, method aliases now recognized #652
Conversation
|
@kukulich I see this is a draft: is it still WIP on your end? |
|
We discussed this privately with @kukulich - there's no assert about the |
|
@ondrejmirtes What do you think? |
|
@kukulich It still doesn't work in some cases - if you make the parent |
kukulich
force-pushed
the
trait-abstract
branch
2 times, most recently
from
1df774e
to
e7025b5
Compare
kukulich
changed the title
kukulich
force-pushed
the
trait-abstract
branch
2 times, most recently
from
315a1e3
to
91355f7
Compare
|
@Ocramius Please just merge it. I don't want to see the code again for some time... :) |
|
@Ocramius This is ready to review too :) |
| assert($parent instanceof self); | ||
| if ($this->cachedParentClass === null) { | ||
| $parent = $this->reflector->reflect($this->node->extends->toString()); | ||
| // @TODO use actual `ClassReflector` or `FunctionReflector`? |
There was a problem hiding this comment.
I think this TODO is solved for now: we can improve on the type system of reflectors later on.
There was a problem hiding this comment.
Not relevant for this patch: can be removed later
| $this, | ||
| $method->getAliasName(), | ||
| return array_merge( | ||
| [], |
There was a problem hiding this comment.
BTW, since we are on PHP 7.4+, array_merge([], ...$others) is no longer needed: https://3v4l.org/HUtnH
There was a problem hiding this comment.
Not relevant for this patch: maybe interesting for slevomat/coding-standard :D
| $parent = $this->reflector->reflect($this->node->extends->toString()); | ||
| // @TODO use actual `ClassReflector` or `FunctionReflector`? | ||
| assert($parent instanceof self); | ||
| if ($this->cachedParentClass === null) { |
There was a problem hiding this comment.
Caching this is a nice addition 👍
Ocramius
changed the title
Fixes #647
Fixes #659
Also fixes methods order: https://3v4l.org/LiTi4