Validate command doesn't detect missing packages anymore, neither does 'update nothing'/'update --lock'

[PrinsFrank]

Since Composer 2.0.0 (still present in 2.0.12), packages that are present in the composer.json but are not locked in the composer.lock or installed in the vendor folder are not correctly detected as an out of sync lock file, or resolved when running composer update nothing or composer update --lock. In composer 1.10.21 and before, this behaviour was correct.

Reproduction

1. Create a new folder with this composer.json:

{
    "name": "reproduction/reproduction",
    "description": "reproduction",
    "license": "MIT",
    "require": {
        "php": "^7.4"
    }
}

2. Run composer update to generate an up to date lock file.
3. Add an extra dependency manually in the composer.json file (For example a dependency to "composer/semver": "^3.0")

This is not the correct way to add a dependency, but the composer.json and composer.lock might get out of sync in invalid merges, or editors like phpstorm suggest edits to the composer.json without updating the lock file etc.

4. Run composer validate. An error about an out of date lock file is displayed:
   The lock file is not up to date with the latest changes in composer.json, it is recommended that you run `composer update` or `composer update <package name>`.
5. Run composer u nothing or composer u --lock
6. Run composer validate. The warning is now gone implicating the lock file is up to date. The lock file is not up to date however as dependencies from the composer.json are not in the lock file or installed into the vendor folder.

Expected behaviour

1. When a package is present in the composer.json file but not locked in the composer.lock file a warning is displayed when running composer validate.
2. When composer update nothing or composer update --lock is ran, the package is installed and added to the lock file (How this previously worked in v1.*) or a warning is displayed to not manually add to the composer.json file with a message to run composer require vendor/package-name to require the package correctly and fix the discrepancy between the two files.

[stof]

@Seldaek this looks like a bug of composer update --lock. It should not only change the content-hash if the rest does not match

[PrinsFrank]

I've just looked more thoroughly trough the code for anything that might give this functionality and couldn't find where this should be present. Maybe this was lost during the move to composer 2? If anyone can point me to a place I should look at I'm willing to contribute.

I do however doubt when this is not a current feature anymore if this should be really a functionality of the composer update --lock command, or if this should just be a warning/error with instructions on how to re-require when running composer validate. Currently, the composer update --lock only regenerates the lock file metadata, so with seperation of concerns I doubt if it is really the right place.

Please let me know how I can contribute!

[PrinsFrank]

@Seldaek I just did a more deep dive, and found the revision where the new behaviour for updating the lock file was introduced with issue #9514 attached. (Installer.php:837-863) You made the same point I was trying to make above, that installing missing packages is "doing a bit more than --lock should do" .

I do have some time to open a PR. Wich one of the following would you prefer?

1. Partially revert the behaviour and automatically add missing root requirements with their child dependencies to the lock file.
2. Introduce an error when a user is updating their lock file while there are packages that are missing in it with instructions on how to re-require them?

[Seldaek]

1. IMO is not acceptable, we discussed this already in the linked issues.. I don't think it's good to silently add new requirements at that stage.
2. composer install on a lock file which is missing a package currently works fine. If we'd error on install though, then the update --lock would fail in the first place during the install step as the generated lock file wouldn't be installable anymore.

So really this seems like a workflow problem more than anything to me. If your way of conflict resolution is "take old lock file from main branch and run composer update --lock to shove stuff under the rug", then it'll not be fine. Adding new requirements or warning if they're missing may fix part of the problem, but not the entire problem. If the PR you merged contained some critical security update for example you just lost that.

The correct way to resolve such conflicts is to replay the updates that were done in the branch over the latest lock file, to make sure you get the same versions again (or greater I guess..).

[PrinsFrank]

I agree it should not be a part of anyone's workflow and no incorrect shortcuts should be offered, but when another developer does incorrectly merge there is currently no way of figuring that out besides looking thoroughly at the lock file in pull/merge-requests for the added package(s), something that could potentially be missed.

I wholeheartedly agree on the correct way to merge lock files, but have worked in several teams where this workflow was either not clear or developers still took the shortcut because they didn't see or care about the consequences. (hence the PR for the article) Besides, there's currently no way to enforce this correct way of merging aside from only allowing certain people to commit on the lock file.

Previously, running composer validate in a merge-blocking pipeline prevented at least that an out of sync lock file went unnoticed. It did give a hint about the composer u --lock command, which encouraged taking this shortcut.

What do you think about adding the following error when running composer validate with missing (or too much) packages in the lock file?

The composer.lock file is not in sync with the packages required in your composer.json file.
This usually happens when the lock file is incorrectly merged.
Read more about correctly resolving merge conflicts -> https://getcomposer.org/doc/articles/resolving-merge-conflicts.md

Adding this error would mean that debugging this scenario is more easy (currently you have to run composer why vendor/package on all required packages to even detect the scenario), and would also mean that it is possible to block any merges early on by failing a merge/PR pipeline.

[Seldaek]

Yeah if you can reasonably add that functionality to the ValidateCommand that sounds good to me. Detecting missing packages is fairly easy going through require/require-dev and seeing if the names are provided by the lock repository. Detecting superfluous packages may be a bit trickier, but it's also less important so not sure it's worth doing.