Merge pull request #1622 from alphagov/parse-cli-args-better

Better cli argument parsing
alphagov · Sep 27, 2022 · 8de4e2a · 8de4e2a
2 parents 1f1802a + aad22f5
commit 8de4e2a
Show file tree

Hide file tree

Showing 4 changed files with 233 additions and 21 deletions.
diff --git a/bin/cli b/bin/cli
@@ -6,12 +6,12 @@ const path = require('path')
 
 const { exec } = require('../lib/exec')
 const { runUpgrade } = require('../lib/upgradeToV13')
+const { parse } = require('./utils/argvParser')
 
 const currentDirectory = process.cwd()
 const kitRoot = path.join(__dirname, '..')
 
-const additionalOptions = [].concat(...process.argv).splice(2)
-const command = additionalOptions.shift()
+const argv = parse(process.argv)
 
 const npmrc = `
 audit=false
@@ -55,28 +55,24 @@ ${prog} start`
 }
 
 const getInstallLocation = () => {
-  const lastOption = ('' + additionalOptions[additionalOptions.length - 2]).startsWith('-') ? undefined : additionalOptions[additionalOptions.length - 1]
-  if (lastOption) {
-    if (lastOption.startsWith('/')) {
-      return lastOption
+  const chosenPath = argv.paths[0]
+  if (chosenPath) {
+    if (chosenPath.startsWith('/')) {
+      return chosenPath
     }
-    return path.join(currentDirectory, lastOption)
+    return path.join(currentDirectory, chosenPath)
   }
   return currentDirectory
 }
 
 const getChosenKitDependency = () => {
   const defaultValue = 'govuk-prototype-kit'
-  let versionFlagIndex = additionalOptions.indexOf('--version')
-  if (versionFlagIndex === -1) {
-    versionFlagIndex = additionalOptions.indexOf('-v')
-  }
-  if (versionFlagIndex === -1) {
+  const versionRequested = argv.options.version || argv.options.v
+
+  if (!versionRequested) {
     return defaultValue
   }
 
-  const versionRequested = additionalOptions[versionFlagIndex + 1]
-
   if (versionRequested === 'local') {
     return kitRoot
   } else if (versionRequested) {
@@ -93,7 +89,7 @@ const getChosenKitDependency = () => {
 };
 
 (async () => {
-  if (command === 'create') {
+  if (argv.command === 'create') {
     // Install as a two-stage bootstrap process.
     //
     // In stage one (`create`) we create an empty project folder and install
@@ -135,17 +131,17 @@ const getChosenKitDependency = () => {
       stdio: 'inherit',
       cwd: installDirectory
     }, console.log)
-  } else if (command === 'init') {
+  } else if (argv.command === 'init') {
     // `init` is stage two of the install process (see above), it should be
     // called by `create` with the correct arguments.
 
-    if (additionalOptions[0] !== '--') {
+    if (process.argv[3] !== '--') {
       usage()
       process.exitCode = 2
       return
     }
 
-    const installDirectory = additionalOptions[1]
+    const installDirectory = process.argv[4]
 
     const copyFile = (fileName) => fs.copy(path.join(kitRoot, fileName), path.join(installDirectory, fileName))
     const updatePackageJson = async (packageJsonPath) => {
@@ -161,9 +157,9 @@ const getChosenKitDependency = () => {
       copyFile('LICENCE.txt'),
       updatePackageJson(path.join(installDirectory, 'package.json'))
     ])
-  } else if (command === 'start') {
+  } else if (argv.command === 'start') {
     require('../start')
-  } else if (command === 'upgrade') {
+  } else if (argv.command === 'upgrade') {
     await runUpgrade()
   } else {
     usage()

diff --git a/bin/utils/argvParser.js b/bin/utils/argvParser.js
@@ -0,0 +1,48 @@
+module.exports = {
+  parse: (argvInput) => {
+    const args = [...argvInput].splice(2)
+    const options = {}
+    const paths = []
+    let command
+    let contextFromPrevious
+
+    const processOptionName = (unprocessed) => {
+      if (unprocessed.startsWith('--')) {
+        return unprocessed.substring(2)
+      }
+      if (unprocessed.startsWith('-')) {
+        return unprocessed.substring(1)
+      }
+    }
+
+    args.forEach(arg => {
+      if (arg.startsWith('-')) {
+        if (arg.includes('=')) {
+          const parts = arg.split('=')
+          options[processOptionName(parts[0])] = parts[1]
+          return
+        }
+        contextFromPrevious = {
+          option: arg
+        }
+        return
+      }
+      if (contextFromPrevious && contextFromPrevious.option) {
+        options[processOptionName(contextFromPrevious.option)] = arg
+        contextFromPrevious = undefined
+        return
+      }
+      if (command) {
+        paths.push(arg)
+      } else {
+        command = arg
+      }
+    })
+
+    return {
+      command,
+      options,
+      paths
+    }
+  }
+}
diff --git a/bin/utils/argvParser.spec.js b/bin/utils/argvParser.spec.js
@@ -0,0 +1,168 @@
+const parser = require('./argvParser')
+
+const addStandardArgs = arr => [
+  '/Users/user.name/.nvm/versions/node/node.version/bin/node',
+  '/Users/user.name/.nvm/versions/node/node.version/bin/govuk-prototype-kit',
+  ...arr
+]
+
+describe('argv parser', () => {
+  it('should parse a basic command', () => {
+    const result = parser.parse(addStandardArgs(['hello']))
+
+    expect(result).toEqual({
+      command: 'hello',
+      options: {},
+      paths: []
+    })
+  })
+  it('should parse a different basic command', () => {
+    const result = parser.parse(addStandardArgs(['goodbye']))
+
+    expect(result).toEqual({
+      command: 'goodbye',
+      options: {},
+      paths: []
+    })
+  })
+  it('should parse an option with double-hyphen', () => {
+    const result = parser.parse(addStandardArgs([
+      '--name',
+      'Alex',
+      'hello'
+    ]))
+
+    expect(result).toEqual({
+      command: 'hello',
+      options: {
+        name: 'Alex'
+      },
+      paths: []
+    })
+  })
+  it('should handle multiple parameters with double-hyphens', () => {
+    const result = parser.parse(addStandardArgs([
+      '--name',
+      'Alex',
+      '--pronouns',
+      'they/them',
+      'hello'
+    ]))
+
+    expect(result).toEqual({
+      command: 'hello',
+      options: {
+        name: 'Alex',
+        pronouns: 'they/them'
+      },
+      paths: []
+    })
+  })
+  it('should handle paths after the command', () => {
+    const result = parser.parse(addStandardArgs([
+      'create',
+      '/tmp/abc'
+    ]))
+
+    expect(result).toEqual({
+      command: 'create',
+      options: {},
+      paths: ['/tmp/abc']
+    })
+  })
+  it('should support the longest command we currently have', () => {
+    const result = parser.parse(addStandardArgs([
+      '--version',
+      'local',
+      'create',
+      '/tmp/hello-world'
+    ]))
+
+    expect(result).toEqual({
+      command: 'create',
+      options: {
+        version: 'local'
+      },
+      paths: ['/tmp/hello-world']
+    })
+  })
+  it('should support options between the command and path(s)', () => {
+    const result = parser.parse(addStandardArgs([
+      'create',
+      '--version',
+      'local',
+      '/tmp/hello-world'
+    ]))
+
+    expect(result).toEqual({
+      command: 'create',
+      options: {
+        version: 'local'
+      },
+      paths: ['/tmp/hello-world']
+    })
+  })
+  it('should support single-hyphen options', () => {
+    const result = parser.parse(addStandardArgs([
+      'create',
+      '-v',
+      'local',
+      '/tmp/hello-world'
+    ]))
+
+    expect(result).toEqual({
+      command: 'create',
+      options: {
+        v: 'local'
+      },
+      paths: ['/tmp/hello-world']
+    })
+  })
+  it('should support options after path(s)', () => {
+    const result = parser.parse(addStandardArgs([
+      'create',
+      '/tmp/hello-world',
+      '--version',
+      'local'
+    ]))
+
+    expect(result).toEqual({
+      command: 'create',
+      options: {
+        version: 'local'
+      },
+      paths: ['/tmp/hello-world']
+    })
+  })
+  it('should support semvar numbers as values', () => {
+    const result = parser.parse(addStandardArgs([
+      '--version',
+      '13.0.1',
+      'create',
+      '/tmp/hello-world'
+    ]))
+
+    expect(result).toEqual({
+      command: 'create',
+      options: {
+        version: '13.0.1'
+      },
+      paths: ['/tmp/hello-world']
+    })
+  })
+  it('should support equals to set an option', () => {
+    const result = parser.parse(addStandardArgs([
+      '--version=local',
+      'create',
+      '/tmp/hello-world'
+    ]))
+
+    expect(result).toEqual({
+      command: 'create',
+      options: {
+        version: 'local'
+      },
+      paths: ['/tmp/hello-world']
+    })
+  })
+})
diff --git a/package.json b/package.json
@@ -34,7 +34,7 @@
     "test:acceptance": "KIT_TEST_DIR=tmp/test-prototype-package start-server-and-test 'node cypress/scripts/run-starter-prototype' 3000 'cypress run'",
     "test:acceptance:open": "KIT_TEST_DIR=tmp/test-prototype-package start-server-and-test 'node cypress/scripts/run-starter-prototype' 3000 'cypress open'",
     "test:smoke": "cypress run --spec \"cypress/integration/0-smoke-tests/*\"",
-    "test:unit": "jest --detectOpenHandles lib",
+    "test:unit": "jest --detectOpenHandles lib bin",
     "test:integration": "IS_INTEGRATION_TEST=true jest --detectOpenHandles --testTimeout=30000 __tests__",
     "test": "npm run test:unit && npm run test:integration && npm run lint"
   },