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: #19373
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>

.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',
       {

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

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;

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';

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.

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;

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) {

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;

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 ' +

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]';

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!

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;

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, ' +

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;

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

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];