Remove inefficient caching from PhpMethodReflection#3534
Remove inefficient caching from PhpMethodReflection#3534ondrejmirtes merged 15 commits intophpstan:2.0.xfrom
Conversation
|
You've opened the pull request against the latest branch 2.0.x. PHPStan 2.0 is not going to be released for months. If your code is relevant on 1.12.x and you want it to be released sooner, please rebase your pull request and change its target to 1.12.x. |
|
as of now this PR only implements the |
af3a0c8 to
0b51b89
Compare
|
This pull request has been marked as ready for review. |
|
PhpFunctionReflection too please. |
here we go |
ondrejmirtes
left a comment
There was a problem hiding this comment.
Also, having SimpleParserTest unit test to check for the attributes existence would also be nice.
c91049b to
15fc96d
Compare
21e164b to
2496e8a
Compare
|
btw: On my mac with this PR we are ~1-2 seconds slower when running |
There was a problem hiding this comment.
We don't need this property. We should always read the last entry in classStack
There was a problem hiding this comment.
AnonymousClassNode is not contained in ClassLike
There was a problem hiding this comment.
Err, what? final class AnonymousClassNode extends Class_, class Class_ extends ClassLike.
There was a problem hiding this comment.
This isn't sufficiently unique. This was recently fixed elsewhere by #3328.
There was a problem hiding this comment.
This should do the class@anonymous thing that's done by the visitor too. I don't think we'll be able to replicate the anonymous class index here and we should do more something like a column instead.
There was a problem hiding this comment.
I did not find column information in the reflection api.
instead I used start/end-of line combination of the class definition for now.
There was a problem hiding this comment.
because of this special case I had to re-introduce the inClassLike property to make sure the keys of a anonymous class don't contain a namespace. thats required so I am able to build a matching array-offset in PhpMethodReflection.
|
with the latest push we are now able to properly handle |
This reverts commit 083aaf3b64b2a539ac11db1bb5ebbc0574af9967.
|
This pull request has been marked as ready for review. |
|
Hey, I still saw too many things I'd do differently so I rolled up my sleeves and did them: d4d5c1a No tests are failing for me. Feel free to write more tests if you think your code covered a scenario that mine doesn't. Thanks! |
|
thank you. thats way simpler - awesome. |
| return TrinaryLogic::createFromBoolean($this->acceptsNamedArguments); | ||
| } | ||
|
|
||
| private function isFunctionNodeVariadic(Function_ $node): bool |
There was a problem hiding this comment.
I wonder why this $node->params logic is no longer reflected in the patch. it seems it was dead code before
There was a problem hiding this comment.
It's really old code, maybe there just wasn't a test for it. Since we're in 2.0.x, we can risk it, and maybe introduce it back if it breaks someone's scenario.
There was a problem hiding this comment.
I think it was unnecessary logic I implemented with 0ee67d5#diff-3ac4e5d4a520618a490963b70e3fcd428d56aea61683641020393167fff2ca5d
deleting the lines on 2.0.x before this PR, also doesn't break tests :).
all good.
|
It's a bit slower but maybe we'll find opportunities elsewhere :) Thank you. |
|
thank you! |
|
Oh yeah, I did it: 19cd999 Previously |
|
In my measurements it's now fast as before without writing anything to disk. |
as described in phpstan/phpstan#11748 (comment)