Repository: lirantal/lockfile-lint (js)

Issue:
Bug: CLI Argument parsing typos invokes other validates
<!--- Provide a general summary of the issue in the Title above -->

## Expected Behavior
<!--- If you're describing a bug, tell us what should happen -->
<!--- If you're suggesting a change/improvement, tell us how it should work -->

Not to validate the integrity by default via CLI command
e.g.) 
```
npx lockfile-lint --validate-https -validate-package-names --validate-integrity --path yarn.lock
```


## Current Behavior

<!--- If describing a bug, tell us what happens instead of the expected behavior -->
<!--- If suggesting a change/improvement, explain the difference from current behavior -->
```
npx lockfile-lint --validate-https -validate-package-names --validate-integrity false --path yarn.lock
```
Always seems to validate the integrity is sha512



## Possible Solution

<!--- Not obligatory, but suggest a fix/reason for the bug, -->
<!--- or ideas how to implement the addition or change -->

## Steps to Reproduce (for bugs)

<!--- Provide a link to a live example, or an unambiguous set of steps to -->
<!--- reproduce this bug. Include code to reproduce, if relevant -->

1. Run `npx lockfile-lint --validate-https -validate-package-names --validate-integrity --path yarn.lock`
with a lockfile that is using sha1 integrity
<img width="659" alt="image" src="https://user-images.githubusercontent.com/1212576/193951700-6fbf701b-fb4e-4445-908c-3662089b0be7.png">

Have also tried passing false as an aargument to `--validate-integrity` which also did not work


## Context

<!--- How has this issue affected you? What are you trying to accomplish? -->
<!--- Providing context helps us come up with a solution that is most useful in the real world -->

## Your Environment

<!--- Include as many relevant details about the environment you experienced the bug in -->

- Library Version used: 4.9.5
- Node.js version (e.g. Node.js 5.4): 16
- Operating System and version (desktop or mobile): MacOS 12.6


Pull request:
Fix #145: cli parsing - handle correctly false value for validator
<!--- Provide a general summary of your changes in the Title above -->

## Description
When adding a validator boolean flag to command line, the value was ignored, and the validator was activated even if its value was "false".

<!--- Describe your changes in detail -->

## Types of changes
* Fixed some CLI tests failing on Windows with "Error: spawn UNKNOWN" due to running with `childProcess.spawn(cliExecPath, [...])`. Changed to `childProcess.spawn('node', [cliExecPath, ...])`
* Added validator only if commandValue is not falsy 
* Added tests to make sure validator does not run when flag is missing, or flag is set to "false"
* Added another cli test for the integrity validator

>**Note:** This does not resolve command-line with typos.

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Breaking change (fix or feature that would cause existing functionality to change)

## Related Issue
https://github.com/lirantal/lockfile-lint/issues/145

<!--- This project only accepts pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps to reproduce -->
<!--- Please link to the issue here: -->

## Motivation and Context

<!--- Why is this change required? What problem does it solve? -->

## How Has This Been Tested?

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
<!--- Unit tests are expected for all changes. -->

## Screenshots (if appropriate):

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [ ] I have updated the documentation (if required).
- [X] I have read the **CONTRIBUTING** document.
- [X] I have added tests to cover my changes.
- [X] All new and existing tests passed.
- [ ] I added a picture of a cute animal cause it's fun


Patch:
diff --git a/packages/lockfile-lint/bin/lockfile-lint.js b/packages/lockfile-lint/bin/lockfile-lint.js
index 66b3904..e1b959e 100755
--- a/packages/lockfile-lint/bin/lockfile-lint.js
+++ b/packages/lockfile-lint/bin/lockfile-lint.js
@@ -61,7 +61,7 @@ for (const [commandArgument, commandValue] of Object.entries(config)) {
     continue
   }
 
