fix: error handling for empty yarn lock files (#158)

[candrews]

Description

fix error handling for empty yarn lock files (#158)

Currently, running any validator against an empty lock file results in a exception with a stack trace, like this:

$ touch yarn.lock && npx lockfile-lint@4.10.1 --path yarn.lock --type yarn --allowed-hosts yarn npm yarn
 ℹ ABORTING lockfile lint process due to error exceptions 
 
Unable to parse yarn lockfile "yarn.lock" 

TypeError: undefined is not iterable (cannot read property Symbol(Symbol.iterator))
    at checkSampleContent (/home/candrews/.npm/_npx/456ed9a151c5043a/node_modules/lockfile-lint-api/src/ParseLockfile.js:24:36)
    at yarnParseAndVerify (/home/candrews/.npm/_npx/456ed9a151c5043a/node_modules/lockfile-lint-api/src/ParseLockfile.js:41:49)
    at ParseLockfile.parseYarnLockfile (/home/candrews/.npm/_npx/456ed9a151c5043a/node_modules/lockfile-lint-api/src/ParseLockfile.js:160:20)
    at ParseLockfile.parseSync (/home/candrews/.npm/_npx/456ed9a151c5043a/node_modules/lockfile-lint-api/src/ParseLockfile.js:122:27)
    at ValidateHostManager (/home/candrews/.npm/_npx/456ed9a151c5043a/node_modules/lockfile-lint/src/validators/index.js:49:27)
    at /home/candrews/.npm/_npx/456ed9a151c5043a/node_modules/lockfile-lint/src/main.js:41:28
    at Array.forEach (<anonymous>)
    at Object.runValidators (/home/candrews/.npm/_npx/456ed9a151c5043a/node_modules/lockfile-lint/src/main.js:31:14)
    at Object.<anonymous> (/home/candrews/.npm/_npx/456ed9a151c5043a/node_modules/lockfile-lint/bin/lockfile-lint.js:80:17)
    at Module._compile (node:internal/modules/cjs/loader:1254:14) 

/home/candrews/.npm/_npx/456ed9a151c5043a/node_modules/lockfile-lint/bin/lockfile-lint.js:89
  error('Error: command failed with exit code 1')
  ^

TypeError: error is not a function
    at Object.<anonymous> (/home/candrews/.npm/_npx/456ed9a151c5043a/node_modules/lockfile-lint/bin/lockfile-lint.js:89:3)
    at Module._compile (node:internal/modules/cjs/loader:1254:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
    at Module.load (node:internal/modules/cjs/loader:1117:32)
    at Module._load (node:internal/modules/cjs/loader:958:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:23:47

Node.js v18.16.0
[1]

This PR fixes that issue by adding validation to the validators.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Related Issue

#158

Motivation and Context

Empty lock files are not handled appropriately - this MR fixes that issue.

How Has This Been Tested?

I've added two new unit tests (one for yarn, one for npm).

Screenshots (if appropriate):

n/a

Checklist:

• I have updated the documentation (if required).
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• I added a picture of a cute animal cause it's fun

[lirantal]

Thanks in advance! I'm currently traveling but will do my best to get to this in a few days!

[lirantal]

@candrews I'm not entirely sure I get the expected result of this PR/general capability here.
Do you want to "fail gracefully" or do you want to introduce a change that empty lockfiles are valid? The test suite names are a bit confusing to me with how I read this original PR's intent :)

[candrews]

Do you want to "fail gracefully" or do you want to introduce a change that empty lockfiles are valid?

npm and yarn both consider empty lock files to be invalid, so I think that failing with an error indicating that the lockfile is invalid makes the most sense.

The test suite names are a bit confusing to me with how I read this original PR's intent :)

The names of the tests that I've added in this PR are:

• should fail with an empty yarn lock file
• should fail with an empty npm lock file

I think those are reasonably decent names... if you have a suggestion for improving them, I'm happy to implement it #namingthingsishard

[codecov-commenter]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (969ed05) 97.74% compared to head (d386047) 97.75%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #159      +/-   ##
==========================================
+ Coverage   97.74%   97.75%   +0.01%     
==========================================
  Files          13       13              
  Lines         354      356       +2     
  Branches       77       78       +1     
==========================================
+ Hits          346      348       +2     
  Misses          8        8              

Impacted Files	Coverage Δ
packages/lockfile-lint-api/src/ParseLockfile.js	98.91% <100.00%> (+0.02%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[lirantal]

Are you sure you've actually tested that?

The tests pass because you're asserting on a string that is part of the error, but the error still throws with a full stack trace. Is this what you expect?

[lirantal]

@candrews making sure you see my above comment

[candrews]

Are you sure you've actually tested that? ![image](https://user-images.githubusercontent.com/316371

Yes, I've run similar command as you've run, and now I've even run the same one: node packages/lockfile-lint/bin/lockfile-lint.js --type yarn --path packages/lockfile-lint/__tests__/fixtures/empty.json --allowed-hosts npm I've also run the test suite.

The tests pass because you're asserting on a string that is part of the error, but the error still throws with a full stack trace. Is this what you expect?

It is what I expected. If you pass a lock file that is invalid (like a file containing just the { character) you also get an error with a stack trace. Therefore, the behavior implemented by this PR is consistent with the existing similar cases.

I'm happy to implement a different approach, though - tell me what you want and I'll do my best to make it happen :)

[lirantal]

Ok then, let's land this :-)

[candrews]

Thank you very much!

[lirantal]

Thank you!