chore(refactor): promisify completion scripts

We also removed the "none" script because we handle a missing
script just fine. There is no need to put an empty one in

PR-URL: #2759
Credit: @wraithgar
Close: #2759
Reviewed-by: @nlf

Author: wraithgar

lib/access.js

@@ -59,17 +59,17 @@ const access = async ([cmd, ...args], cb) => {
   return fn(args, { ...npm.flatOptions })
 }
 
-const completion = function (opts, cb) {
-  var argv = opts.conf.argv.remain
+const completion = async (opts) => {
+  const argv = opts.conf.argv.remain
   if (argv.length === 2)
-    return cb(null, subcommands)
+    return subcommands
 
   switch (argv[2]) {
     case 'grant':
       if (argv.length === 3)
-        return cb(null, ['read-only', 'read-write'])
+        return ['read-only', 'read-write']
       else
-        return cb(null, [])
+        return []
 
     case 'public':
     case 'restricted':
@@ -79,9 +79,9 @@ const completion = function (opts, cb) {
     case '2fa-required':
     case '2fa-not-required':
     case 'revoke':
-      return cb(null, [])
+      return []
     default:
-      return cb(new Error(argv[2] + ' not recognized'))
+      throw new Error(argv[2] + ' not recognized')
   }
 }
 

lib/adduser.js

@@ -15,8 +15,6 @@ const usage = usageUtil(
   'npm adduser [--registry=url] [--scope=@orgname] [--always-auth]'
 )
 
-const completion = require('./utils/completion/none.js')
-
 const cmd = (args, cb) => adduser(args).then(() => cb()).catch(cb)
 
 const getRegistry = ({ scope, registry }) => {
@@ -74,4 +72,4 @@ const adduser = async (args) => {
   output(message)
 }
 
-module.exports = Object.assign(cmd, { completion, usage })
+module.exports = Object.assign(cmd, { usage })

lib/audit.js

@@ -38,17 +38,17 @@ const usage = usageUtil(
   '[--force|--package-lock-only|--dry-run|--production|--only=(dev|prod)]'
 )
 
-const completion = (opts, cb) => {
+const completion = async (opts) => {
   const argv = opts.conf.argv.remain
 
   if (argv.length === 2)
-    return cb(null, ['fix'])
+    return ['fix']
 
   switch (argv[2]) {
     case 'fix':
-      return cb(null, [])
+      return []
     default:
-      return cb(new Error(argv[2] + ' not recognized'))
+      throw new Error(argv[2] + ' not recognized')
   }
 }
 

lib/bin.js

@@ -1,7 +1,6 @@
 const npm = require('./npm.js')
 const output = require('./utils/output.js')
 const usageUtil = require('./utils/usage.js')
-const completion = require('./utils/completion/none.js')
 const PATH = require('./utils/path.js')
 const cmd = (args, cb) => bin(args).then(() => cb()).catch(cb)
 const usage = usageUtil('bin', 'npm bin [-g]')
@@ -11,4 +10,4 @@ const bin = async (args, cb) => {
   if (npm.flatOptions.global && !PATH.includes(b))
     console.error('(not in PATH env variable)')
 }
-module.exports = Object.assign(cmd, { usage, completion })
+module.exports = Object.assign(cmd, { usage })

lib/bugs.js

@@ -7,7 +7,6 @@ const npm = require('./npm.js')
 const hostedFromMani = require('./utils/hosted-git-info-from-manifest.js')
 
 const usage = usageUtil('bugs', 'npm bugs [<pkgname>]')
-const completion = require('./utils/completion/none.js')
 
 const cmd = (args, cb) => bugs(args).then(() => cb()).catch(cb)
 
@@ -44,4 +43,4 @@ const getBugs = async pkg => {
   await openUrl(url, `${mani.name} bug list available at the following URL`)
 }
 
-module.exports = Object.assign(cmd, { usage, completion })
+module.exports = Object.assign(cmd, { usage })

lib/cache.js

@@ -19,17 +19,17 @@ const usage = usageUtil('cache',
   '\nnpm cache verify'
 )
 
-const completion = (opts, cb) => {
+const completion = async (opts) => {
   const argv = opts.conf.argv.remain
   if (argv.length === 2)
-    return cb(null, ['add', 'clean', 'verify'])
+    return ['add', 'clean', 'verify']
 
   // TODO - eventually...
   switch (argv[2]) {
     case 'verify':
     case 'clean':
     case 'add':
-      return cb(null, [])
+      return []
   }
 }
 

lib/ci.js

@@ -11,7 +11,6 @@ const npm = require('./npm.js')
 const usageUtil = require('./utils/usage.js')
 
 const usage = usageUtil('ci', 'npm ci')
-const completion = require('./utils/completion/none.js')
 
 const cmd = (args, cb) => ci().then(() => cb()).catch(cb)
 
@@ -76,4 +75,4 @@ const ci = async () => {
   await reifyFinish(arb)
 }
 
-module.exports = Object.assign(cmd, { completion, usage })
+module.exports = Object.assign(cmd, {usage})

lib/completion.js

@@ -28,8 +28,6 @@
 // one per line for the shell completion method to consume in IFS=$'\n' mode
 // as an array.
 //
-// TODO: make all the implementation completion methods promise-returning
-// instead of callback-taking.
 
 const npm = require('./npm.js')
 const { types, shorthands } = require('./utils/config.js')
@@ -52,9 +50,9 @@ const { promisify } = require('util')
 const cmd = (args, cb) => compl(args).then(() => cb()).catch(cb)
 
 // completion for the completion command
-const completion = async (opts, cb) => {
+const completion = async (opts) => {
   if (opts.w > 2)
-    return cb()
+    return
 
   const { resolve } = require('path')
   const [bashExists, zshExists] = await Promise.all([
@@ -68,7 +66,7 @@ const completion = async (opts, cb) => {
   if (bashExists)
     out.push(['>>', '~/.bashrc'])
 
-  cb(null, out)
+  return out
 }
 
 const compl = async args => {
@@ -121,18 +119,16 @@ const compl = async args => {
     raw: args,
   }
 
-  const wrap = getWrap(opts)
-
   if (partialWords.slice(0, -1).indexOf('--') === -1) {
     if (word.charAt(0) === '-')
-      return wrap(configCompl(opts))
+      return wrap(opts, configCompl(opts))
 
     if (words[w - 1] &&
         words[w - 1].charAt(0) === '-' &&
         !isFlag(words[w - 1])) {
       // awaiting a value for a non-bool config.
       // don't even try to do this for now
-      return wrap(configValueCompl(opts))
+      return wrap(opts, configValueCompl(opts))
     }
   }
 
@@ -146,7 +142,7 @@ const compl = async args => {
   // check if there's a command already.
   const cmd = parsed.argv.remain[1]
   if (!cmd)
-    return wrap(cmdCompl(opts))
+    return wrap(opts, cmdCompl(opts))
 
   Object.keys(parsed).forEach(k => npm.config.set(k, parsed[k]))
 
@@ -155,10 +151,8 @@ const compl = async args => {
   // otherwise, do nothing
   const impl = npm.commands[cmd]
   if (impl && impl.completion) {
-    // XXX promisify all the cmd.completion functions
-    return await new Promise((res, rej) => {
-      impl.completion(opts, (er, comps) => er ? rej(er) : res(wrap(comps)))
-    })
+    const comps = await impl.completion(opts)
+    return wrap(opts, comps)
   }
 }
 
@@ -215,7 +209,7 @@ const escape = w => !/\s+/.test(w) ? w
 // If any of the items are arrays, then join them with a space.
 // Ie, returning ['a', 'b c', ['d', 'e']] would allow it to expand
 // to: 'a', 'b c', or 'd' 'e'
-const getWrap = opts => compls => {
+const wrap = (opts, compls) => {
   if (!Array.isArray(compls))
     compls = compls ? [compls] : []
 

lib/config.js

@@ -26,7 +26,7 @@ const usage = usageUtil(
 
 const cmd = (args, cb) => config(args).then(() => cb()).catch(cb)
 
-const completion = (opts, cb) => {
+const completion = async (opts) => {
   const argv = opts.conf.argv.remain
   if (argv[1] !== 'config')
     argv.unshift('config')
@@ -36,27 +36,27 @@ const completion = (opts, cb) => {
     if (opts.partialWord !== 'l')
       cmds.push('list')
 
-    return cb(null, cmds)
+    return cmds
   }
 
   const action = argv[2]
   switch (action) {
     case 'set':
       // todo: complete with valid values, if possible.
       if (argv.length > 3)
-        return cb(null, [])
+        return []
 
       // fallthrough
       /* eslint no-fallthrough:0 */
     case 'get':
     case 'delete':
     case 'rm':
-      return cb(null, Object.keys(types))
+      return Object.keys(types)
     case 'edit':
     case 'list':
     case 'ls':
     default:
-      return cb(null, [])
+      return []
   }
 }
 

lib/dedupe.js

@@ -5,7 +5,6 @@ const usageUtil = require('./utils/usage.js')
 const reifyFinish = require('./utils/reify-finish.js')
 
 const usage = usageUtil('dedupe', 'npm dedupe')
-const completion = require('./utils/completion/none.js')
 
 const cmd = (args, cb) => dedupe(args).then(() => cb()).catch(cb)
 
@@ -27,4 +26,4 @@ const dedupe = async (args) => {
   await reifyFinish(arb)
 }
 
-module.exports = Object.assign(cmd, { usage, completion })
+module.exports = Object.assign(cmd, { usage })

lib/deprecate.js

@@ -17,19 +17,17 @@ const usage = usageUtil(
   'npm deprecate <pkg>[@<version>] <message>'
 )
 
-const completion = (opts, cb) => {
+const completion = async (opts) => {
   if (opts.conf.argv.remain.length > 1)
-    return cb(null, [])
+    return []
 
-  return getIdentity(npm.flatOptions).then((username) => {
-    return libaccess.lsPackages(username, npm.flatOptions).then((packages) => {
-      return Object.keys(packages)
-        .filter((name) => packages[name] === 'write' &&
-          (opts.conf.argv.remain.length === 0 ||
-            name.startsWith(opts.conf.argv.remain[0]))
-        )
-    })
-  }).then((list) => cb(null, list), (err) => cb(err))
+  const username = await getIdentity(npm.flatOptions)
+  const packages = await libaccess.lsPackages(username, npm.flatOptions)
+  return Object.keys(packages)
+    .filter((name) =>
+      packages[name] === 'write' &&
+      (opts.conf.argv.remain.length === 0 ||
+        name.startsWith(opts.conf.argv.remain[0])))
 }
 
 const cmd = (args, cb) =>

lib/diff.js

@@ -11,7 +11,6 @@ const pickManifest = require('npm-pick-manifest')
 const npm = require('./npm.js')
 const usageUtil = require('./utils/usage.js')
 const output = require('./utils/output.js')
-const completion = require('./utils/completion/none.js')
 const readLocalPkg = require('./utils/read-local-package.js')
 
 const usage = usageUtil(
@@ -263,4 +262,4 @@ const findVersionsByPackageName = async (specs) => {
   })
 }
 
-module.exports = Object.assign(cmd, { completion, usage })
+module.exports = Object.assign(cmd, { usage })

lib/dist-tag.js

@@ -16,14 +16,14 @@ const usage = usageUtil(
   '\nnpm dist-tag ls [<pkg>]'
 )
 
-const completion = function (opts, cb) {
+const completion = async (opts) => {
   const argv = opts.conf.argv.remain
   if (argv.length === 2)
-    return cb(null, ['add', 'rm', 'ls'])
+    return ['add', 'rm', 'ls']
 
   switch (argv[2]) {
     default:
-      return cb()
+      return []
   }
 }
 

lib/docs.js

@@ -7,7 +7,6 @@ const npm = require('./npm.js')
 const hostedFromMani = require('./utils/hosted-git-info-from-manifest.js')
 
 const usage = usageUtil('docs', 'npm docs [<pkgname> [<pkgname> ...]]')
-const completion = require('./utils/completion/none.js')
 
 const cmd = (args, cb) => docs(args).then(() => cb()).catch(cb)
 
@@ -37,4 +36,4 @@ const getDocs = async pkg => {
   await openUrl(url, `${mani.name} docs available at the following URL`)
 }
 
-module.exports = Object.assign(cmd, { usage, completion })
+module.exports = Object.assign(cmd, { usage })

lib/doctor.js

@@ -4,7 +4,6 @@ const chalk = require('chalk')
 const ansiTrim = require('./utils/ansi-trim.js')
 const table = require('text-table')
 const output = require('./utils/output.js')
-const completion = require('./utils/completion/none.js')
 const usageUtil = require('./utils/usage.js')
 const usage = usageUtil('doctor', 'npm doctor')
 const { resolve } = require('path')
@@ -285,4 +284,4 @@ const doctor = async args => {
     throw 'Some problems found. See above for recommendations.'
 }
 
-module.exports = Object.assign(cmd, { completion, usage })
+module.exports = Object.assign(cmd, { usage })

lib/exec.js

@@ -21,8 +21,6 @@ const usage = usageUtil('exec',
   '-c <cmd> --call=<cmd> (may not be mixed with positional arguments)'
 )
 
-const completion = require('./utils/completion/installed-shallow.js')
-
 const { promisify } = require('util')
 const read = promisify(require('read'))
 
@@ -284,4 +282,4 @@ const getHash = packages =>
     .digest('hex')
     .slice(0, 16)
 
-module.exports = Object.assign(cmd, { completion, usage })
+module.exports = Object.assign(cmd, { usage })

lib/find-dupes.js

@@ -3,7 +3,6 @@ const dedupe = require('./dedupe.js')
 const usageUtil = require('./utils/usage.js')
 
 const usage = usageUtil('find-dupes', 'npm find-dupes')
-const completion = require('./utils/completion/none.js')
 const cmd = (args, cb) => dedupe({ dryRun: true }, cb)
 
-module.exports = Object.assign(cmd, { usage, completion })
+module.exports = Object.assign(cmd, { usage })

lib/get.js

@@ -1,12 +1,13 @@
 const npm = require('./npm.js')
+const config = require('./config.js')
 const usageUtil = require('./utils/usage.js')
 
 const usage = usageUtil(
   'get',
   'npm get [<key> ...] (See `npm config`)'
 )
 
-const completion = npm.commands.config.completion
+const completion = config.completion
 
 const cmd = (args, cb) =>
   npm.commands.config(['get'].concat(args), cb)