Repository navigation
[Configuration] Auto-detect installed packages in withComposerBased() called with no argument - #8748
Conversation
samsonasik
left a comment
There was a problem hiding this comment.
I think "framework" can be changed to "package" for generic wording
|
I like the idea 👍 The Symfony/Laravel would have to be triggered by multiple packages though. E.g. |
|
Thanks to both of you, updated in 995bbfe:
The test now uses a fixture project with |
|
The failing "PHP 8.4 tests (ubuntu-latest)" job is not related to this PR. It is a race between two test classes that share one fixture directory.
The first push passed this job, and nothing under |
| 'twig' => ['twig/twig'], | ||
| 'doctrine' => ['doctrine/orm', 'doctrine/dbal', 'doctrine/doctrine-bundle'], | ||
| 'phpunit' => ['phpunit/phpunit'], | ||
| 'symfony' => ['symfony/framework-bundle', 'symfony/http-kernel', 'symfony/console'], |
There was a problem hiding this comment.
This should be 20-30 packages, check the linked set for more packages we upgrade.
Ideally build a script that extracts these values, and commit it here, so we have this list up-to-date.
There was a problem hiding this comment.
Done in aa524fe. scripts/generate-composer-based-trigger-packages.php loads each composer-based set and collects the packages its rules are bound to. It covers both ComposerPackageConstraintInterface rules and ruleWithConfigurationComposerVersionBound() configurations, and writes them to src/Configuration/ComposerBasedTriggerPackages.php.
The result is 57 packages for Symfony and 9 for Doctrine. Laravel and Drupal stay hardcoded in the script, because rector-laravel and drupal-rector are not required in this repository. A test checks that every withComposerBased() argument has an entry in the generated list.
Should CI also run the script and fail when the generated list is out of date? I left that out because rector-symfony and rector-doctrine are pulled in as dev-main. Any change there would then fail unrelated PRs until someone regenerates the list. I'm happy to add it if you prefer.
…() called with no argument
02c2666 to
383581d
Compare
|
Lets ship it, thank you for all the improvements 👍 |
withPhpSets()with no argument reads the PHP version fromcomposer.json.withComposerBased()with no argument currently does nothing, so every project lists its packages by hand:This PR makes the no-argument call pick the sets of the packages installed in the project:
->withComposerBased()Passing any argument keeps today's behavior: only the passed sets are loaded, and detection is skipped.
withComposerBased(phpunit: false)loads nothing, as before.Detection
A set loads when any of its trigger packages is installed.
InstalledPackageResolverreads the installed packages.The trigger packages are listed in
src/Configuration/ComposerBasedTriggerPackages.php. That file is generated byphp scripts/generate-composer-based-trigger-packages.php, which reads every package the composer-based sets are bound to. It covers both rules that implementComposerPackageConstraintInterfaceand configurations registered withruleWithConfigurationComposerVersionBound(). The "Code Analysis" workflow runs the script on every commit and fails when the generated file is out of date.symfonysymfony/*components and bundles, plustwig/twigdoctrinedoctrine/orm,doctrine/dbal,doctrine/doctrine-bundle,doctrine/mongodb-odm,gedmo/doctrine-extensions, and otherstwigtwig/twigphpunitphpunit/phpunitlaravellaravel/framework,illuminate/support(hardcoded in the script, as rector-laravel is not required here)drupaldrupal/core(hardcoded in the script, as drupal-rector is not required here)Each rule in these sets is bound to its own package version, so a broad trigger is safe. A rule bound to a package that is not installed stays inactive.
BC
bool ... = false). The no-argument path is detected withfunc_num_args() === 0.withComposerBased()with no arguments loaded nothing before. Now it loads the detected sets, which is the intent of such a call.Tests
tests/Configuration/ComposerBasedAutoDetectTest.phpuses two fixture projects. One hasdoctrine/dbal,phpunit/phpunitandsymfony/consoleinstalled; the other has none of the trigger packages. It covers four cases, and a fifth test checks that everywithComposerBased()argument has an entry in the generated list:doctrine/dbaltriggers Doctrine andsymfony/consoletriggers Symfony.twig: trueloads only Twig.phpunit: falseloads nothing.