Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Add MigrateBackendModuleRegistrationRector #4243

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

simonschaufi
Copy link
Collaborator

Resolves: #3347

@simonschaufi
Copy link
Collaborator Author

simonschaufi commented Jul 6, 2024

@samsonasik Dear Abdul, this is probably the most difficult Rector rule ever you have seen so far because I not only need to change code within a file but across different files. Creating files and writing to it is no problem for me as we have several other rules where we do this already. The big challenge here is to parse an existing php array and add a new item into it. You can see the purpose in the linked issue. TYPO3 decided to move configuration into a new file with a different syntax. I'm not sure if other frameworks have similar challenges?

My initial thought was that i just create the new php array in pure php and then let the php parser do its thing and then I just merge the 2 nodes together but this isn't working because of some internal things I don't understand. For some reason the old nodes get duplicated even though the nodes are properly merged when you debug the node tree. My guess is that is has to do with the attributes that get attached to each node but I'm not sure.

My second try was to create the ArrayItem by hand but then I have the issue that the written output is just a one liner (see the 2 versions below the comment Merge the arrays).

Maybe you have some deeper understanding on what is going on internally in the php parser and have an idea that could solve my issue here?

@simonschaufi
Copy link
Collaborator Author

@samsonasik thank you very much for looking into it bit even with this change the problem still exists that the output is currently still the duplicate of tools_toolsmaintenance instead of the added tools_toolssettings

@samsonasik
Copy link
Contributor

the array structure may be appended, the structure may be need to be unset and append by index, like using array_splice()...

@simonschaufi
Copy link
Collaborator Author

I'm merging the two arrays here: https://github.com/sabbelasichon/typo3-rector/pull/4243/files#diff-6659bbad2dc4a8d574769de61529b6e6d9387889d39813bbe2f246c873ba3bfeR461

When you inspect the code afterwards, it looks correct but the generated code is not correct and that is strange. Is there somebody in the Rector team who has more understanding on what is going on under the hood?

@simonschaufi
Copy link
Collaborator Author

simonschaufi commented Aug 30, 2024

ping @samsonasik @TomasVotruba Please see also the referenced issue in nikic/PHP-Parser before ⬆️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: #96733 - New backend module registration API
3 participants