Roave/BetterReflection

Source of a closure or arrow function

Wuffz opened this issue · 7 comments

Wuffz commented

I was working on a proof of concept at the time php 8.2 wasn't out yet.
now i wanted to pick it up and see if i can make it work and cleanup a bit, however after i updated the dependencies,

commit 3ed251d broke my code by removing some of the features i used (getBodyAst and getStmts ) in my concept, how would you suggest i fixed these bugs? is there a better workaround? or was there another good reason why it was removed ?

We stopped exposing the AST because we wanted to remove it from the memory footprint and runtime complexity.

The backing reason is that PHPStan started making heavy use of roave/better-reflection, and the performance impact became massive.

The cleanest way to implement such logic now would be to:

  1. get the reflection symbol
  2. extract the file path from reflection information
  3. parse the tree and extract the contents from there

We can also try and introduce utilities in this library, but they must be designed in a way that doesn't keep the AST inside the reflection symbols.

Wuffz commented

We stopped exposing the AST because we wanted to remove it from the memory footprint and runtime complexity.

The backing reason is that PHPStan started making heavy use of roave/better-reflection, and the performance impact became massive.

That is a extremely good reason.

The cleanest way to implement such logic now would be to:

  1. get the reflection symbol
  2. extract the file path from reflection information
  3. parse the tree and extract the contents from there

We can also try and introduce utilities in this library, but they must be designed in a way that doesn't keep the AST inside the reflection symbols.

A separate utility would be awesome. But maybe that is overkill and out of the scope of this project. My project heavily relies on closure and arrow functions.

I will look into it later today

@Wuffz depends on your will to contribute and test these improvements.

If it is useful, documented and tested well, you should certainly consider contributing it back to upstream for everyone else to benefit from it.

Wuffz commented

@Ocramius how about a ReflectionClosure class which extends the ReflectionFunction class? That would abstract it away.

I haven't looked at the amount of work, or even if it should just use ReflectFunctionAbstract trait.

This is just a mind spin, I am willing to contribute if I think it belongs in this library and not seperated

I think it would be best to have something like a separate AST traversal/retrieval utility.

final class RetrieveAst
{
    public function forClass(ReflectionClass $class): ?Ast { ... }
    public function forFunction(ReflectionFunction $function): ?Ast { ... }
    ...
}
Wuffz commented

I created a new pull request, it seems that that one isn't linked here

Closing here as per #1336 patch, which was started, but won't be finished.

If interested in this feature, please start checking from there, and provide a new patch.