Skip to content

[Sets] Drop per-version composer-triggered sets already covered by composer-based set - #1010

Merged
TomasVotruba merged 3 commits into
mainfrom
remove-duplicated-composer-triggered-sets
Aug 5, 2026
Merged

[Sets] Drop per-version composer-triggered sets already covered by composer-based set#1010
TomasVotruba merged 3 commits into
mainfrom
remove-duplicated-composer-triggered-sets

Conversation

@TomasVotruba

@TomasVotruba TomasVotruba commented Aug 5, 2026

Copy link
Copy Markdown
Member

The Symfony sets were registered twice: once per version as ComposerTriggeredSets, once as composer-based.php. I verified the overlap mechanically — across all 208 triggered sets in Symfony3SetProviderSymfony8SetProvider and TwigSetProvider, 0 rules were missing from the matching composer-based set. Every one of them was a duplicate.

So the per-version registrations go, and only the composer-based sets stay.

 final class Symfony8SetProvider implements SetProviderInterface
 {
     public function provide(): array
     {
-        return [
-            new ComposerTriggeredSet(
-                SetGroup::SYMFONY,
-                'symfony/uid',
-                '8.1',
-                __DIR__ . '/../../../config/sets/symfony/symfony8/symfony81/symfony81-uid.php'
-            ),
-            // ... 5 more
-        ];
+        return [];
     }
 }

The trigger package list goes too

SymfonySetProvider gated composer-based.php behind a hand-maintained list of 52 packages and versions. That list duplicated, one level up, what each rule already declares for itself:

public function provideComposerPackageConstraint(): ComposerPackageConstraint
{
    return new ComposerPackageConstraint('twig/twig', '>=3.21');
}

It had also drifted. Seven packages were gated above the version their rules need — symfony/framework-bundle at >=3.3 where a rule needs 2.5, symfony/routing at >=6.0 where a rule needs 3.4, and so on — and four packages with rules in the set (var-dumper, intl, uid, filesystem) had no entry at all. Every drift like that silently disables rules.

With nothing left to trigger, SymfonySetProvider goes as well. This changes nothing for users: withComposerBased(symfony: true) already adds SymfonySetList::COMPOSER_BASED directly rather than going through SetGroup::SYMFONY, exactly as it does for PHPUnit. The remaining Symfony sets — Configs, Code Quality, Constructor Injection, SwiftMailer and the attribute sets — stay reachable through their SymfonySetList constants.

rectorphp/rector-src#8292 drops the matching new SymfonySetProvider() from SetProviderCollector.

TwigSetProvider keeps its single twig/twig >=1.12 trigger, since SetGroup::TWIG is wired into withComposerBased().

Kept on purpose

Left to migrate

Still outside the composer-based encoding, all plain opt-in sets with no version binding:

  • Configs, Code Quality, Constructor Injection
  • SwiftMailer to Symfony Mailer
  • attribute sets: FOS Rest, JMS, Sensiolabs, Symfony, Symfony Validator

And the 209 per-version set files under config/sets/symfony/symfony3..8/ and config/sets/twig/, now reachable only through the set list constants.

…mposer-based set

Every rule in the per-version composer-triggered sets is already registered in composer-based.php, where it carries its own package version constraint. Keeping both meant the same rules were loaded twice, from two sources of truth.

Only the composer-based set stays composer-triggered. The trigger package list is re-derived from composer-based.php itself, which fixes gates that sat above the version their rules actually need (framework-bundle 3.3 -> 2.5, form and validator 3.0 -> 2.5, yaml 3.1 -> 2.8, routing 6.0 -> 3.4, twig-bridge 6.2 -> 6.1, messenger 6.3 -> 6.2) and adds four packages that had no trigger at all (var-dumper, intl, uid, filesystem).

The per-version set files stay in place and reachable through SymfonySetList and TwigSetList constants. Symfony3-8SetProvider keep empty provide(), since rector-src instantiates them by name.
Every rule inside the set already declares the package and version it needs, so the 52-entry trigger package list only duplicated that information one level up - and each new package had to be added there by hand or its rules would never run.

The set is now a plain Set, listed as "Composer Based".
The composer-based set is reachable through SymfonySetList::COMPOSER_BASED, which is what withComposerBased(symfony: true) uses, and the remaining sets through their SymfonySetList constants. The provider only listed them a second time.
TomasVotruba added a commit to rectorphp/rector-src that referenced this pull request Aug 5, 2026
Symfony3SetProvider to Symfony8SetProvider only registered composer-triggered sets that the Symfony composer-based set already covers, rule for rule. rector-symfony empties them in rectorphp/rector-symfony#1010, so the collector can stop asking them for sets.

SymfonySetProvider and TwigSetProvider stay, as they carry the composer-based sets.
@TomasVotruba
TomasVotruba merged commit 340bfcc into main Aug 5, 2026
7 checks passed
@TomasVotruba
TomasVotruba deleted the remove-duplicated-composer-triggered-sets branch August 5, 2026 09:29
TomasVotruba added a commit to rectorphp/rector-src that referenced this pull request Aug 5, 2026
rectorphp/rector-symfony#1010 trims TwigSetProvider to its composer-based trigger, as every rule of the per-version Twig sets is already in it, bound to the twig/twig version it needs.
TomasVotruba added a commit to rectorphp/rector-src that referenced this pull request Aug 5, 2026
…traintInterface

A set triggered on a single major version has to be repeated for every version an upgrade passes through. A rule bonded with ComposerPackageConstraintInterface, added in #7877, states the exact package version its target API is available from and applies from there upwards, so one set covers every upgrade path.

rectorphp/rector-symfony#1010 and rectorphp/rector-phpunit#758 moved both extensions over, leaving Twig as the only remaining user. The internal resolving of the class is kept working and ignored in phpstan.neon until that lands too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant