search

backdrop-contrib / coder_upgrade

Helps automate some/most of the work required to upgrade a module from Drupal to Backdrop 1.x
GNU General Public License v2.0
4 stars 7 forks source link

Upgrade plans to Grammar Parser 2.x? #72

Open smaiorana opened 2 years ago

smaiorana commented 2 years ago

Hello,

We are wondering if an upgrade to Grammar Parser 2.x is in the works or has been completed already, perhaps in a fork? @rbargerhuff and I noticed that the current version of Coder Upgrade uses the Grammar Parser 1.x library, but the module here has a 2.x branch: https://www.drupal.org/project/grammar_parser

We attempted to swap out the Grammar Parser library in-place, but it makes some fundamental changes that break several Coder Upgrade functions and hooks.

We were looking into this because the Grammar Parser 2.x library has better support for newer PHP features such as traits, and seemed to have overall better handling of object-oriented code. We thought it might be a better solution than simply back-porting everything into Grammar Parser 1.x. But thus far, it hasn't worked well and has left us will little confidence.

Cheers.

bugfolder commented 2 years ago

When I joined the maintainers a while back, I started looking at upgrading to Grammar Parser 2.x, for the reasons you mention, and fairly soon thereafter encountered the problems you mention 😉. So, at the moment, there are no plans to attempt that. We would certainly welcome someone taking on the project (which would then warrant a new branch of CU).

Keep in mind, though, that the primary application for CU is upgrading existing D7 modules, most of which don't use newer PHP features. So the important thing for an upgrade would be to cover the same upgrade issues as the existing version, plus more.

smaiorana commented 2 years ago

Thanks for the response. This makes perfect sense.

We've been running on PHP 7.4 for a while and have a lot of in-house code that is not backwards-compatible with PHP 5. We found that the Grammar Parser has a hard time handling constructs like typed class properties and nullable class properties.

Grammar Parser 2.x does handle these, so we thought it might be easier to simply upgrade the library.

But after toying around with Grammar Parser 1.x for a while, I think we've resolved the issues noted above in the existing library. We plan to confirm and submit issues/patches for them next week.

oadaeh commented 2 years ago

I thought I would point out that there is no reason why there couldn't be two branches of Coder Upgrade, one that supports the 1.x branch of Grammar Parser and one that supported the 2.x branch. Yes, that's more work, but it means that one can still be viable while the other is being working on and improved, and both can be available for whatever purpose people need.

rbargerhuff commented 2 years ago

I also see no reason why there could not be 2 branches of Coder Upgrade. However speaking for my team, due to time constraints, we are not in any position to work on Coder Upgrade 2.x branch. I will say that when we tried to use Grammar Parser 2.x with Coder Upgrade 1.x, there was minor updating needed for Grammar Parser to even work and once those changes were applied, Coder Upgrade threw several warnings/errors due to its incompatiblity with Grammar Parser 2.x.

docwilmot commented 1 year ago

Grammar Parser was used in Drupal mainly for Coder Upgrade and for the API module. None of those use Grammar Parser anymore.

Seems like API now uses https://github.com/nikic/PHP-Parser. The Drupal Module Upgrader seems to be the successor of Coder Upgrade for converting D7 to later versions, and that uses Pharborist.

The last available version of GP 2.2 hasn't had a commit in 8 years, since 2015, when PHP 7.0 came out, so if we're concerned about support for even newer PHP features perhaps even GP version 2.2 still wont be ideal.

So if we're going to do anything about this, perhaps we should be considering PHP-Parser or Pharborist or some other parser?

docwilmot commented 1 year ago

I've gone ahead and upgraded to GP 2.x code. Took way to long to figure out how to get and update function names. We've gone from accessing the function name $name = $item->name['value'] to now $name = $name_value = &$item->name->last()->data->last()->data. This is because whereas the names of functions found by getFunctionCalls() were just stored in a simple array, the names are now made into PGPExpressions themselves, with values stored in a PGPNode in that expression. As far as I can tell. I'm hopeful there is a simpler way to do this, but can't seem to find it.