False negative when not updating .lock correctly

[Jean85]

Today I was bitten by one mistake that this tool unfortunately didn't catch.

To reproduce:

• start using a dev dependency under src
• move the dependency manually in the composer.json
• run the checker

This happened with 3.8, but I switched to the shim and I can confirm it happens on the latest version too; I've patched my CI with a composer validate which will alert me of the issue, but I would expect this tool to fail too in this case.

[Ocramius]

@Jean85 is that because of composer.lock and installed.json not being in sync?

I don't think this tool should be in the crossfire here: that's already a problem in your setup, and this tool is (correctly) using installed.json as authoritative location for the declaration:

https://github.com/maglnet/ComposerRequireChecker/blob/5a801ae22c3423d0ee0213861158249d23e452ec/src/ComposerRequireChecker/FileLocator/LocateComposerPackageDirectDependenciesSourceFiles.php#LL52C66-L52C66

[Ocramius]

I would expect this tool to fail too in this case.

I would say here that the behavior is undefined: the setup is already broken-ish at this stage 🤔

[Jean85]

I didn't know it used installed.json, I thought it read the composer.lock file. I'm not sure it's undefined though, because looking at installed.json I would expect to see the culprit still as a dev package, since I didn't change the whole setup.

Let me check manually...

[Ocramius]

In theory, installed.json matches whatever composer install ended up doing :D

[Jean85]

Then it's inconsistent in that behavior; I just checked and:

• if I do composer install --no-dev (as my deploy did, releasing a bug) I don't get the package installed (as expected, since Composer is using the composer.lock as authoritative)
• if I do a plain composer install in that inconsistent state, I get the same installed.json, with the package listed as dev (which is the inconsistency that caused my false negative).

[Ocramius]

@Jean85 I would check with @duncan3dc why we did 57cbad2

Specifically, why installed.json vs composer.lock 🤔

Anyway, composer.json alone does not suffice, and that's clear from #105

[Jean85]

This seems the reason: #105 (comment)

The wikimedia/ip-set package is set to not export composer.json.

This causes the tool to ignore the package when it's installed by dist and then report the symbols as unknown.

Do you think it makes sense to report this as a bug upstream to Composer?

[Ocramius]

If you think there is a problem in Composer itself, yes.

IMO, composer.lock taken as authoritative could be a good step here too, perhaps.

[duncan3dc]

I don't recall now why I chose installed.json over composer.lock, but I would presume it's because installed.json matches the format of composer.json, so it was a low impact replacement. If composer.lock contains everything this tool needs then I think it makes sense to use that

[Ocramius]

Yeh, but a reminder for @Jean85 that composer.lock and composer.json being in sync is not this tool's responsibility anyway :D

[Jean85]

Yeh, but a reminder for @Jean85 that composer.lock and composer.json being in sync is not this tool's responsibility anyway :D

Absolutely! But I would expect composer.lock and installed.json to be, and that's why I was proposing to report it upstream to Composer!

[fredden]

I don't think the problem is that composer.lock and installed.json are out of sync, but that composer.lock and has an out-of-date content-hash. When I tested this locally yesterday, I could see that making changes to composer.json alone made the alert from this tool go away, without making any changes to composer.lock nor running any composer ... commands (and by extension installed.json).

Making changes to this tool to work around the problem reported here (composer.lock isn't up to date with composer.json) seems difficult, aside from running composer validate or similar*.

I was thinking we could read composer.lock instead of composer.json, but I can't immediately think of a good way to identify direct requirements without also reading composer.json.

* See an example of how to check the freshness of a lock-file here:
https://github.com/ergebnis/composer-normalize/blob/df1d0f33df60e9ea3e9eda9bfbfa60346cad817c/src/Command/NormalizeCommand.php#L174-L184