refactor(cli): use util.parseArgs to parse arguments (#283)

* feat: use util.parseArgs

* refactor: destrucure argument in parseCLIArguments

This moved argument destructuring to the parseCliArguments function, renames values to options,
renames positionals to patterns, and moves the try catch to the run function

* refactor: move default pattern to parseCliArgument

* fix: typo isQuiet->shouldBeQuiet

* fix: show help on illegal option value

* test: added tests for illegal argument usage

* feat: add support for --no-check and --no-quiet

* Revert "feat: add support for --no-check and --no-quiet"

This reverts commit 10a6067.

* test: add tests which throw for --no-* arguments

* test: update tests

* test: update snapshots

* style: linting

* refactor: use `Object.hasOwn`

---------

Co-authored-by: fisker Cheung <[email protected]>

cli.js

@@ -1,6 +1,7 @@
 #!/usr/bin/env node
 import { globSync } from 'tinyglobby'
 import fs from 'node:fs'
+import { parseArgs } from 'node:util'
 import getStdin from 'get-stdin'
 import sortPackageJson from './index.js'
 import Reporter from './reporter.js'
@@ -30,6 +31,32 @@ If file/glob is omitted, './package.json' file will be processed.
   )
 }
 
+function parseCliArguments() {
+  const { values: options, positionals: patterns } = parseArgs({
+    options: {
+      check: { type: 'boolean', short: 'c', default: false },
+      quiet: { type: 'boolean', short: 'q', default: false },
+      stdin: { type: 'boolean', default: false },
+      ignore: {
+        type: 'string',
+        short: 'i',
+        multiple: true,
+        default: ['node_modules/**'],
+      },
+      version: { type: 'boolean', short: 'v', default: false },
+      help: { type: 'boolean', short: 'h', default: false },
+    },
+    allowPositionals: true,
+    strict: true,
+  })
+
+  if (patterns.length === 0) {
+    patterns[0] = 'package.json'
+  }
+
+  return { options, patterns }
+}
+
 function sortPackageJsonFile(file, reporter, isCheck) {
   const original = fs.readFileSync(file, 'utf8')
   const sorted = sortPackageJson(original)
@@ -46,6 +73,7 @@ function sortPackageJsonFile(file, reporter, isCheck) {
 
 function sortPackageJsonFiles(patterns, { ignore, ...options }) {
   const files = globSync(patterns, { ignore })
+
   const reporter = new Reporter(files, options)
   const { isCheck } = options
 
@@ -64,61 +92,38 @@ async function sortPackageJsonFromStdin() {
 }
 
 function run() {
-  const cliArguments = process.argv
-    .slice(2)
-    .map((arg) => arg.split('='))
-    .flat()
-
-  if (
-    cliArguments.some((argument) => argument === '--help' || argument === '-h')
-  ) {
+  let options, patterns
+  try {
+    ;({ options, patterns } = parseCliArguments())
+  } catch (error) {
+    process.exitCode = 2
+    console.error(error.message)
+    if (
+      error.code === 'ERR_PARSE_ARGS_UNKNOWN_OPTION' ||
+      error.code === 'ERR_PARSE_ARGS_INVALID_OPTION_VALUE'
+    ) {
+      console.error(`Try 'sort-package-json --help' for more information.`)
+    }
+    return
+  }
+
+  if (options.help) {
     return showHelpInformation()
   }
 
-  if (
-    cliArguments.some(
-      (argument) => argument === '--version' || argument === '-v',
-    )
-  ) {
+  if (options.version) {
     return showVersion()
   }
 
-  if (cliArguments.some((argument) => argument === '--stdin')) {
+  if (options.stdin) {
     return sortPackageJsonFromStdin()
   }
 
-  const patterns = []
-  const ignore = []
-  let isCheck = false
-  let shouldBeQuiet = false
-
-  let lastArg
-  for (const argument of cliArguments) {
-    if (lastArg === '--ignore' || lastArg === '-i') {
-      ignore.push(argument)
-      lastArg = undefined
-      continue
-    }
-    if (argument === '--check' || argument === '-c') {
-      isCheck = true
-    } else if (argument === '--quiet' || argument === '-q') {
-      shouldBeQuiet = true
-    } else if (argument === '--ignore' || argument === '-i') {
-      lastArg = argument
-    } else {
-      patterns.push(argument)
-    }
-  }
-
-  if (!patterns.length) {
-    patterns[0] = 'package.json'
-  }
-
-  if (!ignore.length) {
-    ignore[0] = 'node_modules'
-  }
-
-  sortPackageJsonFiles(patterns, { ignore, isCheck, shouldBeQuiet })
+  sortPackageJsonFiles(patterns, {
+    ignore: options.ignore,
+    isCheck: options.check,
+    shouldBeQuiet: options.quiet,
+  })
 }
 
 run()

eslint.config.js

@@ -13,6 +13,7 @@ export default [
     languageOptions: {
       globals: { ...globals.builtin, ...globals.node },
     },
+    settings: { node: { version: '20' } },
   },
   { ignores: ['index.cjs'] },
 ]

index.js

@@ -5,11 +5,6 @@ import gitHooks from 'git-hooks-list'
 import isPlainObject from 'is-plain-obj'
 import semver from 'semver'
 
-const hasOwn =
-  // eslint-disable-next-line n/no-unsupported-features/es-builtins, n/no-unsupported-features/es-syntax -- will enable later
-  Object.hasOwn ||
-  // TODO: Remove this when we drop supported for Node.js v14
-  ((object, property) => Object.prototype.hasOwnProperty.call(object, property))
 const pipe =
   (fns) =>
   (x, ...args) =>
@@ -51,7 +46,7 @@ const sortDirectories = sortObjectBy([
 const overProperty =
   (property, over) =>
   (object, ...args) =>
-    hasOwn(object, property)
+    Object.hasOwn(object, property)
       ? { ...object, [property]: over(object[property], ...args) }
       : object
 const sortGitHooks = sortObjectBy(gitHooks)
@@ -218,8 +213,8 @@ const defaultNpmScripts = new Set([
 
 const hasDevDependency = (dependency, packageJson) => {
   return (
-    hasOwn(packageJson, 'devDependencies') &&
-    hasOwn(packageJson.devDependencies, dependency)
+    Object.hasOwn(packageJson, 'devDependencies') &&
+    Object.hasOwn(packageJson.devDependencies, dependency)
   )
 }
 

reporter.js

@@ -60,7 +60,7 @@ class Reporter {
       return
     }
 
-    const { isCheck, isQuiet } = this.#options
+    const { isCheck, shouldBeQuiet } = this.#options
 
     if (isCheck && changedFilesCount) {
       process.exitCode = 1
@@ -70,7 +70,7 @@ class Reporter {
       process.exitCode = 2
     }
 
-    if (isQuiet) {
+    if (shouldBeQuiet) {
       return
     }
 

tests/cli.js

@@ -63,6 +63,31 @@ test('run `cli --help` with `--version`', macro.testCLI, {
   message: 'Should prioritize help over version.',
 })
 
+test('run `cli --help=value`', macro.testCLI, {
+  args: ['--help=value'],
+  message: 'Should report illegal argument and suggest help.',
+})
+
+test('run `cli --version=true`', macro.testCLI, {
+  args: ['--version=true'],
+  message: 'Should report illegal argument and suggest help.',
+})
+
+test('run `cli --unknown-option`', macro.testCLI, {
+  args: ['--unknown-option'],
+  message: 'Should report unknown option and suggest help.',
+})
+
+test('run `cli -u` with unknown option', macro.testCLI, {
+  args: ['-u'],
+  message: 'Should report unknown option and suggest help.',
+})
+
+test('run `cli --no-version`', macro.testCLI, {
+  args: ['--no-version'],
+  message: 'A snapshot to show how `--no-*` works, not care about result.',
+})
+
 test('run `cli` with no patterns', macro.testCLI, {
   fixtures: [
     {
@@ -87,6 +112,11 @@ test('run `cli --quiet` with no patterns', macro.testCLI, {
   message: 'Should format package.json without message.',
 })
 
+test('run `cli --quiet=value`', macro.testCLI, {
+  args: ['--quiet=value'],
+  message: 'Should report illegal argument and suggest help.',
+})
+
 test('run `cli -q` with no patterns', macro.testCLI, {
   fixtures: [
     {
@@ -111,6 +141,11 @@ test('run `cli --check` with no patterns', macro.testCLI, {
   message: 'Should not sort package.json',
 })
 
+test('run `cli --check=value`', macro.testCLI, {
+  args: ['--check=value'],
+  message: 'Should report illegal argument and suggest help.',
+})
+
 test('run `cli --check --quiet` with no patterns', macro.testCLI, {
   fixtures: [
     {
@@ -147,6 +182,18 @@ test('run `cli -c -q` with no patterns', macro.testCLI, {
   message: 'Should support `-q` alias',
 })
 
+test('run `cli -cq` with no patterns', macro.testCLI, {
+  fixtures: [
+    {
+      file: 'package.json',
+      content: badJson,
+      expect: badJson,
+    },
+  ],
+  args: ['-cq'],
+  message: 'Should support option aggregation',
+})
+
 test('run `cli` on 1 bad file', macro.testCLI, {
   fixtures: [
     {