Repository: lirantal/lockfile-lint
Pull request #134 (2022-07-27 08:52:00)

feat: add format options for report output
## Description

Adds a flag for setting what type of report output is desired
- plain - raw text essentially without colors/symbols
- pretty - (default) same as before with colors symbols

## Types of changes

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Related Issue

#133 

## Motivation and Context

Sadly many environments and parsing tools don't like fancy characters or color sequences, this just gives an alternative friendly output for more basic systems.

## How Has This Been Tested?

- Tested manually under WSL windows
- Unit tests added for error output

I could add tests for the 'success' (no errors) path if desired.

## 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! -->

- [x] 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.
- [x] I added a picture of a cute animal cause it's fun
🦍 <-- cute animal


rheimus: Interesting note, this repo does not work under native windows, has problems spawning processes for the unit tests.
lirantal: We'll look into it shortly @rheimus 🙏🏼
lirantal: @rheimus looks good to me to land. Thank you for refactoring to support the error/warn/success functions and building this plain/pretty capability into the lockfile-lint CLI. Much appreciated 👏🙏🏼
codecov-commenter: # [Codecov](https://codecov.io/gh/lirantal/lockfile-lint/pull/134?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Liran+Tal) Report
> Merging [#134](https://codecov.io/gh/lirantal/lockfile-lint/pull/134?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Liran+Tal) (875e6d9) into [master](https://codecov.io/gh/lirantal/lockfile-lint/commit/48a93a28d17b6f6fa10cb2efa2a786ca3acd228a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Liran+Tal) (48a93a2) will **not change** coverage.
> The diff coverage is `n/a`.

> :exclamation: Current head 875e6d9 differs from pull request most recent head 5d0693c. Consider uploading reports for the commit 5d0693c to get more accurate results

```diff
@@           Coverage Diff           @@
##           master     #134   +/-   ##
=======================================
  Coverage   97.75%   97.75%           
=======================================
  Files          12       12           
  Lines         312      312           
  Branches       67       67           
=======================================
  Hits          305      305           
  Misses          7        7           
```


| [Impacted Files](https://codecov.io/gh/lirantal/lockfile-lint/pull/134?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Liran+Tal) | Coverage Δ | |
|---|---|---|
| [packages/lockfile-lint/src/config.js](https://codecov.io/gh/lirantal/lockfile-lint/pull/134/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Liran+Tal#diff-cGFja2FnZXMvbG9ja2ZpbGUtbGludC9zcmMvY29uZmlnLmpz) | `100.00% <ø> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Liran+Tal). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Liran+Tal)

rheimus: Applied the requested changes and fixed a typo in the plain formatting test set.
lirantal: Looks great, thank you!

Patch:
diff --git a/packages/lockfile-lint/README.md b/packages/lockfile-lint/README.md
index ce306a4..8a1aaab 100644
--- a/packages/lockfile-lint/README.md
+++ b/packages/lockfile-lint/README.md
@@ -80,6 +80,7 @@ lockfile-lint --path yarn.lock --allowed-hosts yarn --allowed-urls https://githu
 | -------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------- |
 | `--path`, `-p`                   | path to the lockfile                                                                                                                                                                                                                                                                       | ✅             |
 | `--type`, `-t`                   | lockfile type, options are `npm` or `yarn`                                                                                                                                                                                                                                                 | ✅             |
+| `--format`, `-f` | sets what type of report output is desired, one of [ `pretty`, `plain` ] with `plain` removing colors & status symbols from output | ✅             |
 | `--validate-https`, `-s`         | validates the use of HTTPS as protocol schema for all resources in the lockfile                                                                                                                                                                                                            | ✅             |
 | `--allowed-hosts`, `-a`          | validates a list of allowed hosts to be used for all resources in the lockfile. Supported short-hands aliases are `npm`, `yarn`, and `verdaccio` which will match URLs `https://registry.npmjs.org`, `https://registry.yarnpkg.com` and `https://registry.verdaccio.org` respectively      | ✅             |
 | `--allowed-schemes`, `-o`        | allowed [URI schemes](https://tools.ietf.org/html/rfc2396#section-3.1) such as "https:", "http", "git+ssh:", or "git+https:"                                                                                                                                                               | ✅             |
diff --git a/packages/lockfile-lint/bin/lockfile-lint.js b/packages/lockfile-lint/bin/lockfile-lint.js
index a5c4fd5..4bbc5d0 100755
--- a/packages/lockfile-lint/bin/lockfile-lint.js
+++ b/packages/lockfile-lint/bin/lockfile-lint.js
@@ -36,6 +36,8 @@ try {
   process.exit(1)
 }
 
+const isPrettyFormat = config['format'] === 'pretty'
+
 let validators = []
 const supportedValidators = new Map([
   ['allowed-hosts', 'validateHosts'],
@@ -80,15 +82,10 @@ try {
     validators
   })
 } catch (error) {
-  console.error(
-    YELLOW,
-    `${symbols.info} ABORTING lockfile lint process due to error exceptions`,
-    '\n',
-    RESET
-  )
+  warn('ABORTING lockfile lint process due to error exceptions')
   console.error(error.message, '\n')
   console.error(error.stack, '\n')
-  console.error(RED, `${symbols.error} Error: command failed with exit code 1`, '\n', RESET)
+  error('Error: command failed with exit code 1')
   process.exit(1)
 }
 
@@ -99,8 +96,44 @@ debug(`total validator failures: ${validatorFailures}`)
 debug(`total validator successes: ${validatorSuccesses}`)
 
 if (validatorFailures !== 0) {
-  console.error(RED, `${symbols.error} Error: security issues detected!`, '\n', RESET)
+  error('Error: security issues detected!')
   process.exit(1)
 } else {
-  console.info(GREEN, `${symbols.success} No issues detected`, '\n', RESET)
+  success('No issues detected')
+}
+
+function success (message) {
+  const m = [
+    isPrettyFormat ? GREEN : '',
+    isPrettyFormat ? symbols.success : '',
+    message,
+    '\n',
+    isPrettyFormat ? RESET : ''
+  ].filter(e => !!e)
+
+  console.info(m.join(' '))
+}
+
+function warn (message) {
+  const m = [
+    isPrettyFormat ? YELLOW : '',
+    isPrettyFormat ? symbols.info : '',
+    message,
+    '\n',
+    isPrettyFormat ? RESET : ''
+  ].filter(e => !!e)
+
+  console.error(m.join(' '))
+}
+
+function error (message) {
+  const m = [
+    isPrettyFormat ? RED : '',
+    isPrettyFormat ? symbols.error : '',
+    message,
+    '\n',
+    isPrettyFormat ? RESET : ''
+  ].filter(e => !!e)
+
+  console.error(m.join(' '))
 }
diff --git a/packages/lockfile-lint/src/config.js b/packages/lockfile-lint/src/config.js
index 2dfefde..7564213 100644
--- a/packages/lockfile-lint/src/config.js
+++ b/packages/lockfile-lint/src/config.js
@@ -74,6 +74,13 @@ module.exports = (argv, exitProcess = false, searchFrom = process.cwd()) => {
         alias: ['allowed-urls'],
         type: 'array',
         describe: 'validates a whitelist of allowed URLs to be used for resources in the lockfile'
+      },
+      f: {
+        alias: ['format'],
+        type: 'string',
+        description: 'format of the report output',
+        choices: ['plain', 'pretty'],
+        default: 'pretty'
       }
     })
     .example('lockfile-lint --path yarn.lock --validate-https')


Test patch:
diff --git a/packages/lockfile-lint/__tests__/cli.test.js b/packages/lockfile-lint/__tests__/cli.test.js
index 9975604..471d883 100644
--- a/packages/lockfile-lint/__tests__/cli.test.js
+++ b/packages/lockfile-lint/__tests__/cli.test.js
@@ -149,6 +149,82 @@ describe('CLI tests', () => {
     })
   })
 
