Skip to content

Commit

Permalink
errors: refactor to use a method that formats a list string
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
daeyeon authored and juanarbol committed Jan 31, 2023
1 parent 64d3e3f commit 2047577
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 38 deletions.
57 changes: 20 additions & 37 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const {
ArrayPrototypeIndexOf,
ArrayPrototypeJoin,
ArrayPrototypeMap,
ArrayPrototypePop,
ArrayPrototypePush,
ArrayPrototypeSlice,
ArrayPrototypeSplice,
Expand Down Expand Up @@ -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,
Expand All @@ -909,6 +921,7 @@ module.exports = {
errnoException,
exceptionWithHostPort,
fatalExceptionStackEnhancers,
formatList,
genericNodeError,
getMessage,
hideInternalStackFrames,
Expand Down Expand Up @@ -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 ';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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';
Expand Down
2 changes: 1 addition & 1 deletion test/es-module/test-esm-dynamic-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
20 changes: 20 additions & 0 deletions test/parallel/test-error-format-list.js
Original file line number Diff line number Diff line change
@@ -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));
}
}

0 comments on commit 2047577

Please sign in to comment.