errors: refactor to use a method that formats a list string

daeyeon · RafaelGSS · commit 8046e0ef533d · 2023-01-05T13:11:36.000-03:00
Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com> PR-URL: #45793 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -19,7 +19,6 @@ const {
   ArrayPrototypeIndexOf,
   ArrayPrototypeJoin,
   ArrayPrototypeMap,
-  ArrayPrototypePop,
   ArrayPrototypePush,
   ArrayPrototypeSlice,
   ArrayPrototypeSplice,
@@ -895,6 +894,19 @@ function determineSpecificType(value) {
   return `type ${typeof value} (${inspected})`;
 }
 
+/**
+ * Create a list string in the form like 'A and B' or 'A, B, ..., and Z'.
+ * We cannot use Intl.ListFormat because it's not available in
+ * --without-intl builds.
+ * @param {string[]} array An array of strings.
+ * @param {string} [type] The list type to be inserted before the last element.
+ * @returns {string}
+ */
+function formatList(array, type = 'and') {
+  return array.length < 3 ? ArrayPrototypeJoin(array, ` ${type} `) :
+    `${ArrayPrototypeJoin(ArrayPrototypeSlice(array, 0, -1), ', ')}, ${type} ${array[array.length - 1]}`;
+}
+
 module.exports = {
   AbortError,
   aggregateTwoErrors,
@@ -909,6 +921,7 @@ module.exports = {
   errnoException,
   exceptionWithHostPort,
   fatalExceptionStackEnhancers,
+  formatList,
   genericNodeError,
   getMessage,
   hideInternalStackFrames,
@@ -1240,39 +1253,20 @@ E('ERR_INVALID_ARG_TYPE',
     }
 
     if (types.length > 0) {
-      if (types.length > 2) {
-        const last = ArrayPrototypePop(types);
-        msg += `one of type ${ArrayPrototypeJoin(types, ', ')}, or ${last}`;
-      } else if (types.length === 2) {
-        msg += `one of type ${types[0]} or ${types[1]}`;
-      } else {
-        msg += `of type ${types[0]}`;
-      }
+      msg += `${types.length > 1 ? 'one of type' : 'of type'} ${formatList(types, 'or')}`;
       if (instances.length > 0 || other.length > 0)
         msg += ' or ';
     }
 
     if (instances.length > 0) {
-      if (instances.length > 2) {
-        const last = ArrayPrototypePop(instances);
-        msg +=
-          `an instance of ${ArrayPrototypeJoin(instances, ', ')}, or ${last}`;
-      } else {
-        msg += `an instance of ${instances[0]}`;
-        if (instances.length === 2) {
-          msg += ` or ${instances[1]}`;
-        }
-      }
+      msg += `an instance of ${formatList(instances, 'or')}`;
       if (other.length > 0)
         msg += ' or ';
     }
 
     if (other.length > 0) {
-      if (other.length > 2) {
-        const last = ArrayPrototypePop(other);
-        msg += `one of ${ArrayPrototypeJoin(other, ', ')}, or ${last}`;
-      } else if (other.length === 2) {
-        msg += `one of ${other[0]} or ${other[1]}`;
+      if (other.length > 1) {
+        msg += `one of ${formatList(other, 'or')}`;
       } else {
         if (StringPrototypeToLowerCase(other[0]) !== other[0])
           msg += 'an ';
@@ -1452,18 +1446,7 @@ E('ERR_MISSING_ARGS',
         ArrayPrototypeJoin(ArrayPrototypeMap(a, wrap), ' or ') :
         wrap(a))
     );
-    switch (len) {
-      case 1:
-        msg += `${args[0]} argument`;
-        break;
-      case 2:
-        msg += `${args[0]} and ${args[1]} arguments`;
-        break;
-      default:
-        msg += ArrayPrototypeJoin(ArrayPrototypeSlice(args, 0, len - 1), ', ');
-        msg += `, and ${args[len - 1]} arguments`;
-        break;
-    }
+    msg += `${formatList(args)} argument${len > 1 ? 's' : ''}`;
     return `${msg} must be specified`;
   }, TypeError);
 E('ERR_MISSING_OPTION', '%s is required', TypeError);
@@ -1696,7 +1679,7 @@ E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError);
 E('ERR_UNSUPPORTED_DIR_IMPORT', "Directory import '%s' is not supported " +
 'resolving ES modules imported from %s', Error);
 E('ERR_UNSUPPORTED_ESM_URL_SCHEME', (url, supported) => {
-  let msg = `Only URLs with a scheme in: ${ArrayPrototypeJoin(supported, ', ')} are supported by the default ESM loader`;
+  let msg = `Only URLs with a scheme in: ${formatList(supported)} are supported by the default ESM loader`;
   if (isWindows && url.protocol.length === 2) {
     msg +=
       '. On Windows, absolute paths must be valid file:// URLs';
diff --git a/test/es-module/test-esm-dynamic-import.js b/test/es-module/test-esm-dynamic-import.js
@@ -59,7 +59,7 @@ function expectFsNamespace(result) {
                     'ERR_UNSUPPORTED_ESM_URL_SCHEME');
   if (common.isWindows) {
     const msg =
-      'Only URLs with a scheme in: file, data are supported by the default ' +
+      'Only URLs with a scheme in: file and data are supported by the default ' +
       'ESM loader. On Windows, absolute paths must be valid file:// URLs. ' +
       "Received protocol 'c:'";
     expectModuleError(import('C:\\example\\foo.mjs'),
diff --git a/test/parallel/test-error-format-list.js b/test/parallel/test-error-format-list.js
@@ -0,0 +1,20 @@
+// Flags: --expose-internals
+'use strict';
+
+const common = require('../common');
+const { strictEqual } = require('node:assert');
+const { formatList } = require('internal/errors');
+
+if (!common.hasIntl) common.skip('missing Intl');
+
+{
+  const and = new Intl.ListFormat('en', { style: 'long', type: 'conjunction' });
+  const or = new Intl.ListFormat('en', { style: 'long', type: 'disjunction' });
+
+  const input = ['apple', 'banana', 'orange'];
+  for (let i = 0; i < input.length; i++) {
+    const slicedInput = input.slice(0, i);
+    strictEqual(formatList(slicedInput), and.format(slicedInput));
+    strictEqual(formatList(slicedInput, 'or'), or.format(slicedInput));
+  }
+}
