Migrate on own parser

[alexander-akait]

/cc @czosel

Current parser does not improve and does not fix bugs, I propose to go to your own parser, we can use https://github.com/nikic/PHP-Parser/tree/master/grammar as example. I know it is require time, but unfortunately it seems we have no other way.

[loilo]

Any chance we could viably compile the original PHP parser to WebAssembly?

[loilo]

PHP as a whole has already been compiled to WASM, but I have no insight into that field and can't tell whether compiling parts (i.e. just the parser) would be feasible.

[alexander-akait]

@loilo i am afraid what original php parsers is not enough for prettier, a lot of information about nodes is missing because php no need this

[karptonite]

Any possibility the plugin could use the https://github.com/nikic/PHP-Parser directly, or would this have the same issue with missing nodes? Obviously that means running php processes as part of prettier PHP (no necessarily simple), but it would have the benefit that it would be easier to stay up to date with future language changes.

[alexander-akait]

@karptonite same problem not enough node information 😞 I think I can implement parser using bison/yacc (https://github.com/nikic/PHP-Parser using modified version of yacc, but it is not a problem) for two weeks, but i don't have time right now, if somebody want to help me will be great.

[karptonite]

Perhaps the PHP version is extensible enough to allow it to be extended to get the additional node information? I'm looking at this issue as an example of getting information that doesn't normally get stored by the parser: nikic/PHP-Parser#384
It also looks like there are some lexer options that might give you more information: https://github.com/nikic/PHP-Parser/blob/1.x/doc/component/Lexer.markdown#lexer-options
I don't know much about how these things work, though, so I'm sort of guessing here.

[alexander-akait]

To be honestly for us interesting only https://github.com/nikic/PHP-Parser/blob/master/grammar/php5.y and https://github.com/nikic/PHP-Parser/blob/master/grammar/php7.y and https://github.com/nikic/PHP-Parser/blob/master/grammar/tokens.y

What we should do:

• Install bison/yacc
• Create js version https://github.com/nikic/PHP-Parser/blob/master/grammar/parser.template
• Rewrite this on js https://github.com/nikic/PHP-Parser/blob/master/grammar/rebuildParsers.php
• Implement each Node (example https://github.com/nikic/PHP-Parser/blob/master/grammar/php7.y#L39) on js, given the names and structure of the current nodes what we use in code, most likely we will have even less of them
• Copy https://github.com/nikic/PHP-Parser/blob/master/grammar/php5.y , https://github.com/nikic/PHP-Parser/blob/master/grammar/php7.y and https://github.com/nikic/PHP-Parser/blob/master/grammar/tokens.y

It is not hard, node(s) is very simple

[alexander-akait]

I can try to find time on PoC

[czosel]

Hi @evilebottnawi,
first of all sorry about the delay, I've been swamped with work lately.
This sounds like a big step, and before jumping right into it, I'm wondering

• Which are the show-stopping issues in the parser we're currently using right now?
• Can those issues not be fixed in the current parser?
• If we decide to implement a new parser, what would be fundamentally different in it's design that would solve the issues we're having now?

[alexander-akait]

Which are the show-stopping issues in the parser we're currently using right now?

• maximum supporting php version is 7.2 (7.3 features doesn't support)
• invalid ast in some cases (parens and *lookup nodes)
• bad ast for inline nodes
• invalid position for comments in some cases

Can those issues not be fixed in the current parser?

I think yes, but nobody do this. Also bison/yacc parser better maintenance.

If we decide to implement a new parser, what would be fundamentally different in it's design that would solve the issues we're having now?

BFN (EBFN) grammar and generate parser based on this rules. You can see https://github.com/nikic/PHP-Parser/tree/master/grammar here how it can be done.

[czosel]

Alright, thanks for sharing the details - I'll try to take a closer look when I find the time 🙂

[ichiriac]

Hi @evilebottnawi and @czosel, interresting approach. In the past I've used https://www.npmjs.com/package/jison but at that time the LR(1) generated code was not good enough to parse some syntax collisions on PHP, and by resolving them is was too permissive and also too slow compared to now, but agree that the maintenance of the syntax may be easier on grammar files than code.

Did you succeed with a POC or prototype ?

[alexander-akait]

@ichiriac not, is is just idea, it it no high priority, the problem is that we get fixes/features extremely slowly and very few guys have experience with parsers, supporting parser generator based on bfn/ebfn will be easy

[alexander-akait]

@czosel @loilo i can't continue update php parser due glayzzle/php-parser#359 and glayzzle/php-parser#358, it is big regressions for us, i think we need fork parser and revert this and continue development

[alexander-akait]

I do not have a lot of time, but in my plan to finish the stable version this month

[czosel]

@evilebottnawi I think it's fine to continue development on your fork for the moment, most likely @ichiriac will revert the change soon.
Would be awesome if we can make a new release soon! 🎉

[alexander-akait]

@czosel i think we should:

• full support php7.3
• implement nodes for <?php and ?> (need discussion) for better format inline content
• some fixes (some already done)

[czosel]

Sounds good! To me, especially better inline support seems critical - unfortunately, that’s probably also the hardest part.

[ichiriac]

@czosel - better inline support is related to glayzzle/php-parser#170
@evilebottnawi - I'll reverse bad commits, and the php 7.3 glayzzle/php-parser#345 & php 7.4 glayzzle/php-parser#346 are planned on next release

---

I think the actual approach of php-parser is clearly not maintenable - I have some nights to pass on this, so I'm ok with the idea to migrate the parser on the nikic implementation, initially I was concerned by the speed, but now with hindsight I prefer maintainability - glayzzle/php-parser#372

[alexander-akait]

@ichiriac maybe you can allow to me merge without your review (bug fixes), we do great work and our parser not a bad, i just need review when implement new feature

[alexander-akait]

/cc @czosel I have a lot of PRs with fixes and features https://github.com/glayzzle/php-parser/pulls, I can finish support php 7.3 and php 7.4 and inline nodes by the end of the month if my PR someone will merge (will be great if review also). But the developer is extremely inactive, and I can’t finish it in the right time because of this.

[czosel]

I'd also be ok with using your fork of the parser until your PRs get merged. Unfortunately, I neither have the time nor the know-how to do reviews of your work on the parser 😞

[alexander-akait]

@czosel hm, here there solution:

• do fork
• move parser to this repo (it is allow fast fixing, but increase count of PRs)
• Alternative: create branch prettier in php-parser and allow me to merge commits in this branch and use this branch, when @ichiriac will have time he can merge commit from our branch to master

[czosel]

I’d suggest either option 1 or 3. I think keeping the parser separate is important both for separation of concerns and for being able to easily merge your changes back in the upstream parser.

[alexander-akait]

/cc @ichiriac what about branch for prettier?

[ichiriac]

Hi @evilebottnawi ,

You are registered as collaborator (and also @czosel) from a long time ago, and I already gave you since then the write access level on the repository.

In principle you can merge any PR by your own, even if I prefer to check them when it comes to major changes.

Lets go with the prettier branch option, here your branch : https://github.com/glayzzle/php-parser/tree/prettier (BTW you can also create as many branches you need)

• You can make PR & merge them on it, I've started to change pending PR on it
• You also may retrieve PR from master on it in order to keep it sync
• Once you are ready to release, make a PR on master

[alexander-akait]

@ichiriac Thanks!

[jpickwell]

This may also be of some use: https://github.com/nikic/php-ast

Same developer as PHP-Parser.

[flexchar]

Somewhat off topic but this plugin has saved me a long way keeping my code base tidy. Absolutely ♥ it!

I understand, however, that PHP plugin is a volunteer project and you can only put so much work. I'd love to help! I don't understand parsers but how about financially wise? Perhaps fellow developers like me would be interested and that would also help you.

I use it consistently with PHP 7.3 (f.ex. Laravel) projects. It works flawlessly 99% of time. However, since PHP 7.4 is standing outside and knocking the door - the tense rises. :)

[alexander-akait]

@flexchar thanks for the desire to help, but we need help in the code and not in the money, we need to implement all the constructs from php7.4 and some from php7.3, here parser glayzzle/php-parser#407, it is not hard and not easy 😄

[czosel]

Thanks for the kind words @flexchar, I’m happy to hear the plugin is working well for you!
I totally agree with @evilebottnawi though, what the project currently needs most is active contributors. My knowledge on the parsing side is also limited, but I’m happy to help with onboarding in the printing side.

[joemaller]

@czosel & @evilebottnawi I also want to thank you for your work on this, the project is a huge help. I would really like to try and contribute, but having just spent a few hours attempting to get things working, I ended up stuck on setup, tooling and workflow.

I know this is a huge ask, but any additional documentation towards getting a basic development environment working would be really welcome. I've read over the Contributing doc (and Prettier's) but some things don't make sense or seem to point to missing files (the docs REPL?).

A few places I got stuck

• Basic setup: What steps should someone take between cloning the repo and starting to code on a bug?
• How do you iterate quickly? The full test suite takes 75-90 seconds to run for me and --watch doesn't work. Any setup/workflow tips would be welcome.
• How to attach the vscode debugger when testing? Do we need to run Prettier from source before we can attach to this project's plugin?

[czosel]

Hi @joemaller

I'll answer the questions right here - afterwards we can think about what can be added to the documentation.

• Basic setup: yarn && yarn test
• Iterate quickly: run a specific test file (i often use the "kitchen sink" test for temporary changes) with yarn test tests/kitchen_sink
• Debugging: I'm not using VScode, but debugging in Chrome works with node --inspect-brk node_modules/.bin/jest --runInBand tests/kitchen_sink/ (see https://jestjs.io/docs/en/troubleshooting)

Update: See #1267

[czosel]

@joemaller The docs should be a little better now - let me know if you see further improvements!