new PHP Parser

Jakob Petsovits jpetso at gmx.at
Sat Jun 7 09:13:00 UTC 2008


On Saturday, 7. June 2008, Niko Sams wrote:
> I just committed a new parser for the php language to
> trunk/playground/devtools/kdevelop4-extra-plugins/php

Whoo!

> It successfully parses 12000 files (Zend Framework, Joomla, Typo3,
> phpBB3, wordpress, phpMyAdmin, drupal, horde, pear) though probably
> not 100% correct (more testing needed)

/me looks forward to do Drupal development on KDevelop4
with PHP autocompletion :)

> Please take a look at the code and review it.

Looks good at first sight.

On Saturday, 7. June 2008, New PHP Parser wrote:
> elseifList: dangling-else conflict i guess (is this a problem?)

No, dangling-else conflicts are normally not a problem.
I can't completely tell for sure, but I believe that this one is no exception.

> PAAMAYIM_NEKUDOTAYIM ("::")

Er, pretty strange name for a token?

> IS_EQUAL ("=="), (...), EQUAL ("=")

Might just be personal taste, but I think it would be good for readability to 
make those two a *bit* more easier to tell apart. Like, renaming the 
assignment operators from EQUAL to ASSIGN.

>     parameter=functionCallParameterListElement @ COMMA | 0
>  -> functionCallParameterList ;;
(and similar rules)

I found it a good idea to not have any rules that may be empty (except for 
pseudo data processing rules), as that makes stuff more predictable and 
slightly easier to debug. Most importantly, it prevents you from having the 
optional zero both in the caller rule and in the called rule.

Also, "parameter" should be a list here ("#parameter") if I get that right.

>     VarExpressionState m_varExpressionState;
>     bool m_varExpressionIsVariable;
>
>     struct ParserState {
>         VarExpressionState varExpressionState;
>         bool varExpressionIsVariable;
>     };
>     ParserState _MState;

Maybe I'm missing something, but what's the point of having those variables 
duplicated? I think you should either drop _MState or the two separate ones.
Please do also care for consistency - if you keep _MState, it should probably 
be named m_state like the other member variables that you defined.

All minor nitpicking, of course. I guess I could go on with pointless style 
issues all day, but let's instead just enjoy a fine new parser :D

Wishes,
  Jakob




More information about the KDevelop-devel mailing list