fix(usage): tie usage to config

This starts us down the path of tying the params our commands accept to
their config items.  For now it is optional, and not every current
config item would cleanly render if we added them today.

The ones that are added here DO render nicely, and we can iterate from
here.  We can also at a later date do the same kind of appraoch with our
positional args.

PR-URL: https://github.com/npm/cli/pull/2908
Credit: @wraithgar
Close: #2908
Reviewed-by: @nlf, @isaacs

4 files changed, 45 insertions, 2 deletions

diff --git a/test/lib/load-all-commands.js b/test/lib/load-all-commands.js
index 2a2d41818..d7eb2eae0 100644
--- a/test/lib/load-all-commands.js
+++ b/test/lib/load-all-commands.js
@@ -1,6 +1,8 @@
 // Thanks to nyc not working properly with proxies this doesn't affect
-// coverage. but it does ensure that every command has a usage that contains
-// its name, a description, and if it has completion it is a function
+// coverage. but it does ensure that every command has a usage that renders,
+// contains its name, a description, and if it has completion it is a function.
+// That it renders also ensures that any params we've defined in our commands
+// work.
 const requireInject = require('require-inject')
 const npm = requireInject('../../lib/npm.js')
 const t = require('tap')
diff --git a/test/lib/set-script.js b/test/lib/set-script.js
index 7a057c503..b6b6e2519 100644
--- a/test/lib/set-script.js
+++ b/test/lib/set-script.js
@@ -31,6 +31,7 @@ test.test('fails when package.json not found', (t) => {
 })
 test.test('fails on invalid JSON', (t) => {
   const SetScript = requireInject('../../lib/set-script.js', {
+    '../../lib/utils/config/definitions.js': {},
     fs: {
       readFile: () => {}, // read-package-json-fast explodes w/o this
       readFileSync: (name, charcode) => {
@@ -45,6 +46,7 @@ test.test('fails on invalid JSON', (t) => {
 test.test('creates scripts object', (t) => {
   var mockFile = ''
   const SetScript = requireInject('../../lib/set-script.js', {
+    '../../lib/utils/config/definitions.js': {},
     fs: {
       readFileSync: (name, charcode) => {
         return '{}'
@@ -70,6 +72,7 @@ test.test('creates scripts object', (t) => {
 test.test('warns before overwriting', (t) => {
   var warningListened = ''
   const SetScript = requireInject('../../lib/set-script.js', {
+    '../../lib/utils/config/definitions.js': {},
     fs: {
       readFileSync: (name, charcode) => {
         return JSON.stringify({
@@ -102,6 +105,7 @@ test.test('warns before overwriting', (t) => {
 test.test('provided indentation and eol is used', (t) => {
   var mockFile = ''
   const SetScript = requireInject('../../lib/set-script.js', {
+    '../../lib/utils/config/definitions.js': {},
     fs: {
       readFileSync: (name, charcode) => {
         return '{}'
@@ -128,6 +132,7 @@ test.test('provided indentation and eol is used', (t) => {
 test.test('goes to default when undefined indent and eol provided', (t) => {
   var mockFile = ''
   const SetScript = requireInject('../../lib/set-script.js', {
+    '../../lib/utils/config/definitions.js': {},
     fs: {
       readFileSync: (name, charcode) => {
         return '{}'
diff --git a/test/lib/shrinkwrap.js b/test/lib/shrinkwrap.js
index b3de4f5fa..faf452ea7 100644
--- a/test/lib/shrinkwrap.js
+++ b/test/lib/shrinkwrap.js
@@ -37,6 +37,7 @@ const mocks = {
     }
   },
   '../../lib/utils/usage.js': () => 'usage instructions',
+  '../../lib/utils/config/definitions.js': {},
 }
 
 t.afterEach(cb => {
diff --git a/test/lib/utils/config/definition.js b/test/lib/utils/config/definition.js
index 25530f723..56e10da0c 100644
--- a/test/lib/utils/config/definition.js
+++ b/test/lib/utils/config/definition.js
@@ -21,6 +21,8 @@ t.test('basic definition', async t => {
     default: 'some default value',
     defaultDescription: '"some default value"',
     type: [Number, String],
+    hint: '<key>',
+    usage: '--key <key>|--key <key>',
     typeDescription: 'Number or String',
     description: 'just a test thingie',
   })
@@ -78,6 +80,39 @@ t.test('basic definition', async t => {
     description: 'asdf',
   })
   t.equal(multi123Semver.typeDescription, '1, 2, 3, or SemVer string (can be set multiple times)')
+  const hasUsage = new Definition('key', {
+    default: 'test default',
+    type: String,
+    description: 'test description',
+    usage: 'test usage',
+  })
+  t.equal(hasUsage.usage, 'test usage')
+  const hasShort = new Definition('key', {
+    default: 'test default',
+    short: 't',
+    type: String,
+    description: 'test description',
+  })
+  t.equal(hasShort.usage, '-t|--key <key>')
+  const hardCodedTypes = new Definition('key', {
+    default: 'test default',
+    type: ['string1', 'string2'],
+    description: 'test description',
+  })
+  t.equal(hardCodedTypes.usage, '--key <string1|string2>')
+  const hardCodedOptionalTypes = new Definition('key', {
+    default: 'test default',
+    type: [null, 'string1', 'string2'],
+    description: 'test description',
+  })
+  t.equal(hardCodedOptionalTypes.usage, '--key <string1|string2>')
+  const hasHint = new Definition('key', {
+    default: 'test default',
+    type: String,
+    description: 'test description',
+    hint: '<testparam>',
+  })
+  t.equal(hasHint.usage, '--key <testparam>')
 })
 
 t.test('missing fields', async t => {