From 204757719c2ad553596fdbae2a395aa0f6cf8d57 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Fri, 23 Dec 2022 09:49:11 +0900 Subject: [PATCH] errors: refactor to use a method that formats a list string Signed-off-by: Daeyeon Jeong PR-URL: https://github.com/nodejs/node/pull/45793 Reviewed-By: James M Snell Reviewed-By: Antoine du Hamel --- lib/internal/errors.js | 57 ++++++++--------------- test/es-module/test-esm-dynamic-import.js | 2 +- test/parallel/test-error-format-list.js | 20 ++++++++ 3 files changed, 41 insertions(+), 38 deletions(-) create mode 100644 test/parallel/test-error-format-list.js diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 754479e63ca98a..b23f594378b67a 100644 --- 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 index eed5f230cc87a5..95d34244357e7c 100644 --- 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 new file mode 100644 index 00000000000000..54ae4e0aee714d --- /dev/null +++ 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)); + } +}