From 698c3781b7ad2c089bc12ae3f9551f82ee459b04 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Fri, 9 Dec 2022 11:38:25 +0900 Subject: [PATCH 1/5] errors: refactor to use a method that formats a list string Signed-off-by: Daeyeon Jeong --- lib/internal/errors.js | 53 ++++++++----------------- test/parallel/test-error-format-list.js | 28 +++++++++++++ 2 files changed, 45 insertions(+), 36 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..e6e1cc33d6aa02 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,17 @@ function determineSpecificType(value) { return `type ${typeof value} (${inspected})`; } +/** + * Create a list string in the form like 'A and B' or 'A, B, ..., and Z'. + * @param {Array} array An Array + * @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 +919,7 @@ module.exports = { errnoException, exceptionWithHostPort, fatalExceptionStackEnhancers, + formatList, genericNodeError, getMessage, hideInternalStackFrames, @@ -1240,39 +1251,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 +1444,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); diff --git a/test/parallel/test-error-format-list.js b/test/parallel/test-error-format-list.js new file mode 100644 index 00000000000000..9aae5da4558f85 --- /dev/null +++ b/test/parallel/test-error-format-list.js @@ -0,0 +1,28 @@ +// Flags: --expose-internals +'use strict'; + +require('../common'); +const { strictEqual } = require('node:assert'); +const { formatList } = require('internal/errors'); + +strictEqual(formatList([]), ''); + +strictEqual(formatList([], 'or'), ''); + +strictEqual(formatList(['apple']), 'apple'); + +strictEqual(formatList(['apple'], 'or'), 'apple'); + +strictEqual(formatList(['apple', 'banana']), 'apple and banana'); + +strictEqual(formatList(['apple', 'banana'], 'or'), 'apple or banana'); + +strictEqual( + formatList(['apple', 'banana', 'orange']), + 'apple, banana, and orange' +); + +strictEqual( + formatList(['apple', 'banana', 'orange'], 'or'), + 'apple, banana, or orange' +); From 657e6609227c901eac3317a9a5c242f97553b3b3 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Fri, 9 Dec 2022 11:48:47 +0900 Subject: [PATCH 2/5] errors: fix ERR_UNSUPPORTED_ESM_URL_SCHEME error message Signed-off-by: Daeyeon Jeong --- lib/internal/errors.js | 2 +- test/es-module/test-esm-dynamic-import.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index e6e1cc33d6aa02..4ddaec493f275b 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1677,7 +1677,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'), From 9a04ac11cae075e1dc7b9a8ad83e8fee214b0925 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Fri, 9 Dec 2022 18:46:47 +0900 Subject: [PATCH 3/5] fixup! errors: fix ERR_UNSUPPORTED_ESM_URL_SCHEME error message Signed-off-by: Daeyeon Jeong Co-authored-by: Antoine du Hamel --- test/parallel/test-error-format-list.js | 34 +++++++++---------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/test/parallel/test-error-format-list.js b/test/parallel/test-error-format-list.js index 9aae5da4558f85..aeb3e4011e3b97 100644 --- a/test/parallel/test-error-format-list.js +++ b/test/parallel/test-error-format-list.js @@ -1,28 +1,18 @@ // Flags: --expose-internals 'use strict'; -require('../common'); +const common = require('../common'); const { strictEqual } = require('node:assert'); const { formatList } = require('internal/errors'); -strictEqual(formatList([]), ''); - -strictEqual(formatList([], 'or'), ''); - -strictEqual(formatList(['apple']), 'apple'); - -strictEqual(formatList(['apple'], 'or'), 'apple'); - -strictEqual(formatList(['apple', 'banana']), 'apple and banana'); - -strictEqual(formatList(['apple', 'banana'], 'or'), 'apple or banana'); - -strictEqual( - formatList(['apple', 'banana', 'orange']), - 'apple, banana, and orange' -); - -strictEqual( - formatList(['apple', 'banana', 'orange'], 'or'), - 'apple, banana, or orange' -); +if (common.hasIntl) { + 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)); + } +} From d513efbf9ff0c46abfa9e7d104921e6915545a38 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Fri, 9 Dec 2022 19:16:39 +0900 Subject: [PATCH 4/5] fixup! fixup! errors: fix ERR_UNSUPPORTED_ESM_URL_SCHEME error message Signed-off-by: Daeyeon Jeong Co-authored-by: Antoine du Hamel --- lib/internal/errors.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 4ddaec493f275b..e641347cc51058 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -896,6 +896,8 @@ function determineSpecificType(value) { /** * 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 {Array} array An Array * @param {string} type The list type to be inserted before the last element. * @returns {string} From a05a23a96e9525652e5449b4755e642527ff644f Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Fri, 9 Dec 2022 20:05:56 +0900 Subject: [PATCH 5/5] fixup! fixup! fixup! errors: fix ERR_UNSUPPORTED_ESM_URL_SCHEME error message Co-authored-by: Antoine du Hamel --- lib/internal/errors.js | 4 ++-- test/parallel/test-error-format-list.js | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index e641347cc51058..b23f594378b67a 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -898,8 +898,8 @@ function determineSpecificType(value) { * 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 {Array} array An Array - * @param {string} type The list type to be inserted before the last element. + * @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') { diff --git a/test/parallel/test-error-format-list.js b/test/parallel/test-error-format-list.js index aeb3e4011e3b97..54ae4e0aee714d 100644 --- a/test/parallel/test-error-format-list.js +++ b/test/parallel/test-error-format-list.js @@ -5,7 +5,9 @@ const common = require('../common'); const { strictEqual } = require('node:assert'); const { formatList } = require('internal/errors'); -if (common.hasIntl) { +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' });