fix(usage): clean up usage declarations

Small refactor of commands to allow usage to be more programmatically generated, leading us in the direction of more tighly coupling each command to the params it accepts. PR-URL: #2821 Credit: @wraithgar Close: #2821 Reviewed-by: @isaacs
npm · Mar 9, 2021 · 9fe0df5 · 9fe0df5
1 parent 85a8694
commit 9fe0df5
Show file tree

Hide file tree

Showing 84 changed files with 1,023 additions and 760 deletions.
diff --git a/lib/access.js b/lib/access.js
@@ -4,8 +4,8 @@ const libaccess = require('libnpmaccess')
 const readPackageJson = require('read-package-json-fast')
 
 const otplease = require('./utils/otplease.js')
-const usageUtil = require('./utils/usage.js')
 const getIdentity = require('./utils/get-identity.js')
+const BaseCommand = require('./base-command.js')
 
 const subcommands = [
   'public',
@@ -19,24 +19,23 @@ const subcommands = [
   '2fa-not-required',
 ]
 
-class Access {
-  constructor (npm) {
-    this.npm = npm
+class Access extends BaseCommand {
+  static get name () {
+    return 'access'
   }
 
-  get usage () {
-    return usageUtil(
-      'access',
-      'npm access public [<package>]\n' +
-      'npm access restricted [<package>]\n' +
-      'npm access grant <read-only|read-write> <scope:team> [<package>]\n' +
-      'npm access revoke <scope:team> [<package>]\n' +
-      'npm access 2fa-required [<package>]\n' +
-      'npm access 2fa-not-required [<package>]\n' +
-      'npm access ls-packages [<user>|<scope>|<scope:team>]\n' +
-      'npm access ls-collaborators [<package> [<user>]]\n' +
-      'npm access edit [<package>]'
-    )
+  static get usage () {
+    return [
+      'public [<package>]',
+      'restricted [<package>]',
+      'grant <read-only|read-write> <scope:team> [<package>]',
+      'revoke <scope:team> [<package>]',
+      '2fa-required [<package>]',
+      '2fa-not-required [<package>]',
+      'ls-packages [<user>|<scope>|<scope:team>]',
+      'ls-collaborators [<package> [<user>]]',
+      'edit [<package>]',
+    ]
   }
 
   async completion (opts) {
@@ -66,12 +65,7 @@ class Access {
   }
 
   exec (args, cb) {
-    this.access(args)
-      .then(x => cb(null, x))
-      .catch(err => err.code === 'EUSAGE'
-        ? cb(err.message)
-        : cb(err)
-      )
+    this.access(args).then(() => cb()).catch(cb)
   }
 
   async access ([cmd, ...args]) {
@@ -202,12 +196,6 @@ class Access {
         return name
     }
   }
-
-  usageError (msg) {
-    return Object.assign(new Error(`\nUsage: ${msg}\n\n` + this.usage), {
-      code: 'EUSAGE',
-    })
-  }
 }
 
 module.exports = Access
diff --git a/lib/adduser.js b/lib/adduser.js
@@ -1,24 +1,20 @@
 const log = require('npmlog')
-const usageUtil = require('./utils/usage.js')
 const replaceInfo = require('./utils/replace-info.js')
+const BaseCommand = require('./base-command.js')
 const authTypes = {
   legacy: require('./auth/legacy.js'),
   oauth: require('./auth/oauth.js'),
   saml: require('./auth/saml.js'),
   sso: require('./auth/sso.js'),
 }
 
-class AddUser {
-  constructor (npm) {
-    this.npm = npm
+class AddUser extends BaseCommand {
+  static get name () {
+    return 'adduser'
   }
 
-  /* istanbul ignore next - see test/lib/load-all-commands.js */
-  get usage () {
-    return usageUtil(
-      'adduser',
-      'npm adduser [--registry=url] [--scope=@orgname] [--always-auth]'
-    )
+  static get usage () {
+    return ['[--registry=url] [--scope=@orgname] [--always-auth]']
   }
 
   exec (args, cb) {

diff --git a/lib/audit.js b/lib/audit.js
@@ -2,21 +2,20 @@ const Arborist = require('@npmcli/arborist')
 const auditReport = require('npm-audit-report')
 const reifyFinish = require('./utils/reify-finish.js')
 const auditError = require('./utils/audit-error.js')
-const usageUtil = require('./utils/usage.js')
+const BaseCommand = require('./base-command.js')
 
-class Audit {
-  constructor (npm) {
-    this.npm = npm
+class Audit extends BaseCommand {
+  /* istanbul ignore next - see test/lib/load-all-commands.js */
+  static get name () {
+    return 'audit'
   }
 
   /* istanbul ignore next - see test/lib/load-all-commands.js */
-  get usage () {
-    return usageUtil(
-      'audit',
-      'npm audit [--json] [--production]' +
-      '\nnpm audit fix ' +
-      '[--force|--package-lock-only|--dry-run|--production|--only=(dev|prod)]'
-    )
+  static get usage () {
+    return [
+      '[--json] [--production]',
+      'fix [--force|--package-lock-only|--dry-run|--production|--only=(dev|prod)]',
+    ]
   }
 
   async completion (opts) {

diff --git a/lib/base-command.js b/lib/base-command.js
@@ -0,0 +1,38 @@
+// Base class for npm.commands[cmd]
+const usageUtil = require('./utils/usage.js')
+
+class BaseCommand {
+  constructor (npm) {
+    this.npm = npm
+  }
+
+  get usage () {
+    let usage = `npm ${this.constructor.name}\n\n`
+    if (this.constructor.description)
+      usage = `${usage}${this.constructor.description}\n\n`
+
+    usage = `${usage}Usage:\n`
+    if (!this.constructor.usage)
+      usage = `${usage}npm ${this.constructor.name}`
+    else
+      usage = `${usage}${this.constructor.usage.map(u => `npm ${this.constructor.name} ${u}`).join('\n')}`
+
+    // Mostly this just appends aliases, this could be more clear
+    usage = usageUtil(this.constructor.name, usage)
+    usage = `${usage}\n\nRun "npm ${this.constructor.name} help" for more info`
+    return usage
+  }
+
+  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}`), {
+      code: 'EUSAGE',
+    })
+  }
+}
+module.exports = BaseCommand
diff --git a/lib/bin.js b/lib/bin.js
@@ -1,14 +1,13 @@
 const envPath = require('./utils/path.js')
-const usageUtil = require('./utils/usage.js')
+const BaseCommand = require('./base-command.js')
 
-class Bin {
-  constructor (npm) {
-    this.npm = npm
+class Bin extends BaseCommand {
+  static get name () {
+    return 'bin'
   }
 
-  /* istanbul ignore next - see test/lib/load-all-commands.js */
-  get usage () {
-    return usageUtil('bin', 'npm bin [-g]')
+  static get usage () {
+    return ['[-g]']
   }
 
   exec (args, cb) {

diff --git a/lib/bugs.js b/lib/bugs.js
@@ -1,17 +1,16 @@
 const log = require('npmlog')
 const pacote = require('pacote')
 const openUrl = require('./utils/open-url.js')
-const usageUtil = require('./utils/usage.js')
 const hostedFromMani = require('./utils/hosted-git-info-from-manifest.js')
+const BaseCommand = require('./base-command.js')
 
-class Bugs {
-  constructor (npm) {
-    this.npm = npm
+class Bugs extends BaseCommand {
+  static get name () {
+    return 'bugs'
   }
 
-  /* istanbul ignore next - see test/lib/load-all-commands.js */
-  get usage () {
-    return usageUtil('bugs', 'npm bugs [<pkgname>]')
+  static get usage () {
+    return ['[<pkgname>]']
   }
 
   exec (args, cb) {

diff --git a/lib/cache.js b/lib/cache.js
@@ -4,23 +4,25 @@ const log = require('npmlog')
 const pacote = require('pacote')
 const path = require('path')
 const rimraf = promisify(require('rimraf'))
+const BaseCommand = require('./base-command.js')
 
-const usageUtil = require('./utils/usage.js')
-class Cache {
-  constructor (npm) {
-    this.npm = npm
+class Cache extends BaseCommand {
+  /* istanbul ignore next - see test/lib/load-all-commands.js */
+  static get name () {
+    return 'cache'
   }
 
-  get usage () {
-    return usageUtil('cache',
-      'npm cache add <tarball file>' +
-      '\nnpm cache add <folder>' +
-      '\nnpm cache add <tarball url>' +
-      '\nnpm cache add <git url>' +
-      '\nnpm cache add <name>@<version>' +
-      '\nnpm cache clean' +
-      '\nnpm cache verify'
-    )
+  /* istanbul ignore next - see test/lib/load-all-commands.js */
+  static get usage () {
+    return [
+      'add <tarball file>',
+      'add <folder>',
+      'add <tarball url>',
+      'add <git url>',
+      'add <name>@<version>',
+      'clean',
+      'verify',
+    ]
   }
 
   async completion (opts) {

diff --git a/lib/ci.js b/lib/ci.js
@@ -7,7 +7,6 @@ const fs = require('fs')
 const readdir = util.promisify(fs.readdir)
 
 const log = require('npmlog')
-const usageUtil = require('./utils/usage.js')
 
 const removeNodeModules = async where => {
   const rimrafOpts = { glob: false }
@@ -18,15 +17,12 @@ const removeNodeModules = async where => {
   await Promise.all(entries.map(f => rimraf(`${path}/${f}`, rimrafOpts)))
   process.emit('timeEnd', 'npm-ci:rm')
 }
+const BaseCommand = require('./base-command.js')
 
-class CI {
-  constructor (npm) {
-    this.npm = npm
-  }
-
+class CI extends BaseCommand {
   /* istanbul ignore next - see test/lib/load-all-commands.js */
-  get usage () {
-    return usageUtil('ci', 'npm ci')
+  static get name () {
+    return 'ci'
   }
 
   exec (args, cb) {

diff --git a/lib/completion.js b/lib/completion.js
@@ -41,17 +41,18 @@ const allConfs = configNames.concat(shorthandNames)
 const isWindowsShell = require('./utils/is-windows-shell.js')
 const fileExists = require('./utils/file-exists.js')
 
-const usageUtil = require('./utils/usage.js')
 const { promisify } = require('util')
+const BaseCommand = require('./base-command.js')
 
-class Completion {
-  constructor (npm) {
-    this.npm = npm
+class Completion extends BaseCommand {
+  /* istanbul ignore next - see test/lib/load-all-commands.js */
+  static get name () {
+    return 'completion'
   }
 
   /* istanbul ignore next - see test/lib/load-all-commands.js */
-  get usage () {
-    return usageUtil('completion', 'source <(npm completion)')
+  static get description () {
+    return 'npm command completion script. save to ~/.bashrc or ~/.zshrc'
   }
 
   // completion for the completion command

diff --git a/lib/config.js b/lib/config.js
@@ -1,5 +1,4 @@
 const { defaults, types } = require('./utils/config.js')
-const usageUtil = require('./utils/usage.js')
 
 const mkdirp = require('mkdirp-infer-owner')
 const { dirname } = require('path')
@@ -28,22 +27,22 @@ const keyValues = args => {
 
 const publicVar = k => !/^(\/\/[^:]+:)?_/.test(k)
 
-class Config {
-  constructor (npm) {
-    this.npm = npm
+const BaseCommand = require('./base-command.js')
+class Config extends BaseCommand {
+  /* istanbul ignore next - see test/lib/load-all-commands.js */
+  static get name () {
+    return 'config'
   }
 
-  get usage () {
-    return usageUtil(
-      'config',
-      'npm config set <key>=<value> [<key>=<value> ...]' +
-      '\nnpm config get [<key> [<key> ...]]' +
-      '\nnpm config delete <key> [<key> ...]' +
-      '\nnpm config list [--json]' +
-      '\nnpm config edit' +
-      '\nnpm set <key>=<value> [<key>=<value> ...]' +
-      '\nnpm get [<key> [<key> ...]]'
-    )
+  /* istanbul ignore next - see test/lib/load-all-commands.js */
+  static get usage () {
+    return [
+      'set <key>=<value> [<key>=<value> ...]',
+      'get [<key> [<key> ...]]',
+      'delete <key> [<key> ...]',
+      'list [--json]',
+      'edit',
+    ]
   }
 
   async completion (opts) {
@@ -253,10 +252,6 @@ ${defData}
     }
     this.npm.output(JSON.stringify(publicConf, null, 2))
   }
-
-  usageError () {
-    return Object.assign(new Error(this.usage), { code: 'EUSAGE' })
-  }
 }
 
 module.exports = Config
diff --git a/lib/dedupe.js b/lib/dedupe.js
@@ -1,16 +1,13 @@
 // dedupe duplicated packages, or find them in the tree
 const Arborist = require('@npmcli/arborist')
-const usageUtil = require('./utils/usage.js')
 const reifyFinish = require('./utils/reify-finish.js')
 
-class Dedupe {
-  constructor (npm) {
-    this.npm = npm
-  }
+const BaseCommand = require('./base-command.js')
 
+class Dedupe extends BaseCommand {
   /* istanbul ignore next - see test/lib/load-all-commands.js */
-  get usage () {
-    return usageUtil('dedupe', 'npm dedupe')
+  static get name () {
+    return 'dedupe'
   }
 
   exec (args, cb) {