Repository navigation
[composer-based] Bond data-fixtures rule and annotation-to-attribute sets to composer package constraints - #497
Merged
Conversation
…sets to composer package constraints
TomasVotruba
force-pushed
the
composer-based-fixtures-attributes
branch
from
August 3, 2026 15:56
381035c to
d28f66a
Compare
…, drop the dev dependency
TomasVotruba
added a commit
to TomasVotruba/drupal-rector
that referenced
this pull request
Aug 10, 2026
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.
TomasVotruba
added a commit
to TomasVotruba/drupal-rector
that referenced
this pull request
Aug 10, 2026
…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.
bbrala
added a commit
to palantirnet/drupal-rector
that referenced
this pull request
Oct 30, 2026
…re version (#419) * feat: add composer-based set binding every rule to an exact drupal/core 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. * fix: keep DrupalSetProvider as BC layer, document withComposerBased(drupal: 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. * docs: deprecate DrupalSetProvider rather than dropping it silently 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. * docs: show withComposerBased(drupal: true) alone in the usage example * docs: fix the Rector requirement note, link the PR that added ruleWithConfigurationComposerVersionBound() * refactor: hand-maintain config/composer-based.php, drop the generator 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. * feat: bond the rules themselves to drupal/core, in the rector-doctrine 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. * fix: make the branch pass CI - `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. * fixup! fix: make the branch pass CI * docs: drop the Symfony/PHPUnit composer-based aside from the README * refactor: replace ComposerBasedSetTest with custom PHPStan rules 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. * chore: drop patches.lock.json, it is a composer-patches artifact * remove conflict for 2.6 * refactor: drop the bundled Symfony, PHPUnit and Twig sets 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 #420 * docs: matrix of every rule's deprecation and removal version 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 #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. * docs: pass 2 of the rule version matrix, verified against the code 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. * docs: consolidated review of PR #419 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. * docs: pass 3, upper bounds joined from core's own deprecation promises 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. * docs: pass 4, upper bounds researched by agent and re-verified against 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 #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. * docs: pass 5, chase the residue through core, the catalog and drupal.org 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 #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. * docs: correct 7 fabricated removal versions in the rule version matrix 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. * docs: add a derived deprecation column to the rule version matrix 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. * feat: let a bound rule name the major its deprecation is removed in 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. * feat: bound the 44 multi-registration groups to the major after removal 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. * feat: bound the single-registration groups in the composer-based set 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. * revert: drop the package allowlist from the bound-constraint rule 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 #419 gave them -- untrue of them, now documented in the matrix along with why the allowlist is the wrong repair. * test: pin each Drupal major's rule tests to its own core version 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. * feat: close the version range on 71 rule class constraints 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. * feat: bound the upstream rename-rule registrations 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. * chore: dropping Drupal 8 and 9 rules, that will just add noise and annoyances. Officle Drupal 9 support ended 3 years ago * chore: remove plan * docs: update skills to adjust to new composer-based-set setup. * docs: describe the range bounds and the Symfony set risk in the composer-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 * fix: return the node when AnnotationToAttributeRector adds an attribute Rector 2.8 only prints nodes a rule returns. In the keep-both path the rule appended the attribute but returned null, so the attribute was silently dropped. * fix: stop using Rector APIs removed in 2.8.0 Inline NodeFactory::createLocalMethodCall() and ParentClassScopeResolver::resolveParentClassName(), and drop the HookConvertRector fallback to AbstractRector::$file; getFile() exists on every Rector release this branch allows. * refactor: remove DrupalSetProvider Rector 2.8.0 deleted SetProviderInterface and ComposerTriggeredSet, so the provider can no longer load. Rules carry their drupal/core version themselves; DrupalSetList::COMPOSER_BASED replaces it. * refactor: drop the Bytes::toInt rename from the Drupal 9.1 set Rector 2.8 deprecated RenameStaticMethodRector and no longer runs it. Bytes::toInt() was removed in Drupal 10.0. * docs: changelog for the DrupalSetProvider and Bytes rename removals * fix: keep DrupalSetProvider as an empty deprecated placeholder Existing rector.php files call withSetProviders(DrupalSetProvider::class). Since Rector 2.6.5 that call is a no-op, but with the class gone static analysis of those files (upgrade_status scans a module's own rector.php) reports "Class not found". Keep an empty class until 2.0.0. * build: require rector/rector ^2.6.5 2.6.5 is the first release where withComposerBased(drupal: true) loads DrupalSetList::COMPOSER_BASED itself and withSetProviders() no longer needs a provider class; on 2.6.0-2.6.4 the documented setup loaded nothing. * docs: DrupalSetProvider deprecation and the ^2.6.5 requirement * fix: keep wrapper imports out of the generated hook class With importNames(), Rector 2.8 re-runs rules on a changed file, so a later HookConvertRector pass over the .module file also sees the LegacyHook and hook-class imports added for the wrappers, and copied them into the hook class. The class then imports its own name, a fatal "Cannot declare class ... because the name is already in use". Found in project_analysis: every hook conversion on Rector 2.8.3 produced a broken hook class. * fix: guard the BC-call check against a missing scope Rector 2.8 leaves some nodes without a scope. isInBackwardsCompatibleCall() called getFunctionCallStackWithParameters() through null, which aborted the whole file: 22 test files in the project_analysis test set on Rector 2.8.3. Without a scope there is no call stack to be inside, so treat it as not wrapped. * feat: load the composer-based set in the shipped rector.php * feat: add rector.contrib.php for contrib modules and themes * docs: site and contrib configs, and the withComposerBased() crash * docs: tighten the unreleased changelog entries * test: drop the unused LegacyHook import from the functional fixture --------- Co-authored-by: Björn Brala <bjorn@swis.nl> Co-authored-by: Björn Brala <github@bjorn.dev>
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
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.
Follow-up to #495. Two gaps remained after the composer-based bonding.
1.
AddGetReferenceTypeRectorhad no composer constraintThe rule fills the second
getReference()argument, which only exists from doctrine/data-fixtures 1.6 (data-fixtures#409). The set provider already knew the version — it registersdata-fixtures-16.phpas aComposerTriggeredSetat1.6— but the rule itself carried noComposerPackageConstraintInterface, and unlike itsdata-fixtures-17sibling it was never added tocomposer-based.php.What the rule does, now only on data-fixtures >= 1.6:
final class SomeFixture extends AbstractFixture { public function run(SomeEntity $someEntity) { - $someEntity->setSomePassedEntity($this->getReference('some-id')); + $someEntity->setSomePassedEntity($this->getReference('some-id', SomePassedEntity::class)); } }doctrine/data-fixturesis added torequire-devso the rule test actually exercises the constraint instead of silently skipping.2. Annotation-to-attribute sets are now composer-bound too
ANNOTATIONS_TO_ATTRIBUTES,GEDMO_ANNOTATIONS_TO_ATTRIBUTESandMONGODB_ANNOTATIONS_TO_ATTRIBUTESconvert annotations to attribute classes that only exist from a specific package version. Each config block moves intocomposer-based.phpbound to the version that first shipped those classes as PHP 8 attributes:doctrine/orm>=2.9#[Attribute]onMapping\Entityabsent in 2.8.0, present in 2.9.0doctrine/mongodb-odm>=2.3doctrine/mongodb-odm-bundle>=4.4Validator\Constraints\Uniqueabsent in 4.3.0, present in 4.4.0gedmo/doctrine-extensions>=3.5Mapping\Annotation\Slugabsent in v3.4.0, present in v3.5.0The
Uniqueconstraint previously travelled inside the MongoDB set but belongs to a different package, so it gets its own bound.Result on an ORM >= 2.9 project using
withComposerBased():The standalone set files stay in place, same as the version sets in #495 — the
DoctrineSetListconstants remain usable.Heads-up on scope
This makes annotation-to-attribute conversion part of
withComposerBased()rather than an explicit opt-in set. Any project on ORM >= 2.9 and PHP 8 (AnnotationToAttributeRectoris still gated on PHP 8.0 by its own min-version) will now get its mapping annotations rewritten to attributes. That is a deliberate migration rather than a version-compat fix, so it is worth a conscious call before merging.Trigger packages
COMPOSER_BASED_TRIGGER_PACKAGESgainsdoctrine/mongodb-odm-bundleandgedmo/doctrine-extensions, lowersdoctrine/mongodb-odmto>=2.3anddoctrine/data-fixturesto>=1.6, so the set is picked up for projects on those versions.New
tests/ComposerBasedfixture covers the ORM annotation-to-attribute path end to end through the composer-based set.