Fix ClassNameReferences.

Authored by pprkut on Aug 14 2018, 8:31 AM.



This extends the existing support for the allowed syntax of ClassNameReferences.
As a bonus, we get rid of one first/first conflict.

Diff Detail

R52 KDevelop: PHP Support
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
pprkut created this revision.Aug 14 2018, 8:31 AM
Restricted Application added a project: KDevelop. · View Herald TranscriptAug 14 2018, 8:31 AM
Restricted Application added a subscriber: kdevelop-devel. · View Herald Transcript
pprkut requested review of this revision.Aug 14 2018, 8:31 AM
brauch added a subscriber: brauch.Aug 17 2018, 9:48 AM

Good amount of tests :) For the actual implementation, I would suggest trying to make it a bit more readable by a) splitting it up into several functions and b) trying to reduce indent depth a bit by using continue instead of nested ifs.


This is a really long function. Can you break it up a bit to make it clearer what's going on?


Is it intended that you use a null type pointer here?


here with a bit of if (!declaration) { continue; } I think the code could be much more readable. This has too much indent for my taste.

pprkut updated this revision to Diff 39935.Aug 17 2018, 4:29 PM

Split out duplicate code and reordered it a bit to make it easier to follow.

pprkut marked 3 inline comments as done.Aug 17 2018, 4:30 PM
pprkut added inline comments.

No, the entire else statement seems redundant. Unit tests still pass without it and my test files render fine with it too. Probably a left-over artifact from an earlier draft. Removed.

brauch accepted this revision.Sep 5 2018, 5:42 PM

I'm not really capable of doing an in-depth review here, since I don't know enough details of PHP nowadays. Style-wise I think it could still profit from reducing the block size of un-named (i.e. not a function with a name), uncommented complex code a bit, since it often requires quite some thinking to grasp what a certain conditional actually checks for. A simple comment like "// not a function" or whatever can do wonders there.
Feel free to merge this as-is or with a bit more comments, better get it in before the beta and still have a fix window before the release than merge it after the beta. Thanks for keeping kdev-php alive!

This revision is now accepted and ready to land.Sep 5 2018, 5:42 PM
This revision was automatically updated to reflect the committed changes.
pprkut marked an inline comment as done.