From 4b82d892abfcac160a0486d432e23e27315a5765 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Thu, 9 Nov 2017 10:18:12 -0500 Subject: [PATCH] errors: consistent format for error message Consistently use printf-style strings for error messages that do not need a custom argument order or processing of arguments. PR-URL: https://github.com/nodejs/node/pull/16904 Reviewed-By: James M Snell Reviewed-By: Michael Dawson Reviewed-By: Anna Henningsen --- lib/internal/errors.js | 68 ++++++++----------- .../test-eslint-prefer-util-format-errors.js | 27 ++++++++ .../eslint-rules/prefer-util-format-errors.js | 39 +++++++++++ 3 files changed, 93 insertions(+), 41 deletions(-) create mode 100644 test/parallel/test-eslint-prefer-util-format-errors.js create mode 100644 tools/eslint-rules/prefer-util-format-errors.js diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 1eac23c42fe6d3..4da05899b09b1b 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1,5 +1,6 @@ /* eslint documented-errors: "error" */ /* eslint alphabetize-errors: "error" */ +/* eslint prefer-util-format-errors: "error" */ 'use strict'; @@ -222,8 +223,8 @@ module.exports = exports = { E('ERR_ARG_NOT_ITERABLE', '%s must be iterable'); E('ERR_ASSERTION', '%s'); -E('ERR_ASYNC_CALLBACK', (name) => `${name} must be a function`); -E('ERR_ASYNC_TYPE', (s) => `Invalid name for async "type": ${s}`); +E('ERR_ASYNC_CALLBACK', '%s must be a function'); +E('ERR_ASYNC_TYPE', 'Invalid name for async "type": %s'); E('ERR_BUFFER_OUT_OF_BOUNDS', bufferOutOfBounds); E('ERR_BUFFER_TOO_LARGE', `Cannot create a Buffer larger than 0x${kMaxLength.toString(16)} bytes`); @@ -247,12 +248,10 @@ E('ERR_CRYPTO_INVALID_STATE', 'Invalid state for operation %s'); E('ERR_CRYPTO_SIGN_KEY_REQUIRED', 'No key provided to sign'); E('ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH', 'Input buffers must have the same length'); -E('ERR_DNS_SET_SERVERS_FAILED', (err, servers) => - `c-ares failed to set servers: "${err}" [${servers}]`); +E('ERR_DNS_SET_SERVERS_FAILED', 'c-ares failed to set servers: "%s" [%s]'); E('ERR_ENCODING_INVALID_ENCODED_DATA', - (enc) => `The encoded data was not valid for encoding ${enc}`); -E('ERR_ENCODING_NOT_SUPPORTED', - (enc) => `The "${enc}" encoding is not supported`); + 'The encoded data was not valid for encoding %s'); +E('ERR_ENCODING_NOT_SUPPORTED', 'The "%s" encoding is not supported'); E('ERR_FALSY_VALUE_REJECTION', 'Promise was rejected with falsy value'); E('ERR_HTTP2_CONNECT_AUTHORITY', ':authority header is required for CONNECT requests'); @@ -272,10 +271,9 @@ E('ERR_HTTP2_HEADERS_AFTER_RESPOND', 'Cannot specify additional headers after response initiated'); E('ERR_HTTP2_HEADERS_OBJECT', 'Headers must be an object'); E('ERR_HTTP2_HEADERS_SENT', 'Response has already been initiated.'); -E('ERR_HTTP2_HEADER_REQUIRED', - (name) => `The ${name} header is required`); +E('ERR_HTTP2_HEADER_REQUIRED', 'The %s header is required'); E('ERR_HTTP2_HEADER_SINGLE_VALUE', - (name) => `Header field "${name}" must have only a single value`); + 'Header field "%s" must have only a single value'); E('ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND', 'Cannot send informational headers after the HTTP message has been sent'); E('ERR_HTTP2_INFO_STATUS_NOT_ALLOWED', @@ -284,23 +282,23 @@ E('ERR_HTTP2_INVALID_CONNECTION_HEADERS', 'HTTP/1 Connection specific headers are forbidden: "%s"'); E('ERR_HTTP2_INVALID_HEADER_VALUE', 'Invalid value "%s" for header "%s"'); E('ERR_HTTP2_INVALID_INFO_STATUS', - (code) => `Invalid informational status code: ${code}`); + 'Invalid informational status code: %s'); E('ERR_HTTP2_INVALID_PACKED_SETTINGS_LENGTH', 'Packed settings length must be a multiple of six'); E('ERR_HTTP2_INVALID_PSEUDOHEADER', - (name) => `"${name}" is an invalid pseudoheader or is used incorrectly`); + '"%s" is an invalid pseudoheader or is used incorrectly'); E('ERR_HTTP2_INVALID_SESSION', 'The session has been destroyed'); E('ERR_HTTP2_INVALID_SETTING_VALUE', - (name, value) => `Invalid value for setting "${name}": ${value}`); + 'Invalid value for setting "%s": %s'); E('ERR_HTTP2_INVALID_STREAM', 'The stream has been destroyed'); E('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK', - (max) => `Maximum number of pending settings acknowledgements (${max})`); + 'Maximum number of pending settings acknowledgements (%s)'); E('ERR_HTTP2_NO_SOCKET_MANIPULATION', 'HTTP/2 sockets should not be directly manipulated (e.g. read and written)'); E('ERR_HTTP2_OUT_OF_STREAMS', 'No stream ID is available because maximum stream ID has been reached'); E('ERR_HTTP2_PAYLOAD_FORBIDDEN', - (code) => `Responses with ${code} status must not have a payload`); + 'Responses with %s status must not have a payload'); E('ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED', 'Cannot set HTTP/2 pseudo-headers'); E('ERR_HTTP2_PUSH_DISABLED', 'HTTP/2 client has disabled push streams'); E('ERR_HTTP2_SEND_FILE', 'Only regular files can be sent'); @@ -308,20 +306,16 @@ E('ERR_HTTP2_SOCKET_BOUND', 'The socket is already bound to an Http2Session'); E('ERR_HTTP2_STATUS_101', 'HTTP status code 101 (Switching Protocols) is forbidden in HTTP/2'); -E('ERR_HTTP2_STATUS_INVALID', - (code) => `Invalid status code: ${code}`); +E('ERR_HTTP2_STATUS_INVALID', 'Invalid status code: %s'); E('ERR_HTTP2_STREAM_CLOSED', 'The stream is already closed'); -E('ERR_HTTP2_STREAM_ERROR', - (code) => `Stream closed with error code ${code}`); +E('ERR_HTTP2_STREAM_ERROR', 'Stream closed with error code %s'); E('ERR_HTTP2_STREAM_SELF_DEPENDENCY', 'A stream cannot depend on itself'); -E('ERR_HTTP2_UNSUPPORTED_PROTOCOL', - (protocol) => `protocol "${protocol}" is unsupported.`); +E('ERR_HTTP2_UNSUPPORTED_PROTOCOL', 'protocol "%s" is unsupported.'); E('ERR_HTTP_HEADERS_SENT', 'Cannot %s headers after they are sent to the client'); E('ERR_HTTP_INVALID_CHAR', 'Invalid character in statusMessage.'); E('ERR_HTTP_INVALID_HEADER_VALUE', 'Invalid value "%s" for header "%s"'); -E('ERR_HTTP_INVALID_STATUS_CODE', - (originalStatusCode) => `Invalid status code: ${originalStatusCode}`); +E('ERR_HTTP_INVALID_STATUS_CODE', 'Invalid status code: %s'); E('ERR_HTTP_TRAILER_INVALID', 'Trailers are invalid with this transfer encoding'); E('ERR_INDEX_OUT_OF_RANGE', 'Index out of range'); @@ -330,16 +324,14 @@ E('ERR_INSPECTOR_CLOSED', 'Session was closed'); E('ERR_INSPECTOR_NOT_AVAILABLE', 'Inspector is not available'); E('ERR_INSPECTOR_NOT_CONNECTED', 'Session is not connected'); E('ERR_INVALID_ARG_TYPE', invalidArgType); -E('ERR_INVALID_ARG_VALUE', - (name, value) => { - return `The value "${String(value)}" is invalid for argument "${name}"`; - }); +E('ERR_INVALID_ARG_VALUE', (name, value) => + `The value "${String(value)}" is invalid for argument "${name}"`); E('ERR_INVALID_ARRAY_LENGTH', (name, len, actual) => { internalAssert(typeof actual === 'number', 'actual must be a number'); return `The array "${name}" (length ${actual}) must be of length ${len}.`; }); -E('ERR_INVALID_ASYNC_ID', (type, id) => `Invalid ${type} value: ${id}`); +E('ERR_INVALID_ASYNC_ID', 'Invalid %s value: %s'); E('ERR_INVALID_BUFFER_SIZE', 'Buffer size must be a multiple of %s'); E('ERR_INVALID_CALLBACK', 'Callback must be a function'); E('ERR_INVALID_CHAR', invalidChar); @@ -354,15 +346,12 @@ E('ERR_INVALID_FILE_URL_PATH', 'File URL path %s'); E('ERR_INVALID_HANDLE_TYPE', 'This handle type cannot be sent'); E('ERR_INVALID_HTTP_TOKEN', '%s must be a valid HTTP token ["%s"]'); E('ERR_INVALID_IP_ADDRESS', 'Invalid IP address: %s'); -E('ERR_INVALID_OPT_VALUE', - (name, value) => { - return `The value "${String(value)}" is invalid for option "${name}"`; - }); +E('ERR_INVALID_OPT_VALUE', (name, value) => + `The value "${String(value)}" is invalid for option "${name}"`); E('ERR_INVALID_OPT_VALUE_ENCODING', - (value) => `The value "${String(value)}" is invalid for option "encoding"`); + 'The value "%s" is invalid for option "encoding"'); E('ERR_INVALID_PERFORMANCE_MARK', 'The "%s" performance mark has not been set'); -E('ERR_INVALID_PROTOCOL', (protocol, expectedProtocol) => - `Protocol "${protocol}" not supported. Expected "${expectedProtocol}"`); +E('ERR_INVALID_PROTOCOL', 'Protocol "%s" not supported. Expected "%s"'); E('ERR_INVALID_REPL_EVAL_CONFIG', 'Cannot specify both "breakEvalOnSigint" and "eval" for REPL'); E('ERR_INVALID_SYNC_FORK_INPUT', @@ -402,8 +391,7 @@ E('ERR_SOCKET_BAD_BUFFER_SIZE', 'Buffer size must be a positive integer'); E('ERR_SOCKET_BAD_PORT', 'Port should be > 0 and < 65536. Received %s.'); E('ERR_SOCKET_BAD_TYPE', 'Bad socket type specified. Valid types are: udp4, udp6'); -E('ERR_SOCKET_BUFFER_SIZE', - (reason) => `Could not get or set buffer size: ${reason}`); +E('ERR_SOCKET_BUFFER_SIZE', 'Could not get or set buffer size: %s'); E('ERR_SOCKET_CANNOT_SEND', 'Unable to send data'); E('ERR_SOCKET_CLOSED', 'Socket is closed'); E('ERR_SOCKET_DGRAM_NOT_RUNNING', 'Not running'); @@ -419,8 +407,7 @@ E('ERR_STREAM_WRITE_AFTER_END', 'write after end'); E('ERR_SYSTEM_ERROR', sysError('A system error occurred')); E('ERR_TLS_CERT_ALTNAME_INVALID', 'Hostname/IP does not match certificate\'s altnames: %s'); -E('ERR_TLS_DH_PARAM_SIZE', (size) => - `DH parameter size ${size} is less than 2048`); +E('ERR_TLS_DH_PARAM_SIZE', 'DH parameter size %s is less than 2048'); E('ERR_TLS_HANDSHAKE_TIMEOUT', 'TLS handshake timeout'); E('ERR_TLS_RENEGOTIATION_FAILED', 'Failed to renegotiate'); E('ERR_TLS_REQUIRED_SERVER_NAME', @@ -430,8 +417,7 @@ E('ERR_TRANSFORM_ALREADY_TRANSFORMING', 'Calling transform done when still transforming'); E('ERR_TRANSFORM_WITH_LENGTH_0', 'Calling transform done when writableState.length != 0'); -E('ERR_UNESCAPED_CHARACTERS', - (name) => `${name} contains unescaped characters`); +E('ERR_UNESCAPED_CHARACTERS', '%s contains unescaped characters'); E('ERR_UNHANDLED_ERROR', (err) => { const msg = 'Unhandled error.'; diff --git a/test/parallel/test-eslint-prefer-util-format-errors.js b/test/parallel/test-eslint-prefer-util-format-errors.js new file mode 100644 index 00000000000000..265a0752c50d43 --- /dev/null +++ b/test/parallel/test-eslint-prefer-util-format-errors.js @@ -0,0 +1,27 @@ +'use strict'; + +/* eslint-disable no-template-curly-in-string */ + +require('../common'); + +const RuleTester = require('../../tools/eslint').RuleTester; +const rule = require('../../tools/eslint-rules/prefer-util-format-errors'); + +new RuleTester({ parserOptions: { ecmaVersion: 6 } }) + .run('prefer-util-format-errors', rule, { + valid: [ + 'E(\'ABC\', \'abc\');', + 'E(\'ABC\', (arg1, arg2) => `${arg2}${arg1}`);', + 'E(\'ABC\', (arg1, arg2) => `${arg1}{arg2.something}`);', + 'E(\'ABC\', (arg1, arg2) => fn(arg1, arg2));' + ], + invalid: [ + { + code: 'E(\'ABC\', (arg1, arg2) => `${arg1}${arg2}`);', + errors: [{ + message: 'Please use a printf-like formatted string that ' + + 'util.format can consume.' + }] + } + ] + }); diff --git a/tools/eslint-rules/prefer-util-format-errors.js b/tools/eslint-rules/prefer-util-format-errors.js new file mode 100644 index 00000000000000..c3f4819e43b51c --- /dev/null +++ b/tools/eslint-rules/prefer-util-format-errors.js @@ -0,0 +1,39 @@ +'use strict'; + +const errMsg = 'Please use a printf-like formatted string that util.format' + + ' can consume.'; + +function isArrowFunctionWithTemplateLiteral(node) { + return node.type === 'ArrowFunctionExpression' && + node.body.type === 'TemplateLiteral'; +} + +function isDefiningError(node) { + return node.expression && + node.expression.type === 'CallExpression' && + node.expression.callee && + node.expression.callee.name === 'E'; +} + +module.exports = { + create: function(context) { + return { + ExpressionStatement: function(node) { + if (!isDefiningError(node)) + return; + + const msg = node.expression.arguments[1]; + if (!isArrowFunctionWithTemplateLiteral(msg)) + return; + + const { expressions } = msg.body; + const hasSequentialParams = msg.params.every((param, index) => { + const expr = expressions[index]; + return expr && expr.type === 'Identifier' && param.name === expr.name; + }); + if (hasSequentialParams) + context.report(msg, errMsg); + } + }; + } +};