[Composer] Target lowest declared version for libraries in composer-based sets - #8359
Conversation
…ased sets A library declares a compatibility range in composer.json (e.g. "^10.5 || ^11.0 || ^12.0"). Composer-based rule filtering used the locally installed version, so an installed PHPUnit 12 made Rector emit 12-only code and silently break the declared support for 10.5. When composer.json has "type": "library", derive the target version from the lowest declared constraint instead of the installed one, making the output deterministic across composer install/update and --prefer-lowest. Refs rectorphp/rector#9858
| private function createInstalledPackages(array $packages): array | ||
| { | ||
| $packageConstraints = $this->resolvePackageConstraints(); | ||
| $isLibrary = $this->isLibrary(); |
There was a problem hiding this comment.
I think library check is not needed, even on project, eg on "framework skeleton", phpunit range may exists to give user ability to use phpunit version based on specific php version.
There was a problem hiding this comment.
The goal is to give projects real latest versions (those in installed.json),
and to give packagest the lowest safest ones. As ^7.3 can be 7.3.0 on a library, but 7.3.50 on final real project
|
Lets give it a go 👍 |
| { | ||
| $projectComposerJson = $this->loadProjectComposerJson(); | ||
|
|
||
| return ($projectComposerJson['type'] ?? null) === 'library'; |
There was a problem hiding this comment.
This is wrong. It should be inverted to isProject() and compared to project. Otherwise you exclude symfony-bundle etc. pp. :)
There was a problem hiding this comment.
"type" may be missing, most likely in projects, so we have to go with a default here.
Could you elaborate a bit more?
There was a problem hiding this comment.
I think we should consider something a project if type equals project. Everything else is not a project and thus should be considered a "library".
There was a problem hiding this comment.
Or in other words: the current solution does not fix the original issue. I have a type symfony-bundle and thus I still get the wrong rules applied.
There was a problem hiding this comment.
I get your point, but if we do that, all projects will be upgraded only when someone fill the keyword explicitly in composer.json.
If a keyword is an issue, then other non-project keywords should be included as well.
There was a problem hiding this comment.
Personally, I'm convinced there should be no check at all and the composer.json should always win. I always want to have to rules applied to "what is compatible", never ever to "what is installed".
There was a problem hiding this comment.
That's why composer.lock/installed.json is present. When project uses ^7.4, they rarely want to use only featuers in 7.4.0. Which is version mostly skipped for being to risky to use.
There was a problem hiding this comment.
That's exactly the point. If they specify ^7.4, the code must be compatible with 7.4.0. So the composer.json decides, no matter if your installed version is 7.4, 8.0 or 8.5.
There was a problem hiding this comment.
I agree with @Toflar here. composer.json should always be leading, as that signals which what versions the project is compatible with. Installed versions can always be downgraded in a later stage (if another dependency adds an additional constraint for example), but will never be downgraded beyond the given range.
4 participants
Fixes deterministic version targeting for libraries in composer-based sets. Refs rectorphp/rector#9858.
Problem
withComposerBased()rules gate on the locally installed package version (vendor/composer/installed.json). That is right for applications, wrong for libraries.A library declares a compatibility range in
composer.json:{ "type": "library", "require": { "phpunit/phpunit": "^10.5 || ^11.0 || ^12.0" } }With PHPUnit 12 installed locally, Rector emitted 12-only code and silently broke the declared support for 10.5. Output changed between
composer install,composer update, andcomposer update --prefer-lowestfor identical source.