From df8c5abd92b047f5526aed513b5d37e0154138cb Mon Sep 17 00:00:00 2001
From: Luke Karrys <luke@lukekarrys.com>
Date: Wed, 3 Nov 2021 14:53:40 -0700
Subject: [PATCH] fix: make prefixed usage errors more consistent

---
 lib/base-command.js        | 12 ++++--------
 lib/commands/cache.js      |  9 ++-------
 lib/commands/diff.js       |  8 ++++----
 test/lib/commands/cache.js | 13 ++++++++-----
 4 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/lib/base-command.js b/lib/base-command.js
index 011429f6acdb7..90462346a3220 100644
--- a/lib/base-command.js
+++ b/lib/base-command.js
@@ -53,14 +53,10 @@ class BaseCommand {
     return results
   }
 
-  usageError (msg) {
-    if (!msg) {
-      return Object.assign(new Error(`\nUsage: ${this.usage}`), {
-        code: 'EUSAGE',
-      })
-    }
-
-    return Object.assign(new Error(`\nUsage: ${msg}\n\n${this.usage}`), {
+  usageError (prefix = '') {
+    if (prefix)
+      prefix += '\n\n'
+    return Object.assign(new Error(`\nUsage: ${prefix}${this.usage}`), {
       code: 'EUSAGE',
     })
   }
diff --git a/lib/commands/cache.js b/lib/commands/cache.js
index 43f52e4e95e68..b4a932d1bec03 100644
--- a/lib/commands/cache.js
+++ b/lib/commands/cache.js
@@ -116,7 +116,7 @@ class Cache extends BaseCommand {
       case 'ls':
         return await this.ls(args)
       default:
-        throw Object.assign(new Error(this.usage), { code: 'EUSAGE' })
+        throw this.usageError()
     }
   }
 
@@ -161,14 +161,9 @@ class Cache extends BaseCommand {
   // npm cache add <tarball>...
   // npm cache add <folder>...
   async add (args) {
-    const usage = 'Usage:\n' +
-      '    npm cache add <tarball-url>...\n' +
-      '    npm cache add <pkg>@<ver>...\n' +
-      '    npm cache add <tarball>...\n' +
-      '    npm cache add <folder>...\n'
     log.silly('cache add', 'args', args)
     if (args.length === 0)
-      throw Object.assign(new Error(usage), { code: 'EUSAGE' })
+      throw this.usageError('First argument to `add` is required')
 
     return Promise.all(args.map(spec => {
       log.silly('cache add', 'spec', spec)
diff --git a/lib/commands/diff.js b/lib/commands/diff.js
index da9b9f5d2c655..9eccb66280384 100644
--- a/lib/commands/diff.js
+++ b/lib/commands/diff.js
@@ -101,7 +101,7 @@ class Diff extends BaseCommand {
     }
 
     if (!name)
-      throw this.usageError('Needs multiple arguments to compare or run from a project dir.\n')
+      throw this.usageError('Needs multiple arguments to compare or run from a project dir.')
 
     return name
   }
@@ -133,7 +133,7 @@ class Diff extends BaseCommand {
       noPackageJson = true
     }
 
-    const missingPackageJson = this.usageError('Needs multiple arguments to compare or run from a project dir.\n')
+    const missingPackageJson = this.usageError('Needs multiple arguments to compare or run from a project dir.')
 
     // using a valid semver range, that means it should just diff
     // the cwd against a published version to the registry using the
@@ -222,7 +222,7 @@ class Diff extends BaseCommand {
         `file:${this.prefix}`,
       ]
     } else
-      throw this.usageError(`Spec type ${spec.type} not supported.\n`)
+      throw this.usageError(`Spec type ${spec.type} not supported.`)
   }
 
   async convertVersionsToSpecs ([a, b]) {
@@ -239,7 +239,7 @@ class Diff extends BaseCommand {
       }
 
       if (!pkgName)
-        throw this.usageError('Needs to be run from a project dir in order to diff two versions.\n')
+        throw this.usageError('Needs to be run from a project dir in order to diff two versions.')
 
       return [`${pkgName}@${a}`, `${pkgName}@${b}`]
     }
diff --git a/test/lib/commands/cache.js b/test/lib/commands/cache.js
index c12318f4e579b..b533ace648bff 100644
--- a/test/lib/commands/cache.js
+++ b/test/lib/commands/cache.js
@@ -3,8 +3,6 @@ const { fake: mockNpm } = require('../../fixtures/mock-npm.js')
 const path = require('path')
 const npa = require('npm-package-arg')
 
-const usageUtil = () => 'usage instructions'
-
 let outputOutput = []
 
 let rimrafPath = ''
@@ -140,7 +138,6 @@ const Cache = t.mock('../../../lib/commands/cache.js', {
   npmlog,
   pacote,
   rimraf,
-  '../../../lib/utils/usage.js': usageUtil,
 })
 
 const npm = mockNpm({
@@ -161,7 +158,10 @@ const cache = new Cache(npm)
 t.test('cache no args', async t => {
   await t.rejects(
     cache.exec([]),
-    'usage instructions',
+    {
+      code: 'EUSAGE',
+      message: /^Usage: npm cache$/m,
+    },
     'should throw usage instructions'
   )
 })
@@ -194,7 +194,10 @@ t.test('cache add no arg', async t => {
 
   await t.rejects(
     cache.exec(['add']),
-    { code: 'EUSAGE' },
+    {
+      code: 'EUSAGE',
+      message: /^Usage: First argument to `add` is required$/m,
+    },
     'throws usage error'
   )
   t.strictSame(logOutput, [