From 6a9f0499688b7515f3156a1754583d7624b25989 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Thu, 15 Mar 2018 14:22:43 +0100 Subject: [PATCH] tools,lib: forbid native Error constructors This adds a rule that forbids the use of native Error constructors in the `lib` directory. This is to encourage use of the `internal/errors` mechanism. The rule is disabled for errors that are not created with the `internal/errors` module but are still assigned an error code. PR-URL: https://github.com/nodejs/node/pull/19373 Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell --- .eslintrc.js | 1 + lib/.eslintrc.yaml | 18 ++++++++++++++++++ lib/_http_client.js | 1 + lib/_tls_wrap.js | 2 ++ lib/assert.js | 1 + lib/buffer.js | 1 + lib/child_process.js | 2 ++ lib/domain.js | 1 + lib/events.js | 1 + lib/internal/bootstrap/loaders.js | 1 + lib/internal/errors.js | 4 ++++ lib/internal/modules/cjs/loader.js | 1 + lib/internal/process/promises.js | 2 ++ lib/internal/process/warning.js | 1 + lib/net.js | 1 + lib/zlib.js | 1 + 16 files changed, 39 insertions(+) diff --git a/.eslintrc.js b/.eslintrc.js index 2250fdba77e..cbadffded25 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -148,6 +148,7 @@ module.exports = { } ], /* eslint-disable max-len, quotes */ + // If this list is modified, please copy the change to lib/.eslintrc.yaml 'no-restricted-syntax': [ 'error', { diff --git a/lib/.eslintrc.yaml b/lib/.eslintrc.yaml index 6ec81a725d4..bf2251525ee 100644 --- a/lib/.eslintrc.yaml +++ b/lib/.eslintrc.yaml @@ -1,4 +1,22 @@ rules: + no-restricted-syntax: + # Config copied from .eslintrc.js + - error + - selector: "CallExpression[callee.object.name='assert'][callee.property.name='doesNotThrow']" + message: "Please replace `assert.doesNotThrow()` and add a comment next to the code instead." + - selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.1.type='Literal']:not([arguments.1.regex])" + message: "use a regular expression for second argument of assert.throws()" + - selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.length<2]" + message: "assert.throws() must be invoked with at least two arguments." + - selector: "CallExpression[callee.name='setTimeout'][arguments.length<2]" + message: "setTimeout() must be invoked with at least two arguments." + - selector: "CallExpression[callee.name='setInterval'][arguments.length<2]" + message: "setInterval() must be invoked with at least 2 arguments." + - selector: "ThrowStatement > CallExpression[callee.name=/Error$/]" + message: "Use new keyword when throwing an Error." + # Config specific to lib + - selector: "NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError)$/])" + message: "Use an error exported by the internal/errors module." # Custom rules in tools/eslint-rules node-core/require-buffer: error node-core/buffer-constructor: error diff --git a/lib/_http_client.js b/lib/_http_client.js index 8e3d61a7561..03c69a9306b 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -307,6 +307,7 @@ function emitAbortNT() { function createHangUpError() { + // eslint-disable-next-line no-restricted-syntax var error = new Error('socket hang up'); error.code = 'ECONNRESET'; return error; diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 8ac497902eb..839aba555be 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -775,6 +775,7 @@ function onSocketClose(err) { // Emit ECONNRESET if (!this._controlReleased && !this[kErrorEmitted]) { this[kErrorEmitted] = true; + // eslint-disable-next-line no-restricted-syntax const connReset = new Error('socket hang up'); connReset.code = 'ECONNRESET'; this._tlsOptions.server.emit('tlsClientError', connReset, this); @@ -1103,6 +1104,7 @@ function onConnectEnd() { if (!this._hadError) { const options = this[kConnectOptions]; this._hadError = true; + // eslint-disable-next-line no-restricted-syntax const error = new Error('Client network socket disconnected before ' + 'secure TLS connection was established'); error.code = 'ECONNRESET'; diff --git a/lib/assert.js b/lib/assert.js index ccc1de3a25d..35132bfb03f 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -216,6 +216,7 @@ function innerOk(fn, argLen, value, message) { } else if (message == null) { // Use the call as error message if possible. // This does not work with e.g. the repl. + // eslint-disable-next-line no-restricted-syntax const err = new Error(); // Make sure the limit is set to 1. Otherwise it could fail (<= 0) or it // does to much work. diff --git a/lib/buffer.js b/lib/buffer.js index b369d27a1ed..c11762aa57c 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -1073,6 +1073,7 @@ if (process.binding('config').hasIntl) { return result; const code = icuErrName(result); + // eslint-disable-next-line no-restricted-syntax const err = new Error(`Unable to transcode Buffer [${code}]`); err.code = code; err.errno = result; diff --git a/lib/child_process.js b/lib/child_process.js index c18329554a4..2106bdc6819 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -278,6 +278,7 @@ exports.execFile = function(file /*, args, options, callback*/) { cmd += ` ${args.join(' ')}`; if (!ex) { + // eslint-disable-next-line no-restricted-syntax ex = new Error('Command failed: ' + cmd + '\n' + stderr); ex.killed = child.killed || killed; ex.code = code < 0 ? getSystemErrorName(code) : code; @@ -588,6 +589,7 @@ function checkExecSyncError(ret, args, cmd) { msg += cmd || args.join(' '); if (ret.stderr && ret.stderr.length > 0) msg += `\n${ret.stderr.toString()}`; + // eslint-disable-next-line no-restricted-syntax err = new Error(msg); } if (err) { diff --git a/lib/domain.js b/lib/domain.js index 170f2917278..2515d996497 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -89,6 +89,7 @@ if (process.hasUncaughtExceptionCaptureCallback()) { } // Get the stack trace at the point where `domain` was required. +// eslint-disable-next-line no-restricted-syntax const domainRequireStack = new Error('require(`domain`) at this point').stack; const { setUncaughtExceptionCaptureCallback } = process; diff --git a/lib/events.js b/lib/events.js index 2a8cf60c91c..46761653a01 100644 --- a/lib/events.js +++ b/lib/events.js @@ -240,6 +240,7 @@ function _addListener(target, type, listener, prepend) { if (m && m > 0 && existing.length > m) { existing.warned = true; // No error code for this since it is a Warning + // eslint-disable-next-line no-restricted-syntax const w = new Error('Possible EventEmitter memory leak detected. ' + `${existing.length} ${String(type)} listeners ` + 'added. Use emitter.setMaxListeners() to ' + diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index f7384b27848..fff82cd0e08 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -127,6 +127,7 @@ // Model the error off the internal/errors.js model, but // do not use that module given that it could actually be // the one causing the error if there's a bug in Node.js + // eslint-disable-next-line no-restricted-syntax const err = new Error(`No such built-in module: ${id}`); err.code = 'ERR_UNKNOWN_BUILTIN_MODULE'; err.name = 'Error [ERR_UNKNOWN_BUILTIN_MODULE]'; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index f7eeb946b64..0df98bffae2 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -444,6 +444,7 @@ function uvException(ctx) { // Pass the message to the constructor instead of setting it on the object // to make sure it is the same as the one created in C++ + // eslint-disable-next-line no-restricted-syntax const err = new Error(message); for (const prop of Object.keys(ctx)) { @@ -482,6 +483,7 @@ function errnoException(err, syscall, original) { const message = original ? `${syscall} ${code} ${original}` : `${syscall} ${code}`; + // eslint-disable-next-line no-restricted-syntax const ex = new Error(message); // TODO(joyeecheung): errno is supposed to err, like in uvException ex.code = ex.errno = code; @@ -517,6 +519,7 @@ function exceptionWithHostPort(err, syscall, address, port, additional) { details += ` - Local (${additional})`; } + // eslint-disable-next-line no-restricted-syntax const ex = new Error(`${syscall} ${code}${details}`); // TODO(joyeecheung): errno is supposed to err, like in uvException ex.code = ex.errno = code; @@ -537,6 +540,7 @@ function exceptionWithHostPort(err, syscall, address, port, additional) { * @returns {Error} */ function dnsException(err, syscall, hostname) { + // eslint-disable-next-line no-restricted-syntax const ex = new Error(); // FIXME(bnoordhuis) Remove this backwards compatibility nonsense and pass // the true error to the user. ENOTFOUND is not even a proper POSIX error! diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index a1722666619..470cf741e13 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -566,6 +566,7 @@ Module._resolveFilename = function(request, parent, isMain, options) { // look up the filename first, since that's the cache key. var filename = Module._findPath(request, paths, isMain); if (!filename) { + // eslint-disable-next-line no-restricted-syntax var err = new Error(`Cannot find module '${request}'`); err.code = 'MODULE_NOT_FOUND'; throw err; diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js index a1c7331a0d9..6cc366d8b2b 100644 --- a/lib/internal/process/promises.js +++ b/lib/internal/process/promises.js @@ -30,6 +30,7 @@ function handledRejection(promise) { if (promiseInfo.warned) { const { uid } = promiseInfo; // Generate the warning object early to get a good stack trace. + // eslint-disable-next-line no-restricted-syntax const warning = new Error('Promise rejection was handled ' + `asynchronously (rejection id: ${uid})`); warning.name = 'PromiseRejectionHandledWarning'; @@ -53,6 +54,7 @@ function emitWarning(uid, reason) { // ignored } + // eslint-disable-next-line no-restricted-syntax const warning = new Error( 'Unhandled promise rejection. This error originated either by ' + 'throwing inside of an async function without a catch block, ' + diff --git a/lib/internal/process/warning.js b/lib/internal/process/warning.js index df2d0c103b0..8c0a503239b 100644 --- a/lib/internal/process/warning.js +++ b/lib/internal/process/warning.js @@ -126,6 +126,7 @@ function setupProcessWarnings() { if (type !== undefined && typeof type !== 'string') throw new ERR_INVALID_ARG_TYPE('type', 'string'); if (warning === undefined || typeof warning === 'string') { + // eslint-disable-next-line no-restricted-syntax warning = new Error(warning); warning.name = String(type || 'Warning'); if (code !== undefined) warning.code = code; diff --git a/lib/net.js b/lib/net.js index 3693aeb6901..c0b4db81e67 100644 --- a/lib/net.js +++ b/lib/net.js @@ -385,6 +385,7 @@ function writeAfterFIN(chunk, encoding, cb) { encoding = null; } + // eslint-disable-next-line no-restricted-syntax var er = new Error('This socket has been ended by the other party'); er.code = 'EPIPE'; // TODO: defer error events consistently everywhere, not just the cb diff --git a/lib/zlib.js b/lib/zlib.js index 4adfd1ffa28..3660bd1a7d2 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -144,6 +144,7 @@ function zlibOnError(message, errno) { _close(self); self._hadError = true; + // eslint-disable-next-line no-restricted-syntax const error = new Error(message); error.errno = errno; error.code = codes[errno];