Add support for file-based configuration

[mtlewis]

Description

This PR adds support for loading lockfile-lint configuration from a file, rather than supplying it on the command line. It uses cosmiconfig, which gives us a bunch of sensible configuration file support for free. Command-line options always override options specified in config files.

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

Welp - I didn't notice that only PRs with open issues would be accepted before I worked on this, so I created one (#74). Sorry for not quite following the guidelines!

Motivation and Context

See #74.

How Has This Been Tested?

So far, I've only tested this manually. I'll be happy to circle back and add tests if there's a positive reaction to #74.

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]

This is wonderful @mtlewis, thanks for working on this ❤️
Lovely picture too!

I'll review it shortly

[lirantal]

@mtlewis is this a synchronous operation? what happens if it throws?

[mtlewis]

Yes, it's synchronous since we're using cosmiconfigSync. I hadn't spotted that it could throw though, thanks for pointing it out. Newest version of the code now handles errors, ignoring file-based config and logging an error.

[lirantal]

@mtlewis I'm happy to land this.
Are you able to add the relevant tests to match up with code coverage requirement? that's why the build is failing now.

[codecov-io]

Codecov Report

Merging #75 into master will increase coverage by 1.79%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #75      +/-   ##
==========================================
+ Coverage   95.08%   96.87%   +1.79%     
==========================================
  Files          11       11              
  Lines         183      192       +9     
  Branches       29       31       +2     
==========================================
+ Hits          174      186      +12     
+ Misses          8        5       -3     
  Partials        1        1

Impacted Files	Coverage Δ
packages/lockfile-lint/src/config.js	100% <100%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e3cdbe...76df197. Read the comment docs.

[mtlewis]

Hey @lirantal, thanks for your review! I've added some tests. I also did a bit of refactoring of the PR, since the way it was before made it harder to retain the argument validation that's configured in yargs. Instead, we now load the cosmiconfig first, then pass it into yargs as the base config. This allows us to keep the configuration rules in yargs. Let me know what you think!

[mtlewis]

By the way - I'm not sure coverage is currently working in cli.js. I believe I've covered all the new code in cli.js in tests, but both before and after this change, the detected coverage in cli.js is 0. I suspect this could be something to do with the fact that the tests for cli.js are currently executing in a separate process.

I guess the reason CI is failing in this PR is that cli.js now has more lines of code, which means its 0% coverage contributes more to the overall number.

[lirantal]

Yep, I see what you mean there about the cli.js not being covered in unit tests (and indeed in a different process they aren't being counted for).

What if you move this code

lockfile-lint/packages/lockfile-lint/src/cli.js

Lines 5 to 20 in 0a90e42

	const {cosmiconfigSync} = require('cosmiconfig')
	let cosmiconfigResult
	try {
	cosmiconfigResult = cosmiconfigSync('lockfile-lint').search()
	} catch (err) {
	debug(`error encountered while loading configuration: ${err}`)
	}
	let fileConfig = {}
	if (cosmiconfigResult) {
	fileConfig = cosmiconfigResult.config
	debug(
	`loaded the following config from ${cosmiconfigResult.filepath}: ${JSON.stringify(fileConfig)}`
	)
	}

to its own config.js module, add unit test coverage to it, and then require it in cli.js?

Tip: to better understand what lines of code are missing coverage, you can run yarn run coverage:view

[mtlewis]

Hey @lirantal thanks for the feedback and suggestion - how about this: 76df197? Since the whole of cli.js was previously uncovered, I figured it might be good to refactor the whole thing so that it's testable without launching a child process, and then separate out the testing of the option parsing/validation from the cli tests in cli.js.

[lirantal]

Ok, taking a look ;-)

[lirantal]

this looks overall good, thanks @mtlewis 🙏