Remove runtime autoloading from NodeTypeResolver #3627
Conversation
| @@ -354,11 +349,7 @@ private function isObjectTypeOfObjectType(ObjectType $resolvedObjectType, Object | |||
| } | |||
|
|
|||
| $classReflection = $this->reflectionProvider->getClass($resolvedObjectType->getClassName()); | |||
| if (! isset($this->traitExistsCache[$classReflection->getName()])) { | |||
There was a problem hiding this comment.
I think since we already checked with hasClass that $resolvedObjectType->getClassName() exists, we only need to check with isTrait() whether its a trait.
in my thinking it should be equivalent to what this cache was doing before.
@keulinho could you verify this has the same perf characteristics for your use case, as measured in #3501 ?
There was a problem hiding this comment.
The change in general looks good to me and the solution way more elegant that mine 🙈
actually i've paused trying to get rector running smoothly with our codebase for now, i will get back when i have the time to look at it again
|
I think the failling job shows us one of the side-effects I mentioned in #3502 (comment). the test-case worked before this PR because it autoloaded the trait at runtime, which doesn't look right to me. |
It must be fixed before this gets merged, so we have always updated and working code here. Could you handle it? |
|
I think the test here would be green after rectorphp/rector-symfony#391 is merged. I was not able to test it though. |
|
build restarted |
|
@staabm test still error unfortunately |
|
@staabm I am reverting your PR rectorphp/rector-symfony#392 |
staabm
changed the title
|
should be good to go now. no changes in rector-symfony are needed anymore |
|
|
||
| if ($this->traitExistsCache[$classReflection->getName()]) { | ||
| foreach ($classReflection->getAncestors() as $ancestorClassReflection) { |
There was a problem hiding this comment.
this ancestor traversal is slow.
after this PR we only do it for trait-classes. before we did it for every class, which was unnecessary, as only traits can be contained in a traitUse.
|
fixed, thanks |
|
Thank you 😊 |
* Remove runtime autoloading from NodeTypeResolver * refactor
see #3502 (comment) for background