chore: refactor command

Usage is driven by actual code, commands are tagged for grouping

Author: wraithgar

docs/lib/content/commands/npm-doctor.md

@@ -31,8 +31,10 @@ Also, in addition to this, there are also very many issue reports due to
 using old versions of npm. Since npm is constantly improving, running
 `npm@latest` is better than an old version.
 
-`npm doctor` verifies the following items in your environment, and if there
-are any recommended changes, it will display them.
+`npm doctor` verifies the following items in your environment, and if
+there are any recommended changes, it will display them.  By default npm
+runs all of these checks. You can limit what checks are ran by
+specifying them as extra arguments.
 
 #### `npm ping`
 

lib/commands/doctor.js

@@ -34,30 +34,104 @@ const maskLabel = mask => {
   return label.join(', ')
 }
 
+const subcommands = [
+  {
+    groups: ['ping', 'registry'],
+    title: 'npm ping',
+    cmd: 'checkPing',
+  }, {
+    groups: ['versions'],
+    title: 'npm -v',
+    cmd: 'getLatestNpmVersion',
+  }, {
+    groups: ['versions'],
+    title: 'node -v',
+    cmd: 'getLatestNodejsVersion',
+  }, {
+    groups: ['registry'],
+    title: 'npm config get registry',
+    cmd: 'checkNpmRegistry',
+  }, {
+    groups: ['environment'],
+    title: 'git executable in PATH',
+    cmd: 'getGitPath',
+  }, {
+    groups: ['environment'],
+    title: 'global bin folder in PATH',
+    cmd: 'getBinPath',
+  }, {
+    groups: ['permissions', 'cache'],
+    title: 'Perms check on cached files',
+    cmd: 'checkCachePermission',
+    windows: false,
+  }, {
+    groups: ['permissions'],
+    title: 'Perms check on local node_modules',
+    cmd: 'checkLocalModulesPermission',
+    windows: false,
+  }, {
+    groups: ['permissions'],
+    title: 'Perms check on global node_modules',
+    cmd: 'checkGlobalModulesPermission',
+    windows: false,
+  }, {
+    groups: ['permissions'],
+    title: 'Perms check on local bin folder',
+    cmd: 'checkLocalBinPermission',
+    windows: false,
+  }, {
+    groups: ['permissions'],
+    title: 'Perms check on global bin folder',
+    cmd: 'checkGlobalBinPermission',
+    windows: false,
+  }, {
+    groups: ['cache'],
+    title: 'Verify cache contents',
+    cmd: 'verifyCachedFiles',
+    windows: false,
+  },
+  // TODO:
+  // group === 'dependencies'?
+  //   - ensure arborist.loadActual() runs without errors and no invalid edges
+  //   - ensure package-lock.json matches loadActual()
+  //   - verify loadActual without hidden lock file matches hidden lockfile
+  // group === '???'
+  //   - verify all local packages have bins linked
+  // What is the fix for these?
+]
 const BaseCommand = require('../base-command.js')
 class Doctor extends BaseCommand {
   static description = 'Check your npm environment'
   static name = 'doctor'
   static params = ['registry']
   static ignoreImplicitWorkspace = false
+  static usage = [`[${subcommands.flatMap(s => s.groups)
+    .filter((value, index, self) => self.indexOf(value) === index)
+    .join('] [')}]`]
+
+  static subcommands = subcommands
+
+  // minimum width of check column, enough for the word `Check`
+  #checkWidth = 5
 
   async exec (args) {
     log.info('Running checkup')
     let allOk = true
 
     const actions = this.actions(args)
-    this.checkWidth = actions.reduce((length, item) => Math.max(item[0].length, length), 5)
+    this.#checkWidth = actions.reduce((length, item) =>
+      Math.max(item.title.length, length), this.#checkWidth)
 
     if (!this.npm.silent) {
       this.output(['Check', 'Value', 'Recommendation/Notes'].map(h => this.npm.chalk.underline(h)))
     }
     // Do the actual work
-    for (const [msg, fn, args] of actions) {
-      const item = [msg]
+    for (const { title, cmd } of actions) {
+      const item = [title]
       try {
-        item.push(true, await this[fn](...args))
-      } catch (er) {
-        item.push(false, er)
+        item.push(true, await this[cmd]())
+      } catch (err) {
+        item.push(false, err)
       }
       if (!item[1]) {
         allOk = false
@@ -72,11 +146,13 @@ class Doctor extends BaseCommand {
       }
     }
 
-    if (!this.npm.silent) {
-      // this.npm.output(table(outTable, tableOpts))
-    }
     if (!allOk) {
-      throw new Error('Some problems found. See above for recommendations.')
+      if (this.npm.silent) {
+        /* eslint-disable-next-line max-len */
+        throw new Error('Some problems found. Check logs or disable silent mode for recommendations.')
+      } else {
+        throw new Error('Some problems found. See above for recommendations.')
+      }
     }
   }
 
@@ -144,15 +220,35 @@ class Doctor extends BaseCommand {
     }
   }
 
-  async checkBinPath (dir) {
-    const tracker = log.newItem('checkBinPath', 1)
-    tracker.info('checkBinPath', 'Finding npm global bin in your PATH')
+  async getBinPath (dir) {
+    const tracker = log.newItem('getBinPath', 1)
+    tracker.info('getBinPath', 'Finding npm global bin in your PATH')
     if (!process.env.PATH.includes(this.npm.globalBin)) {
       throw new Error(`Add ${this.npm.globalBin} to your $PATH`)
     }
     return this.npm.globalBin
   }
 
+  async checkCachePermission () {
+    return this.checkFilesPermission(this.npm.cache, true, R_OK)
+  }
+
+  async checkLocalModulesPermission () {
+    return this.checkFilesPermission(this.npm.localDir, true, R_OK | W_OK, true)
+  }
+
+  async checkGlobalModulesPermission () {
+    return this.checkFilesPermission(this.npm.globalDir, false, R_OK)
+  }
+
+  async checkLocalBinPermission () {
+    return this.checkFilesPermission(this.npm.localBin, false, R_OK | W_OK | X_OK, true)
+  }
+
+  async checkGlobalBinPermission () {
+    return this.checkFilesPermission(this.npm.globalBin, false, X_OK)
+  }
+
   async checkFilesPermission (root, shouldOwn, mask, missingOk) {
     let ok = true
 
@@ -293,79 +389,22 @@ class Doctor extends BaseCommand {
         'right-mid': '',
         middle: '  ' },
       style: { 'padding-left': 0, 'padding-right': 0 },
-      colWidths: [this.checkWidth, 6],
+      colWidths: [this.#checkWidth, 6],
     })
     t.push(row)
     this.npm.output(t.toString())
   }
 
-  actions (subcmds) {
-    const a = []
-    if (!subcmds.length || subcmds.includes('ping')) {
-      a.push(['npm ping', 'checkPing', []])
-    }
-    if (!subcmds.length || subcmds.includes('versions')) {
-      a.push(['npm -v', 'getLatestNpmVersion', []])
-      a.push(['node -v', 'getLatestNodejsVersion', []])
-    }
-    if (!subcmds.length || subcmds.includes('registry')) {
-      a.push(['npm config get registry', 'checkNpmRegistry', []])
-    }
-    if (!subcmds.length || subcmds.includes('git')) {
-      a.push(['which git', 'getGitPath', []])
-    }
-    if (!subcmds.length || subcmds.includes('permissions')) {
-      if (subcmds.includes('permissions') && process.platform === 'win32') {
-        log.warn('Ignoring permissions checks for windows')
-      } else if (process.platform !== 'win32') {
-        a.push([
-          'Perms check on cached files',
-          'checkFilesPermission',
-          [this.npm.cache, true, R_OK],
-        ])
-        a.push([
-          'Perms check on local node_modules',
-          'checkFilesPermission',
-          [this.npm.localDir, true, R_OK | W_OK, true],
-        ])
-        a.push([
-          'Perms check on global node_modules',
-          'checkFilesPermission',
-          [this.npm.globalDir, false, R_OK],
-        ])
-        a.push([
-          'Perms check on local bin folder',
-          'checkFilesPermission',
-          [this.npm.localBin, false, R_OK | W_OK | X_OK, true],
-        ])
-        a.push([
-          'Perms check on global bin folder',
-          'checkFilesPermission',
-          [this.npm.globalBin, false, X_OK],
-        ])
+  actions (params) {
+    return this.constructor.subcommands.filter(subcmd => {
+      if (process.platform === 'win32' && subcmd.windows === false) {
+        return false
       }
-    }
-    if (!subcmds.length || subcmds.includes('cache')) {
-      a.push([
-        'Verify cache contents',
-        'verifyCachedFiles',
-        [this.npm.flatOptions.cache],
-      ])
-    }
-
-    if (!subcmds.length || subcmds.includes('environment')) {
-      a.push(['Global bin folder in PATH', 'checkBinPath', []])
-    }
-
-    // TODO:
-    // subcmd === 'dependencies'?
-    //   - ensure arborist.loadActual() runs without errors and no invalid edges
-    //   - ensure package-lock.json matches loadActual()
-    //   - verify loadActual without hidden lock file matches hidden lockfile
-    // subcmd === '???'
-    //   - verify all local packages have bins linked
-    // What is the fix for these?
-    return a
+      if (params.length) {
+        return params.some(param => subcmd.groups.includes(param))
+      }
+      return true
+    })
   }
 }