Repository navigation
feat: add composer-based set binding every rule to an exact drupal/core version - #419
Open
TomasVotruba wants to merge 35 commits into
Open
TomasVotruba wants to merge 35 commits into
TomasVotruba wants to merge 35 commits into
Conversation
…re version
Replaces `DrupalSetProvider` — Rector deprecated `SetProviderInterface` and
`ComposerTriggeredSet` in favour of binding rules to a package version
directly — with a single generated set, `config/composer-based.php`, exposed
as `DrupalSetList::COMPOSER_BASED`.
Every configurable rule is registered through Rector 2.6's
`RectorConfig::ruleWithConfigurationComposerVersionBound()` with the exact
version its deprecation was introduced in, taken from the value object it is
already configured with:
-$rectorConfig->ruleWithConfiguration(FileSystemBasenameToNativeRector::class, [
- new DrupalIntroducedVersionConfiguration('11.3.0'),
-]);
+$rectorConfig->ruleWithConfigurationComposerVersionBound(FileSystemBasenameToNativeRector::class, [
+ new DrupalIntroducedVersionConfiguration('11.3.0'),
+], 'drupal/core', '>=11.3.0');
Rules that take no configuration cannot use that API, so they are registered
behind a `$ruleSince()` helper that checks the same constraint against the
installed `drupal/core`:
-$rectorConfig->rule(LoadAllIncludesRector::class);
+$ruleSince(LoadAllIncludesRector::class, '>=11.3.0');
Rector then activates only the rules the installed core satisfies — 34 of the
124 bound registrations on core 10.3, 63 on 11.2, 124 on 12.0 — and
`vendor/bin/rector composer-based` lists them with their constraint. Because
the installed version is known exactly, the opt-in "breaking" renames are
included; they cannot fatal on a core guaranteed to have the replacement.
The per-minor `Drupal*SetList` constants are unchanged and remain the way to
look ahead at a Drupal version that is not installed yet.
The set is generated by `scripts/generate-composer-based.php` from the
per-minor configs, so a rule only has to be registered once;
`ComposerBasedSetTest` asserts the committed file is in sync, that every
registration carries an exact `>=X.Y.Z` constraint, and that no rule of the
per-minor sets is missing from it.
…rupal: true) Restores `DrupalSetProvider` so existing `withSetProviders(DrupalSetProvider::class)` configs keep working; its docblock now points at `DrupalSetList::COMPOSER_BASED` as the successor. Drops the explicit `withSets([DrupalSetList::COMPOSER_BASED])` from the README and from the generated set's docblock — `withComposerBased(drupal: true)` is the documented entry point.
Adds the `@deprecated` annotation and a CHANGELOG "Deprecated" entry for `DrupalSetProvider` and the `drupal` / `drupal (breaking)` set groups it provides. The class stays registered so existing configs keep working, and is removed in a later release.
…hConfigurationComposerVersionBound()
TomasVotruba
marked this pull request as draft
August 10, 2026 11:33
Contributor
Author
|
Still WIP... need a bit config tuning. But check the linked PRs in the description to get the idea ahead 👍 |
The generator rewrote the per-minor configs by walking their PHP tokens and patching the registration calls, which made the set a build artifact of a parser rather than a config you can read. Keeps the set as a plain config of real rules, in the shape rector-doctrine uses (rectorphp/rector-doctrine#497), and accepts that a rule is registered both here and in its per-minor config. `ComposerBasedSetTest` fails when a rule of a per-minor config is missing here, so the duplication cannot drift unnoticed.
…e shape Follows rectorphp/rector-doctrine#497: a rule that takes no configuration declares the version its target API landed in on the rule class, instead of being gated by the set that registers it. -$ruleSince(LoadAllIncludesRector::class, '>=11.3.0'); +$rectorConfig->rule(LoadAllIncludesRector::class); +final class LoadAllIncludesRector extends AbstractRector implements ComposerPackageConstraintInterface, DocumentedRuleInterface +{ + public function provideComposerPackageConstraint(): ComposerPackageConstraint + { + return new ComposerPackageConstraint('drupal/core', '>=11.3.0'); + } 76 rules are bonded this way, which drops the `$ruleSince()` helper and the `Semver` lookup from the set. The three rector-phpunit rules the Drupal 10.1 config registers are dropped from the set as well: they are already bonded to `phpunit/phpunit` by rector-phpunit's own composer-based set, which is the accurate constraint for them. Rector applies the constraint filter globally rather than per set, so these rules no longer run against a core older than the deprecation, including when a `Drupal*SetList` set is loaded by hand. Verified against a project on 11.2.3 and one on 11.3.0: `LoadAllIncludesRector` fires only on the latter. Rule tests point `provideComposerJsonFilePath()` at a stub composer.json that requires drupal/core, as this package does not — without it every bonded rule would be filtered out of its own test.
- `ComposerBasedSetTest` used `new ReflectionClass(…)->…`, which only parses on PHP 8.4; the phpunit and phpstan workflows run 8.3. - Both workflows installed `rector/rector:^2` before running, which can resolve below the 2.6 that ships `ruleWithConfigurationComposerVersionBound()` and `ComposerPackageConstraintInterface`. Bumped the matrix to `^2.6`, matching the constraint in composer.json. - `ShouldCallParentMethodsRector` and `AddSymfonyConstraintValidatorTypeDeclarationsRector` were each registered by two per-minor configs. A bonded rule carries one constraint, so each is bonded to the lower of its two versions and registered once, under that version. Verified on PHP 8.3 with `composer require rector/rector:^2.6 --dev` applied the way the workflows do it: php-cs-fixer 0/523, PHPStan no errors, PHPUnit 699 tests.
The checks are static properties of the config and the rule classes, so PHPStan is where they belong — they now report at the offending line rather than as a regex assertion over the file contents, and cover every rule class rather than only what the set file happens to reference. - BoundRuleConfigurationRule — a ruleWithConfigurationComposerVersionBound() call must name "drupal/core" and an exact ">=X.Y.Z" version. - ComposerPackageConstraintRule — the same for every ComposerPackageConstraint a rule class constructs. - PlainlyRegisteredRuleRule — a rule registered with a plain rule() call in the composer-based set must implement ComposerPackageConstraintInterface, otherwise it runs on every Drupal version. - ComposerBasedSetCoverageRule — with RegisteredRectorClassCollector, fails when a per-minor config registers a rule the composer-based set does not. Each rule is covered by a RuleTestCase, and all four were verified against a deliberately broken tree: a ">=8.0" constraint, a rule class stripped of the interface, a "drupal/coder" package name, and a registration deleted from the set each produce the expected error.
TomasVotruba
marked this pull request as ready for review
August 10, 2026 12:48
TomasVotruba
marked this pull request as draft
August 10, 2026 12:48
TomasVotruba
marked this pull request as ready for review
August 10, 2026 12:53
Contributor
Author
|
Ready now 👍 |
Contributor
Author
|
Ping @bbrala |
Contributor
|
We should bump the required rector version and make So that Or all sets needs to check that the constant like |
This was referenced Sep 3, 2026
bbrala
added a commit
that referenced
this pull request
Sep 3, 2026
* fix: conflict with rector/rector >=2.6.2 (#420) Rector 2.6.2 removed every version-specific set constant from its first-party extension packages in favour of the new composer-based sets (rectorphp/rector-phpunit#760). The Drupal 8, 9 and 10 configs reference those constants directly, so from 2.6.2 on they abort before any rule runs. Bisected against real installs; 2.6.1 is the last release on which every Drupal set loads: 2.5.9 / 2.6.0 / 2.6.1 ok 2.6.2 / 2.6.3 / 2.6.6 fail Breakage map on 2.6.6: DRUPAL_90, DRUPAL_100 Could not detect twig set. DRUPAL_91, DRUPAL_92 Undefined constant PHPUnitSetList::PHPUNIT_90 DRUPAL_101 Undefined constant SymfonySetList::SYMFONY_63 DRUPAL_102 Undefined constant SymfonySetList::SYMFONY_64 plus the DRUPAL_8 / DRUPAL_9 / DRUPAL_10 aggregates that include them. The Drupal 11 and 12 sets reference no third-party sets and are unaffected. A conflict rather than a narrowed require constraint, so the supported range stays documented as ^2 and composer reports the incompatibility by name. This is a stopgap to keep installs working. The real fix is porting the sets to the composer-based mechanism (#419), which Rector 2.6.6 already expects: withComposerBased(drupal: true) resolves DrupalRector\Set\DrupalSetList:: COMPOSER_BASED, and SetGroup::DRUPAL is now marked deprecated upstream in favour of a composer-based.php set. The conflict is lifted once that lands. Reported by ptmkenny in #420 * style: use ?int over null|int in RemoveToolkitArgFromImageToolkitOperationConstructor Unrelated to #420, but the codestyle job is failing on main's code and blocks this branch. The @symfony ruleset in php-cs-fixer 3.95.24 enforces nullable_type_declaration with the question_mark style. The dev constraint is ^3.95.1, so the rule arrived by a floating minor rather than by any change here — the file has been unchanged since it was merged green. Only the php-cs-fixer job would have caught it, and the scheduled runs cover phpstan, phpunit and functional tests only, so it went unnoticed. No behavior change; ?int and null|int are the same type.
Collaborator
|
Ok, back from vacation. I did a ninja release for this package to lock it out of >2.6.1 so it doesnt break while this is worked on. There is another path in this, and that is removing the specific rules for twig and phpunit maybe. Also kinda interested in how this works. A rule tells us: ">=10.1.0", does that mean it is selected on anything above, or anything above but belore next major? So does I wouldn't mind to much to drop the set references, but am a little worried to throw things around again with how the sets work. |
Contributor
Author
|
Welcome back :) many news in Rector. Just the >=. Checkout my PRs in Laravel Rector package, where I did the same upgrade. |
The Drupal 8, 9 and 10 sets pulled in SymfonySetList::SYMFONY_40-64, PHPUnitSetList::PHPUNIT_60-90 and TwigSetList::TWIG_24, so a Drupal set silently rewrote Symfony, PHPUnit and Twig code alongside Drupal code. Rector 2.6.2 deleted those per-version set files and the constants naming them, which is what the `rector/rector >=2.6.2` conflict was holding back: on 2.6 the sets abort with "Undefined constant SymfonySetList::SYMFONY_64". Upstream replaced them with one composer-based set per package, so there is no per-version constant left to remap to — the references are dropped and the Drupal sets now carry Drupal rules only. Users who want the upstream rules add ->withComposerBased(phpunit: true, symfony: true, twig: true) themselves; that keys off the installed PHPUnit/Symfony/Twig version rather than the Drupal one, so it is not a like-for-like swap. Dropped with them: - the AddDoesNotPerformAssertionToNonAssertingTestRector skip in the Drupal 8 set, which only existed to suppress a rule of the PHPUnit 6 set - the TwigSetList TWIG_24/TWIG_240 detection dance in the 9.0 and 10.0 sets - 15 now-unmatched SymfonySetList entries in the PHPStan baseline The three PHPUnit 10 rules the Drupal 10.1 set registers by hand are unaffected: those classes still exist in rector-phpunit. Verified on rector 2.6.7 against a scratch project: the six per-minor configs load cleanly where the previous revision aborted on the undefined constant. Refs palantirnet#420
Collaborator
|
I think i will do a min/max for the contraints. This does mean some archeoligy to when things actually god removed, but i think that should work. I really really don't want all rules to run in every major, but probably only the rules of the current major (very very mayby also the previous). This seems like a way to keep stability in check and not just create a drag of old rules that need to work on newer versions. |
Pass 1 of the question "can a rule carry a bounded composer constraint (>=8.5.0 <9.0.0) instead of the open-ended >=8.5.0 PR palantirnet#419 gives it". Change records first, as the primary source: rule -> CR (scraped from the rule class and from the config line registering it, since the generic configurable rules carry the CR beside each configuration entry) -> change_record.change_to_branch for the declared target -> change_record_symbol role='from' -> core_symbol.removal_in for the version the old API actually went away in. Of 154 rules, 121 cite an indexed change record, 76 have a removal version and 39 of those are code-observed rather than merely scheduled. D8 rules resolve to removal in 9.0.0, D9 to 10.0.0, D10 to 11.0.0, D11 to 12.0.0 or 13.0.0 -- all observed at or below 11.0.0, almost all scheduled above it. The records are not reliable on their own, which pass 2 has to check against the tree. Six are wrong or unreadable: four give a removal version equal to the deprecation version, 3349345 removes inside a major (10.2 -> 10.5.0), 3461934 gives one minor of grace (11.0 -> 11.1.0), and 3513877 has a typo in its own branch field. So an upper bound must come from removal_in, never from arithmetic on the deprecation version. For the D8/D9 bounding case specifically: 15 of 43 rules can be bounded from this evidence today, all on observed removals; 27 have no removal version (26 cite no CR at all, mostly the 9.1 AssertLegacyTrait block); and FunctionToFirstArgMethodRector spans three minors so it can never take one class-level bound.
Change records first, then the code, because the records are not reliable on their own. Pass 2 went at the symbols directly: one row per configuration entry rather than per rule (234 entries, since a configurable rule carries one deprecation per value object), reading the deprecated symbol out of the value object's constructor argument instead of out of prose, then resolving each against core_symbol.removal_in / removal_kind, and falling back to repos/drupal-core for what the catalog does not carry. 228 of the 234 entries resolve to a core symbol. The 6 that do not are not core symbols: three name PHPUnit's getMock(), three name Symfony CMF RouteObjectInterface constants (core decoupled from CMF in 9.1.0, 37893880799). Coverage went from 76 rules with a removal version and 39 code-observed, to 100 and 48. Drupal 8 is 16/18 and Drupal 9 23/25, almost entirely observed. For the bounding question: 38 of 43 D8/D9 rules can now be bounded, every one on an observed removal, up from 15 after pass 1. The 9.1 AssertLegacyTrait block -- 12 rules citing no change record at all -- resolved in one lookup, all removed in 10.0.0. The headline correction no change record would have given us: RequestTimeConstRector sits in the 8.3 set, but REQUEST_TIME was not removed until 11.0.0. A bound computed as "deprecation major + 1" would have given it <9.0.0 and silently disabled the rule for every D9 and D10 user. With 3349345 (10.2 -> 10.5.0) and 3461934 (11.0 -> 11.1.0), that is three proofs that an upper bound must come from removal_in, never from arithmetic. Aggregating entries per rule also settles where a bound belongs: FunctionToServiceRector's 97 entries span removals from 9.0.0 to 13.0.0, as do FunctionToStaticRector's and ConstantToClassConstantRector's. No class-level constraint can be right for them; their bounds belong per configuration entry. Stopping for Drupal 11: 42 of its rules still have no removal version, and of the 51 that do only 8 are observed -- the rest are scheduled for 12.0.0 or 13.0.0, and the evidence above says not to bound on a promise.
One report covering the whole review: what the PR does, what was resolved and fixed, what stays open, and the evidence behind each so it does not have to be gathered again. Resolved: the bundled Symfony/PHPUnit/Twig sets had to go and did (e05e5f9, pushed); the global ComposerPackageConstraintFilter turned out not to break the look-ahead workflow, because a rule targets the version its deprecation lands in -- one or two majors before removal -- and contrib is analysed inside a site, so the library-floor path never fires. Open: whether D8/D9 rules stay, with the coverage, maintenance and ecosystem numbers behind it, plus the accidental regression that composer-based.php re-enables all 43 of them; and bounded constraints, with the verified mechanism, the three rules any implementation must follow, and the escape-hatch catch. No code changes.
Core states each deprecation in a machine-readable form that cites the change record that documents it: ModuleHandler::loadAllIncludes() is deprecated in drupal:11.3.0 and is removed from drupal:13.0.0. ... See https://www.drupal.org/node/3536432 There are 1,651 such strings in core, 912 carrying a node link, covering 217 distinct change records. Every rule already has its change record nids, so rule -> nid -> core's promise is a join rather than research. It resolved 17 of the 54 rules that still had no upper bound, each with a file:line citation, at no cost. Coverage: 100 -> 117 of 154 rules with a removal version. The new ones land as scheduled rather than observed -- Drupal 12 does not exist yet, so for an 11.x deprecation there is nothing to observe, only core's stated intention.
…t core Seven Haiku agents took the 27 rules the change-record join could not reach, four rules each, each told to read the rule and its test fixtures, find the deprecated symbol in core, and return the notice copied verbatim with a file and line. A script then re-read every citation and string-matched it. 18 of 21 bounded claims verified. 3 were fabricated: a cited file that does not exist, an evidence_text absent from the line given, and a version not present in the quoted text. Requiring a verbatim citation is what made those visible -- the conclusions of two of the three were actually right, so trusting the answers rather than the evidence would have looked fine and been unreproducible. The three rejects and two not_found results were resolved by hand from core's git history: ReplaceRequestTimeConstantRector 11.0.0 2f44d215e86 (#3442766) MigrateSqlGetMigrationPluginManagerRector 11.0.0 166f3a39e46 (#3439369) RemoveTwigNodeTransTagArgumentRector 11.1.0 cec21638d09 (#3477374) RenameStopProceduralHookScanRector 11.2.0 308ad151024 (#3495943) ReplaceLocaleTranslationPathConfigRector 13.0.0 locale.schema.yml:45 Two lessons for a repeat run. The working tree only shows what is still there, so a pre-11.0 removal needs git log -S plus git tag --contains, not grep -- all three not_found results had that one cause. And where a rule spans several notices with different removals, take the latest; bounding on the earliest silently disables the rule while the deprecation is still live. Coverage: 117 -> 139 of 154 rules with a removal version, 52 code-observed. The remaining 15 are documented as rules that should not get a drupal/core upper bound at all -- 5 are PHPUnit-bound (4 of which PR palantirnet#419 gives a drupal/core constraint, a defect worth its own report), 3 are infrastructure, 5 are behaviour changes with nothing removed, 1 has no stated removal, and 1 needs a human look.
You were right that "nothing there" was too quick. Checking pass 4's leftovers against the symbol catalog, core's git history and the change records on drupal.org turned two into real bounds and corrected the package on a third. FromUriRector was a misidentified symbol, not a missing one. Pass 2 looked up Url::fromUri, found it undeprecated, and filed the rule as a behaviour change. The rule actually matches Url::fromUri(file_create_url($uri)), so its deprecated symbol is file_create_url() -- removed 10.0.0, observed. It has no fixtures, so fixture-based identification could not reach it either. FunctionalTestDefaultThemePropertyRector does have a removal: deprecated 8.8.0 (CR 3083055), BC layer removed in 9.0.0 (305c401f90e, #3110874), and core now throws an exception citing that very change record. AddSymfonyConstraintValidatorTypeDeclarationsRector tracks Symfony 8.0's ConstraintValidatorInterface, per its own @see -- a sixth wrong-package rule, five of which PR palantirnet#419 gives a drupal/core constraint. The other 13 are each chased to a citation rather than left open: - \Drupal::classResolver is added_in 8.3.x with status [] -- not deprecated at all, which settles ViewsConfigUpdaterClassResolverToServiceRector. - $modules became protected static in 8.3.0 (b33af7a964e, #2814035). - state_cache: core cites CR 3177901, not the two the rule cites; "In Drupal 11+, settings 'state_cache' is removed and permanently turned on" is the rule's lower bound, and Settings.php still flags it with no deadline. - ShouldCallParentMethodsRector is not a deprecation rule at all -- it is test hygiene registered in the 9.0 and 10.0 deprecation sets. - getMock removed in PHPUnit 6; getName renamed to name in PHPUnit 10; annotations removed in PHPUnit 12. Also: Classy was removed in 10.1.0, not 10.0.0 -- one more trap for anyone computing an upper bound by arithmetic. Coverage: 139 -> 141 of 154, 54 code-observed. Also replaced the stale "what is left" section, which still quoted pass-2 numbers.
An invariant check — Drupal removes deprecated API only at a major boundary,
so removal must be X.0.0 in a strictly later major than the deprecation —
flagged 11 of 134 matrix rows as impossible.
Seven had captured the deprecation version in the removal column, three of
them a version earlier than the set the rule ships in. All seven came through
the change-record path, none through git archaeology. Re-read from core and
corrected, with file:line citations.
The remaining four are genuine: two are real 11.0.0 removals whose sets are
keyed on removal rather than deprecation, and two are not deprecation removals
at all (an upstream Twig constructor change, and the rename of an attribute
introduced one minor earlier during the OOP-hooks work).
So no rule bounds on a mid-major removal, and the upper bound is always a
clean <{major+1}.0.0 with no special case. Documents the check for CI.
Adds "Deprecated in" beside "Removed in", so each row carries both ends of the bound. Derived, never hand-typed, from four sources in descending trust: notices read from core by hand, the verified pass-4 output, the core-notice evidence in bounds.json, and the change-record join taking the earliest deprecation across a rule's change records. 93 of 154 rows filled. The gap is structural, not random: D11 is 85/98 while D8 is 4/22 and D9 2/27, because those notices were deleted from core years ago and no route that reads an 11.x checkout can see them. Comparing the derived version against the set each rule ships in found 6 disagreements out of 93. Five are coverage gaps in the same direction -- the rule is withheld from sites that already have the deprecation, including RemoveInstallSchemaSystemSequencesRector (set 11.4, deprecated 10.2.0) and ReplaceDialogClassOptionRector (set 11.3, deprecated 10.3.0). One fires early. Also shifts the documented invariant check to the new column position and corrects a bad note from the previous commit: the matrix holds 154 rows and matches the coverage table; the earlier 153 came from a stale line range.
The composer-based set currently accepts only an open-ended `>=X.Y.Z`, so a rule stays active forever once the deprecation lands. This allows the closed form `>=11.3.0 <13.0.0` as well, which is what lets a rule switch itself off once it can no longer apply. The upper bound must be `<N.0.0`: Drupal removes deprecated API only on a major boundary, so any other shape is a mistake rather than a preference. The rule also compares the two majors and rejects an upper bound that is not later than the lower one -- an invariant that would have caught seven removal versions in the version matrix that had silently captured the deprecation version instead. Both checks live in one shared validator, so a constraint declared on a rule class and one declared in the set are held to the same shape. The open-ended form is still accepted while the set is being bounded; it can be rejected once every constraint carries a span.
The six generic rules -- FunctionToService, FunctionToStatic, MethodToMethodWithCheck, ConstantToClassConstant, ClassConstantToClassConstant and FunctionCallRemoval -- are each registered many times, once per deprecation version, and together hold 168 configuration entries. Every group stated only a lower bound, so a Drupal 8 entry still ran against Drupal 11. Each group now closes at the major after the latest removal among its own entries. Where a group's entries span two removals, the later one wins: the rule must stay alive while any of its targets is still live. How the 168 entries were resolved: - 119 by reading the deprecation notice directly above the declaration in core, scoped to the declaring file, with global functions distinguished from same-named methods by declaration column; - 49 by git archaeology, since those symbols are gone from an 11.x tree. Every archaeology result was re-verified mechanically rather than trusted: each cited commit was confirmed to contain a deletion line for the symbol, and `git tag --contains` was confirmed to make the claimed version the first release containing that commit. 30 of 30 git claims and 15 of 15 tree claims verified, with no fabrications. Two groups keep an open lower bound. One holds the Symfony CMF RouteObjectInterface constants, which were never core symbols -- core added its own interface in 9.1.0 and the rewrite target still exists, so there is nothing to bound against. The other is the remaining residue. Also widens the package check from a hard-coded `drupal/core` to a short allowlist, and accepts an upper-bound-only constraint for a package whose API predates the rule. A `drupal/core` bound must still state a lower bound. This unblocks the five PHPUnit rules and one Symfony rule that currently carry a `drupal/core` constraint that is not true of them.
Closes the upper bound on 63 more registrations, taking the set from 44 to 107 bounded of 124. Each inherits its rule's removal version from the version matrix and retires one major later. The 17 that stay open are the rules with no removal version to bind to, plus the four whose constraint names drupal/core but whose deprecation belongs to PHPUnit or Symfony; those need repackaging first. Class-level constraints in src/ are deliberately NOT bounded here -- see the next commit message for why they cannot be, yet.
Widening the PHPStan package check to allow phpunit/phpunit and symfony/validator let six rules name the package they actually track, but it contradicts the rest of this PR. The bundled PHPUnit, Symfony and Twig sets were removed on the grounds that those deprecations are not ours to track, and the README now sends users to ->withComposerBased(phpunit: true, ...). An allowlist re-adopts that responsibility one layer down and commits drupal-rector to following PHPUnit's release cycle. It would also entrench a duplicate: rector-phpunit already ships its own GetMockRector doing the same rewrite, in its own composer-based set, so a user following our README gets it twice. The upper-bound-only form goes with it, since it existed only to express "phpunit/phpunit <6.0.0". A constraint is once again required to state the drupal/core version the deprecation was introduced in. Kept: the closed-range form, the major-ordering check, and the shared validator behind both PHPStan rules. The six wrong-package rules keep the bound palantirnet#419 gave them -- untrue of them, now documented in the matrix along with why the allowlist is the wrong repair.
Rule tests already stand in a fake drupal/core version, because this package does not require drupal/core and every rule bonded through ComposerPackageConstraintInterface would otherwise be filtered out. That stub pinned one global version, 12.0.0, chosen as "the highest any rule is bonded to, so every rule is active". That holds only while every constraint is open-ended. Once a rule states a closed range no single version can activate them all: a rule for an API removed in Drupal 10 caps at <11.0.0, while one deprecated in 11.4 only starts at >=11.4.0. The two are mutually exclusive, so a global pin silently switches off whichever half it does not match -- the rule stops transforming and its own fixture test fails. Each tests/src/Drupal<major> namespace now gets its own pin, high within that major, so every rule in the set is active. Tests outside those namespaces keep the existing fallback. No change to the 143 test classes or the 136 rule configs; the test case derives the file from its own class name. This commit is inert on its own -- verified at 700 tests with no failures before any rule was bounded. One thing it gives up: a rule test now asserts the transformation but no longer exercises activation, since the pin is chosen to satisfy the rule. Activation deserves its own test rather than being implicitly covered by every fixture.
Every rule that declares a ComposerPackageConstraint stated only the version its deprecation was introduced in, so it stayed active forever: a Drupal 8 rule still ran against a Drupal 11 codebase. Each now retires one major after the API was removed. Removal versions come from the version matrix, which reads them from core's own deprecation notices or, where the symbol is already gone from an 11.x tree, from the commit that deleted it plus the first release tag containing that commit. The bound is the major *after* removal, never the removal itself. Rector reads source, not runtime: a codebase can still contain a removed call in dead code or an unported module, and those are exactly the calls worth rewriting. Bounding at the removal version would switch the rule off at the moment the code became broken. Five constraints stay open. Three have no removal version in core to bind to (ProtectedStaticModulesPropertyRector, ShouldCallParentMethodsRector, RemoveStateCacheSettingRector). The other two are part of the six rules whose constraint names drupal/core but whose deprecation belongs to PHPUnit or Symfony; they need the right package before a bound means anything. Depends on the preceding commit: without the per-major test pin these bounds fail 35 of the project's own rule tests, because the shared stub reported a core newer than a D8/D9 rule's upper bound.
RenameClassRector and RenameStaticMethodRector are Rector's own classes, but the registrations, the deprecations and the change records are all Drupal's, so they bound like any other rule. Six of the seven now close: L505 Bytes::toInt >=9.1.0 <11.0.0 removed 10.0.0 L792 MatchingRouteNotFoundException >=11.1.0 <13.0.0 removed 12.0.0 L875 AliasWhitelist >=11.1.0 <13.0.0 removed 12.0.0 L1162 pgsql QueryFactory, migrate >=11.2.0 <13.0.0 removed 12.0.0 L1460 WorkspaceAssociation >=11.3.0 <13.0.0 removed 12.0.0 L2121 NodeViewController >=11.4.0 <14.0.0 removed 13.0.0 Four of those removal versions were stated in the surrounding code comments rather than read from core. All four were re-checked against core's own notices; three held, one did not. The one that did not is the block_content Access group, and the comment was wrong three ways. Drupal\block_content\Access\* was @internal and was MOVED to Drupal\Core\Access\* in 2d8018be0a8, first released in 11.2.0 -- not deprecated in 11.3.0 and removed in 12.0.0. The commit adds no class_alias and no @deprecated marker, so no deprecation cycle ever existed, which is how it landed mid-major without breaching policy. That makes it a genuine open-ended case, but for the opposite reason to the one recorded: code on the old path breaks from 11.2.0 and stays broken, so the rewrite keeps its value indefinitely. Its floor was also a minor too late, withholding the rule from 11.2 sites that already need it, so it moves >=11.3.0 -> >=11.2.0. The comment and its stale TODO are corrected. Set registrations now 113 of 124 bounded.
…noyances. Officle Drupal 9 support ended 3 years ago
…ser-based docs The composer-based.php docblock and the README still described each rule as bound to the exact version its deprecation was introduced in, and the docblock still referred to Symfony/PHPUnit sets the per-minor configs no longer pull in. - describe the bound as a range: introduced version up to the major after the removal - say that class-bound rules are also dropped past their upper bound and on a dev-main checkout, while configurable rules are bound only in this set - warn that withComposerBased(symfony: true) applies Symfony 7 rules on a Drupal 11 site, which breaks code that still supports Drupal 10
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey, I'm improving the way Rector find composer-based rules and sets. All you need is:
ComposerPackageConstraintInterfaceruleWithConfigurationComposerVersionBound- e.g. for generic rules like rename class:The set provider, SetList, versioned set list can be dropped (in one of next PRs, not now).
It's agent generated, but it works well on Doctrine/Symfony/PHPUnit sets, see e.g.
I'll open this for feedback if it's all done correctly :) do not merge yet.
I'll make sure it's part of next Rector release and everything works as smoothly as before.
Feedback welcomed 👍
I've also added custom PHPStan rule to make sure all the rules are registered in
composer-based.phpconfig and nothing is missed by accident 😉Replaces
DrupalSetProvider— Rector deprecatedSetProviderInterfaceandComposerTriggeredSetin favour of binding rules to a package version directly - with a single generated set,config/composer-based.php, exposed asDrupalSetList::COMPOSER_BASED.