Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
The new major release process relies heavily on automated tools to help people upgrade their code for the next major version, and several of these tools depend on drupal-rector.
Rather than deferring the step of creating rector rules for deprecations until the major branch is actually open, it might be better to integrate their creation into the core development process.
Proposed resolution
Make the creation of a rector rule either a blocker for, or a followup to, each core issue.
Remaining tasks
- Decide whether the rules should be in core, with
drupal-rectoras a core development dependency. (So far most everyone is in favor of doing this in the long term.) - Decide whether the rector rules should be commit blockers. (We discussed this point among the committer team and perspectives were split between requiring them to avoid having a huge backlog, versus easing people into the requirement gradually, versus not having it as a requirement since they are temporary and many deprecations are provided on a best-effort basis for internal code.)
- Add a handbook page on d.o on how to create a rector rule for core, with examples.
- Decide whether tests should also be provided for the rector rules we require
Release notes snippet
TBD
Comments
Comment #2
xjmComment #3
mglamanThe only concern/problem I would have with the Rector rules going into Drupal core, is that Drupal doesn't define much in its Composer autoload definition. We'd need to decide on the namespace.
If it's
Drupal\Core\Rectorwe're fine.Maybe it's be
\Drupal\Component\Recotrand still fine.Comment #6
catchI think having the rules in core is a good idea.
We'd need to namespace them (or otherwise) by version so that we can easily remove ones from previous releases.
At least initially, but maybe permanently, I think they should be follow-ups rather than block commits. Sometimes introducing a deprecation is the point of an issue (like refactoring a procedural function to a class), but often it's not - critical bug fixes and all kinds of other issues have to introduce deprecations too, and getting those issues fixed should take precedence over automating things for contrib developers.
We also currently do a lot of best-effort deprecations like constructor parameters which we don't actually offer bc for, just to avoid breaking contrib in minor releases, and don't require deprecation testing for those, so I think those also shouldn't be rector rules at all.
Also in general wondering if there's a way to prioritise writing rules by contrib usage - sometimes we deprecate stuff that's used by literally one contrib module and it'd be quicker to directly write the patch than write the rector rule to generate an automated patch for it.
Do think it would be a good idea to start this while it's still possible to individually open issues for 10.1.x deprecations and keep up with it from there, even if it's just opening the core issues.
Comment #7
bbralaI think it's a good idea to have the issues and create a policy that requires the creation of a related issues regarding a fix thorugh rector. I agree though that this would not be needed for every single deprecation. Although that might be something to be discussing in that issue to keep the process simple.
With the introduction of gitlab we should be able to find out if there are many uses of deprecated api's in some cases, but not in all since the pattern is sometimes way to complicated to search in gitlabs search.
In relation to this, @mglaman mentioned that drupal-rector is relatively unstable because it is not part of the testing suite of rector whereas other frameworks are. This might be something to investigate, although kinda out of scope of this issue.
There has also been some discussion around this in slack. Pasting for proteriety sake. (sorry for the wall of text)
The "prior thread" mentioned in above conversation
Comment #8
mglamancatch convinced me that this could happen. There be some dragons around having Rector as a development dependency due to the way Rector is bundled. Especially if we hit a breaking change and become incompatible. I'm copying what I sent in Slack to describe the build setup when developing a Rector plugin/rules:
Oh yeah, I forgot that you also need to allow it to be patched. This is all in drupal-rector.
One way we can solve this is by having the Rector code off to the side, with its own
composer.jsonlike PHPStan's build setup: https://github.com/phpstan/phpstan-src/tree/1.8.x/buildWhile looking at the rector-phpunit source, it looks like the namespace might be able to be dynamically registered: https://github.com/rectorphp/rector-phpunit/blob/main/config/config.php. It calls
loadon the container configurator. This is from Symfony DI and allows registering PSR-4 namespaces!Or maybe this is how Rector works around the merged code in the compiled Rector package and it doesn't matter to us.
We could have a
rectordirectory alongsidecorethat becomes thedrupal/core-rector-rulespackage or something. This directory contains acomposer.jsonandrulesandrules-testsdirectory. The nice part about this structure is leveraging the Rector generator (it uses arulesandrules-testsstructure.The hard part is ensuring we can run PHPUnit against the rules to verify they work. At first, I thought this meant we needed Drupal code to be autoload-able, but we don't. In the drupal-rector tests we don't recreate Drupal's autoloading; it all just works because the symbols match.