-  if (supportedValidators.has(commandArgument)) {
+  if (commandValue && supportedValidators.has(commandArgument)) {
     const validatorItem = supportedValidators.get(commandArgument)
     validators.push({
       name: validatorItem,


Test patch:
diff --git a/packages/lockfile-lint/__tests__/cli.test.js b/packages/lockfile-lint/__tests__/cli.test.js
index 94d4d4a..bdeda69 100644
--- a/packages/lockfile-lint/__tests__/cli.test.js
+++ b/packages/lockfile-lint/__tests__/cli.test.js
@@ -104,8 +104,81 @@ describe('CLI tests', () => {
     })
   })
 
+  test('Linting a file that has invalid integrity hash type should return exit code 1', done => {
+    const process = childProcess.spawn('node', [
+      cliExecPath,
+      '--type',
+      'npm',
+      '--path',
+      '__tests__/fixtures/package-lock-sha1.json',
+      '--validate-integrity',
+      '--allowed-hosts',
+      'npm'
+    ])
+
+    let output = ''
+    process.stderr.on('data', chunk => {
+      output += chunk
+    })
+
+    process.on('close', exitCode => {
+      expect(output.indexOf('detected invalid integrity hash type for package')).not.toBe(-1)
+      expect(exitCode).toBe(1)
+      done()
+    })
+  })
+
+  test('Linting should not run a validator when its flag is not set', done => {
+    const process = childProcess.spawn('node', [
+      cliExecPath,
+      '--type',
+      'npm',
+      '--path',
+      '__tests__/fixtures/package-lock-sha1.json',
+      '--allowed-hosts',
+      'npm'
+    ])
+
+    let output = ''
+    process.stderr.on('data', chunk => {
+      output += chunk
+    })
+
+    process.on('close', exitCode => {
+      expect(output.indexOf('detected invalid integrity hash type for package')).toBe(-1)
+      expect(exitCode).toBe(0)
+      done()
+    })
+  })
+
+  test('Linting should not run a validator when its flag is set to "false"', done => {
+    const process = childProcess.spawn('node', [
+      cliExecPath,
+      '--type',
+      'npm',
+      '--path',
+      '__tests__/fixtures/package-lock-sha1.json',
+      '--validate-integrity',
+      'false',
+      '--allowed-hosts',
+      'npm'
+    ])
+
+    let output = ''
+    process.stderr.on('data', chunk => {
+      output += chunk
+    })
+
+    process.on('close', exitCode => {
+      expect(output.indexOf('detected invalid integrity hash type for package')).toBe(-1)
+      expect(exitCode).toBe(0)
+      done()
+    })
+  })
+
   test('Providing conflicting arguments should display an error', done => {
-    const process = childProcess.spawn(cliExecPath, [
+    const process = childProcess.spawn('node', [
+      cliExecPath,
       '--type',
       'yarn',
       '--path',
@@ -127,7 +200,8 @@ describe('CLI tests', () => {
   })
 
   test('Allowed hosts and allowed urls flags should work together', done => {
-    const process = childProcess.spawn(cliExecPath, [
+    const process = childProcess.spawn('node', [
+      cliExecPath,
       '--type',
       'yarn',
       '--path',
@@ -227,7 +301,7 @@ describe('CLI tests', () => {
 
   describe('cosmiconfig integration', () => {
     it('options are loaded from cosmiconfig files', done => {
-      const lintProcess = childProcess.spawn(cliExecPath, [], {
+      const lintProcess = childProcess.spawn('node', [cliExecPath], {
         cwd: path.join(__dirname, 'fixtures/valid-config')
       })
 
@@ -244,9 +318,13 @@ describe('CLI tests', () => {
     })
 
     it('command-line options take precedence', done => {
-      const lintProcess = childProcess.spawn(cliExecPath, ['-p', '../yarn-only-http.lock'], {
-        cwd: path.join(__dirname, 'fixtures/valid-config')
-      })
+      const lintProcess = childProcess.spawn(
+        'node',
+        [cliExecPath, '-p', '../yarn-only-http.lock'],
+        {
+          cwd: path.join(__dirname, 'fixtures/valid-config')
+        }
+      )
 
       lintProcess.on('close', exitCode => {
         expect(exitCode).toBe(1)
@@ -256,8 +334,8 @@ describe('CLI tests', () => {
 
     it('invalid config files are ignored', done => {
       const lintProcess = childProcess.spawn(
-        cliExecPath,
-        ['-p', '../yarn-only-https.lock', '--type', 'yarn', '--validate-https'],
+        'node',
+        [cliExecPath, '-p', '../yarn-only-https.lock', '--type', 'yarn', '--validate-https'],
         {
           cwd: path.join(__dirname, 'fixtures/invalid-config'),
           env: Object.assign({}, process.env, {DEBUG: 'lockfile-lint'})