+  test('default usage has output with symbols and color', done => {
+    const process = childProcess.spawn('node', [
+      cliExecPath,
+      '--type',
+      'yarn',
+      '--path',
+      '__tests__/fixtures/yarn-incorrect-package-name.lock',
+      '--validate-package-names',
+      '--allowed-hosts',
+      'yarn'
+    ])
+
+    let output = ''
+    process.stderr.on('data', chunk => {
+      output += chunk
+    })
+
+    process.stderr.on('close', _ => {
+      expect(output).toMatch(/[×✖]/)
+      expect(output).toContain('\x1b[0m')
+      done()
+    })
+  })
+
+  test('When using --pretty formatting then output has symbols and color', done => {
+    const process = childProcess.spawn('node', [
+      cliExecPath,
+      '--type',
+      'yarn',
+      '--path',
+      '__tests__/fixtures/yarn-incorrect-package-name.lock',
+      '--validate-package-names',
+      '--allowed-hosts',
+      'yarn',
+      '--format',
+      'pretty'
+    ])
+
+    let output = ''
+    process.stderr.on('data', chunk => {
+      output += chunk
+    })
+
+    process.stderr.on('close', _ => {
+      expect(output).toMatch(/[×✖]/)
+      expect(output).toContain('\x1b[0m')
+      done()
+    })
+  })
+
+  test('When using --plain formatting then output does not have symbols or color', done => {
+    const process = childProcess.spawn('node', [
+      cliExecPath,
+      '--type',
+      'yarn',
+      '--path',
+      '__tests__/fixtures/yarn-incorrect-package-name.lock',
+      '--validate-package-names',
+      '--allowed-hosts',
+      'yarn',
+      '--format',
+      'plain'
+    ])
+
+    let output = ''
+    process.stderr.on('data', chunk => {
+      output += chunk
+    })
+
+    process.stderr.on('close', _ => {
+      expect(output).not.toMatch(/[×✖]/)
+      expect(output).not.toContain('\x1b[0m')
+      done()
+    })
+  })
+
   describe('cosmiconfig integration', () => {
     it('options are loaded from cosmiconfig files', done => {
       const lintProcess = childProcess.spawn(cliExecPath, [], {
diff --git a/packages/lockfile-lint/__tests__/config.test.js b/packages/lockfile-lint/__tests__/config.test.js
index 28e5cb5..531c1d1 100644
--- a/packages/lockfile-lint/__tests__/config.test.js
+++ b/packages/lockfile-lint/__tests__/config.test.js
@@ -88,14 +88,17 @@ describe('config', () => {
         '--empty-hostname',
         'false',
         '--validate-package-names',
-        'true'
+        'true',
+        '--format',
+        'plain'
       ],
       expected: {
         path: 'yarn.lock',
         type: 'npm',
         validateHttps: true,
         validatePackageNames: true,
-        emptyHostname: false
+        emptyHostname: false,
+        format: 'plain'
       }
     }
   ].forEach(({name, args, expected}) => {