"composer validate" aborts with code 1 with "--no-check-lock" and JSON Text parse error on lock-file contents

[ktomk]

given

{
    "config": {
        "lock": false
    }
}

composer config lock false

and then creating a zero-byte composer.lock file

< /dev/null >| composer.lock

"composer validate --no-check-lock" aborts with exit code 1 choking on the composer.lock file not being valid JSON Text.

Despite the --no-check-lock:

$ composer --no-ansi validate --no-check-lock


  [Seld\JsonLint\ParsingException]
  "./composer.lock" does not contain valid JSON
  Parse error on line 1:

  ^
  Expected one of: 'STRING', 'NUMBER', 'NULL', 'TRUE', 'FALSE', '{', '['


validate [--no-check-all] [--no-check-lock] [--no-check-publish] [--no-check-version] [-A|--with-dependencies] [--strict] [--] [<file>]

$ echo $?
1

Expected Behavior

The composer.lock file is not checked as per --no-check-lock which should include dropping any errors in JSON Text processing if the file exists.

Additional Info

Report against

$ composer --version
Composer version 2.2.8 2022-03-15 12:55:20

An object with no properties prevents the abortion:

printf '{}' >| composer.lock

And interestingly further validation checks on the lock file (?):

$ composer config lock true # for brevity
$ composer --no-ansi validate --strict
./composer.json is valid

Most of the validate messages related to the lock file with lock: false and --no-check-lock appear noisy to me, but this is rather cosmetic.

[Seldaek]

Sorry but that's not doable.. when loading a Composer instance the lock file is loaded if present, and if it is broken it fails hard at that point, no matter which command you are running. If you don't want a lock then don't create a null byte file..

[ktomk]

Okay, this is a bit what I thought as well (from output it didn't look like it's from the validate command specifically).

And yes, I did create the null byte composer.lock intentionally in explorative testing.

What wondered me in the end thought is that an empty object as lock file prevented any errors to be shown regarding the lock file (e.g. the warning that it is out of date and one should run composer update) with the verify command.

another edgy case - but a more real scenario - was that when the lock file is disabled in composer.json and an outdated composer.lock is still laying around (e.g. composer.json changed the lock config option between different checkouts or while rebasing), the verify command still suggests to run composer update (which will never write the lock file and therefore don't change a thing).

anyway, this is sort of edge-case material. IIRC the last case can be turned into exit status 0 (SUCCESS) with the --no-check-lock parameter (output is just noise then but at least the exit status is fine which is most important to my use of composer validate).

One idea I have would be to make --no-check-lock implicit for composer validate if {config: {lock: false}}. And perhaps a --check-lock to override. This would make it more fitting and easier to use (unless with lock: false and a composer.lock file present it is in use (e.g. lock: false only prevents creating

For the case with {} (and lock: true) I'm left puzzled most just because I can't explain this behaviour. It seems that if certain properties are missing, nothing is checked (for the lock file). I also tried with a fake content-hash alone and could not provoke the lock file warning/error.

[Seldaek]

another edgy case - but a more real scenario - was that when the lock file is disabled in composer.json and an outdated composer.lock is still laying around (e.g. composer.json changed the lock config option between different checkouts or while rebasing), the verify command still suggests to run composer update (which will never write the lock file and therefore don't change a thing).

That sounds maybe worth having a look.

One idea I have would be to make --no-check-lock implicit for composer validate if {config: {lock: false}}. And perhaps a --check-lock to override. This would make it more fitting and easier to use (unless with lock: false and a composer.lock file present it is in use (e.g. lock: false only prevents creating

This definitely sounds worth doing and easy enough

For the case with {} (and lock: true) I'm left puzzled most just because I can't explain this behaviour. It seems that if certain properties are missing, nothing is checked (for the lock file). I also tried with a fake content-hash alone and could not provoke the lock file warning/error.

I'll try to take a look at this as well, sounds weird.

[ktomk]

That sounds maybe worth having a look.

&

This definitely sounds worth doing and easy enough

I'll give this a try. However - and maybe you know from top of your head - for other composer commands than validate, most importantly update/install with {config: {lock: false}} is the lock-file ignored (apart from the JSON Text parse errors shown OP)?

Because if not, an implicit --no-check-lock then, it would break that contract and I'd consider the current behaviour to be better aligned and therefore preferable as changing here would mean more features but not really an improvement.

[ktomk]

Drafted a PR #10723 .

It is about implicit --no-check-lock (when the setting config.log is false) that can be overridden w/ --check-lock (which always overrides --no-check-lock regardless of position as symfony console a) does not support vice-versa options for boolean b) the symfony console 5.3 workaround InputOption::VALUE_NEGATABLE is n/a as a dependency).

First commit is a small refactoring let me know if it is preferable or any other feedback, then I can offer to un-draft it and reduce the commits @Seldaek .

[ktomk]

fix against 2.2 in pr 10723 now, thankfully @stof already brought me up speed in regard to diverse things incl. phpstan.

[Seldaek]

However - and maybe you know from top of your head - for other composer commands than validate, most importantly update/install with {config: {lock: false}} is the lock-file ignored (apart from the JSON Text parse errors shown OP)?

Nope, it is not ignored, lock: false really only means it will not write a lock file on update.

[Seldaek]

See #10726 for a fix of the lock: false behavior. IMO this is a more expected/intuitive behavior.

[Seldaek]

For the case with {} (and lock: true) I'm left puzzled most just because I can't explain this behaviour. It seems that if certain properties are missing, nothing is checked (for the lock file). I also tried with a fake content-hash alone and could not provoke the lock file warning/error.

I investigated that, and I believe it's because of Locker::isLocked returning false if the packages key is not present, so it assumes it's not locked if the lock file is definitely not a valid lock file. And validate does not check the lock file freshness if it is not locked yet, as there is strictly speaking no error there. IMO this can be left alone.

[ktomk]

Nice & Thanks.