search

cakephp / upgrade

Upgrade tools for CakePHP meant to facilitate migrating from one version of the framework to another
MIT License
110 stars 60 forks source link

Use Traverser instead of NodeRemover on AppUsesStaticCallToUseStatementRector #236

Closed samsonasik closed 1 year ago

samsonasik commented 1 year ago

Ref https://github.com/cakephp/upgrade/pull/235#pullrequestreview-1473429118 /cc @LordSimal

LordSimal commented 1 year ago

@markstory I leave this up 2 you how you want to handle this. I am very new to the whole AST and PHP Parser system so I have no idea why this is so much more complicated than the previous nodeRemover functionality.

samsonasik commented 1 year ago

@LordSimal I simplified the logic ;) https://github.com/cakephp/upgrade/pull/236/commits/8a7f3aad11a168c09b840779b7bd227d83016ded

samsonasik commented 1 year ago

All green 🎉 I will let you take over ;)

samsonasik commented 1 year ago

I also add fixture test to skip for different static call to ensure no regression ;)

samsonasik commented 1 year ago

✔️ All green 🎉 ✔️ Simplifed code and handled avoid too deep traversal ✔️ Ready to merge 👍

@TomasVotruba I think we need to document this or a blog post how to migrate from NodeRemover to use SimpleCallableNodeTraverser ;)

markstory commented 1 year ago

I'm no expert in PhpParser either, but the input/output looks good.