-
-
Notifications
You must be signed in to change notification settings - Fork 4
Upgrade package to rector/rector 2.x #147
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
Conversation
…ctor to Rector 2.0
…WorkspaceRector to Rector 2.0
…ier to Rector 2.0
kdambekalns
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all: Thanks for tackling that massive task!
Now, I looked a quite a few of these files…
- the
configured_rule.phpfiles all have wrong indetation from the changes done to them, I gave up suggesting a change after a while 😇 - left a comment on some commented block of code – can't that go away completely?
The files I looked at seem to follow a reasonable "change pattern", so here we go with a "👍 by 🫣" under the assumption my remarks get adressed.
src/ContentRepository90/Rules/ContentDimensionCombinatorGetAllAllowedCombinationsRector.php
Outdated
Show resolved
Hide resolved
...0/Rules/ContentDimensionCombinatorGetAllAllowedCombinationsRector/config/configured_rule.php
Outdated
Show resolved
Hide resolved
...ository90/Rules/ContentRepositoryUtilityRenderValidNodeNameRector/config/configured_rule.php
Outdated
Show resolved
Hide resolved
tests/ContentRepository90/Rules/ContextGetFirstLevelNodeCacheRector/config/configured_rule.php
Outdated
Show resolved
Hide resolved
tests/ContentRepository90/Rules/ContextIsInBackendRector/config/configured_rule.php
Outdated
Show resolved
Hide resolved
tests/ContentRepository90/Rules/ContextIsInBackendRector/config/configured_rule.php
Outdated
Show resolved
Hide resolved
tests/ContentRepository90/Rules/ContextGetCurrentRenderingModeRector/config/configured_rule.php
Outdated
Show resolved
Hide resolved
tests/ContentRepository90/Rules/ContextGetCurrentRenderingModeRector/config/configured_rule.php
Outdated
Show resolved
Hide resolved
tests/ContentRepository90/Rules/ContextIsLiveRector/config/configured_rule.php
Outdated
Show resolved
Hide resolved
tests/ContentRepository90/Rules/ContextIsLiveRector/config/configured_rule.php
Outdated
Show resolved
Hide resolved
kdambekalns
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still 👍 by 👀
|
Thanks for all the work and not giving up!!!! |
|
I want to make you aware that TYPO3 had the same problem and we now have a new tool called Fractor for all other file types: https://github.com/andreaswolf/fractor/ If you want to migrate fusion, you can create your own fusion file processor as a standalone composer package and load that into fractor. Yaml is already supported. |
|
Thank you @simonschaufi for the hint. If we had known this some time earlier 🙈 |
|
I was aware of neos/rector but I wasn't aware that you also migrate fusion with it as I'm not aware of Neos development in general. I just read about it in the announcements channel in slack. If you are interested and want to have a small presentation of Fractor, we can have a video call. |
|
We even didn't consider to check if there is already a solution. We only discussed to implement a "similar" solution to Fractor on our own. But we rejected this very quickly. Thank you for the offer, I already had a look. Looks promising. I'll bring this discussion into our next developer meeting and will come back to you. |
|
The way how it works is almost identical like Rector and I also try to make the config and cli params as identical as possible to rector. |
Compatibility with PHPRector 2.x
This PR makes the
neos/rectorpackage compatible withrector/rectorversion 2.x.For more context, please also see: neos/neos-development-collection#5607
PHP compatibility
The migrations are now compatible with PHP 8.x incl. PHP 8.4. The test suite of this package requires PHP 8.2 at minimum due to PHPUnit in v12.
PHP-only migrations
Due to the fact, rector/rector removed the possibility to add custom file handlers, to focus on PHP refactoring, we had to remove the Fusion and YAML migrations from this package. They have been moved into Neos core migrations.
See:
neos/rectorfor Fusion) neos-development-collection#5638Version20251005080230which adjusts EEL in Fusion code to the new Neos 9 API neos-development-collection#5642Improved setup
Using the outdated version of
rector/rectorresulted in conflicting dependency constraints with Neos 9 dependencies. This meant thatneos/rectorhad to be installed separately alongside the Neos project. This issue is now resolved, andneos/rectorcan simply be installed as a dev dependency in any Neos 9 project.# inside your Distribution folder composer require --dev neos/rector:dev-main cp Packages/Libraries/neos/rector/rector.template.php rector.phpThings to know
Why so many changes:
Most changes became necessary because a convenience method to add nodes before a given node
(
nodesToAddCollector->addNodesBeforeNode()) has been removed. Sinceneos/rectorheavily relied on this,most rules had to be rewritten. To achieve the same behavior as before, the rules now apply on a higher level
(mostly statements), making it possible to insert additional statements beforehand.
Test-driven refactoring:
I tried to modify the test cases as little as possible while refactoring the rules. In some cases, I had to
add or remove empty lines around comments, as the handling of comments has changed.