Php-parser: Using the standard format for the AST, used by PHP 7

[chris-l]

Hi!

I read your comment on #41, and I was thinking about how the resulting AST is going to be.

After checking this repository: https://github.com/nikic/php-ast, it seems there is an _official_ format for a PHP AST, that is generated by PHP 7. (I haven't tested it, but according to it's readme, this project is just exposing the AST that PHP7 generates on it's own)

One of the things I noticed there is that it seems the PHP 7 AST is formed by assoc arrays, which are like JavaScript object literals, instead of using numeric arrays.

Now, I do like how your AST looks like (since it reminds me to Lisp, and IMO, Lisp looks very elegant), but I do think an AST based on objects literals would be more practical.

Right now, in certain cases, I'm checking for the number of elements an array has, but that number will change as soon the AST requires new elements. For instance, on php-unparser class translator, I have an if (method.length === 7) to check if the method has a body (otherwise is an abstract method). But that will break if later becomes necessary to add more elements to the methods and then the number of elements a methods goes to 10 or something. On an object-based AST, I could check for something like if (!method.body) instead, and the amount of properties that the method has would be irrelevant.

But more important than just being easier and more stable, it seems to me that is really important to use and follow one standard AST.

Like in the case of esprima, they don't use their own AST, instead, they use an standard JavaScript AST spec called estree, that was originally created by mozilla.

Since the AST of PHP 7 could be considered as the official format of the AST, I think it would be better to follow that format. Perhaps even would be a good idea to create a project like estree to document the PHP7 AST and create a more formal spec.

[ichiriac]

Hi @chris-l,

The PHP RFC is here, but instead of nikic's parser, they do not define a list of AST nodes.

I am using arrays instead object for these reasons :

• less memory consumption : benchmark is hard to be accurate because of memory allocation but should be some memory allocations for properties, more than array indexes
• faster allocation : not sure for plain object as array is also an object, but for function based objects it may be the case
• faster read lookup : properties are indexed instead doing a hash lookup on properties.

But these reasons are not necessarily true, implementation between JS engines may differ, and even if it's true, differences are relevant only on micro benchmarks with high amount of data, so I will not take it into account.

Another argument is the way you can scan recursively the AST. You do not need to handle all types of nodes in order to extract for example only variables usages. But that's also true for objects, as in javascript you can iterate over object properties (but it may be slower to iterate over properties than indexes).

Another point, exporting array to json are less verbose than objects, storing AST for caching purpose with objects may increase the size, and de facto may be slower to serialize/unserialize.

On the other hand, objects are way more expressive, much easy to output debug information, makes code more readable, and you've got the point, adding a property is much easier than adding a new offset at the end of the array.

In current release I would like to make the project stable, with a standard output format. So it's time to chose between objects vs arrays.

+1 for declaring objects, by the way, generating a documentation based on it will be by far more easy with annotations.

[ichiriac]

+1 by using the same structure as https://github.com/nikic/php-ast it will be more easy to automate tests and check if AST nodes are OK, similar to how the lexer is checked with the php lexer.

[ichiriac]

@nevadascout @DaGhostman @TheColorRed, if you are interested you can also share your opinion. This change will totally break old code (if I use objects) but this will be last major change.

[DaGhostman]

To me it makes more sense to have objects instead of arrays since it allows for independent access, for example as of right now when trying to get the methods in interface and class there is a difference i think on interfaces the index was 4 bot on classes (and traits?) 5 or something along those lines, which will be solved with node.methods since clients must not care on the order things are + the already mentioned issue of adding new props vs adding new array indexes

Also regarding the memory consumption etc, I think that it is not too much of a deal since using AST will probably on majority of cases happen as a 1 off (like parse do stuff, leave) so the memory will be allocated and very quickly (instantly?) freed. IMO it will make sense if it is used in some sort of 'live preview' where on every character change the code is reparsed, then it will be a huge problem

+1 for objects

[ichiriac]

@DaGhostman,

Live preview will anyway be a problem since parsing a huge file will take some hundreds of milliseconds, so the char change may trigger a timer that will wait that the text input is paused before triggering a parsing of contents, and maybe the parsing could be partial (like parsing the function body instead all the file)

So lets go, I will change every nodes into objects.

I've made a branch for https://github.com/glayzzle/php-parser/tree/0.1.5, and I will work on https://github.com/glayzzle/php-parser/tree/1.0.0

After I will impact changes on php-reflection & php-unparser.

[ichiriac]

a new release of the 1.0.0 version is ready here : https://github.com/glayzzle/php-parser/tree/develop

AST structure is documented here : https://github.com/glayzzle/php-parser/blob/develop/docs/AST.md (I've used the estree approach)

[ichiriac]

I close this issue as the new version is released.

You can play with the new output online : http://glayzzle.com/php-parser/#demo