tools: prohibit assert.doesNotReject() in Node.js core

This makes sure we do not use `assert.doesNotReject()` anywhere in our code base. This is just a simple wrapper that catches the rejection and then rejects it again in case of an error. Thus, it is not useful to do that. The error message for `assert.doesNotThrow()` is also improved. PR-URL: #27402 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Apr 29, 2019 · 76af4f0 · 76af4f0
1 parent 36d6168
commit 76af4f0
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 38 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -184,7 +184,11 @@ module.exports = {
       },
       {
         selector: "CallExpression[callee.property.name='doesNotThrow']",
-        message: 'Please replace `assert.doesNotThrow()` and add a comment next to the code instead.',
+        message: 'Do not use `assert.doesNotThrow()`. Write the code without the wrapper and add a comment instead.',
+      },
+      {
+        selector: "CallExpression[callee.property.name='doesNotReject']",
+        message: 'Do not use `assert.doesNotReject()`. Write the code without the wrapper and add a comment instead.',
       },
       {
         selector: "CallExpression[callee.property.name='rejects'][arguments.length<2]",

diff --git a/doc/api/assert.md b/doc/api/assert.md
@@ -454,6 +454,7 @@ function. See [`assert.throws()`][] for more details.
 Besides the async nature to await the completion behaves identically to
 [`assert.doesNotThrow()`][].
 
+<!-- eslint-disable no-restricted-syntax -->
 ```js
 (async () => {
   await assert.doesNotReject(
@@ -465,6 +466,7 @@ Besides the async nature to await the completion behaves identically to
 })();
 ```
 
+<!-- eslint-disable no-restricted-syntax -->
 ```js
 assert.doesNotReject(Promise.reject(new TypeError('Wrong value')))
   .then(() => {

diff --git a/test/parallel/test-assert-async.js b/test/parallel/test-assert-async.js
@@ -114,11 +114,36 @@ promises.push(assert.rejects(
   }
 ));
 
+{
+  const handler = (generated, actual, err) => {
+    assert.strictEqual(err.generatedMessage, generated);
+    assert.strictEqual(err.code, 'ERR_ASSERTION');
+    assert.strictEqual(err.actual, actual);
+    assert.strictEqual(err.operator, 'rejects');
+    assert(/rejects/.test(err.stack));
+    return true;
+  };
+  const err = new Error();
+  promises.push(assert.rejects(
+    assert.rejects(Promise.reject(null), { code: 'FOO' }),
+    handler.bind(null, true, null)
+  ));
+  promises.push(assert.rejects(
+    assert.rejects(Promise.reject(5), { code: 'FOO' }, 'AAAAA'),
+    handler.bind(null, false, 5)
+  ));
+  promises.push(assert.rejects(
+    assert.rejects(Promise.reject(err), { code: 'FOO' }, 'AAAAA'),
+    handler.bind(null, false, err)
+  ));
+}
+
 // Check `assert.doesNotReject`.
 {
   // `assert.doesNotReject` accepts a function or a promise
   // or a thenable as first argument.
-  const promise = assert.doesNotReject(() => new Map(), common.mustNotCall());
+  /* eslint-disable no-restricted-syntax */
+  let promise = assert.doesNotReject(() => new Map(), common.mustNotCall());
   promises.push(assert.rejects(promise, {
     message: 'Expected instance of Promise to be returned ' +
              'from the "promiseFn" function but got instance of Map.',
@@ -149,9 +174,7 @@ promises.push(assert.rejects(
       code: 'ERR_INVALID_RETURN_VALUE'
     })
   );
-}
 
-{
   const handler1 = (err) => {
     assert(err instanceof assert.AssertionError,
            `${err.name} is not instance of AssertionError`);
@@ -173,47 +196,24 @@ promises.push(assert.rejects(
 
   const rejectingFn = async () => assert.fail();
 
-  let promise = assert.doesNotReject(rejectingFn, common.mustCall(handler1));
+  promise = assert.doesNotReject(rejectingFn, common.mustCall(handler1));
   promises.push(assert.rejects(promise, common.mustCall(handler2)));
 
   promise = assert.doesNotReject(rejectingFn(), common.mustCall(handler1));
   promises.push(assert.rejects(promise, common.mustCall(handler2)));
 
   promise = assert.doesNotReject(() => assert.fail(), common.mustNotCall());
   promises.push(assert.rejects(promise, common.mustCall(handler1)));
-}
 
-promises.push(assert.rejects(
-  assert.doesNotReject(123),
-  {
-    code: 'ERR_INVALID_ARG_TYPE',
-    message: 'The "promiseFn" argument must be one of type ' +
-             'Function or Promise. Received type number'
-  }
-));
-
-{
-  const handler = (generated, actual, err) => {
-    assert.strictEqual(err.generatedMessage, generated);
-    assert.strictEqual(err.code, 'ERR_ASSERTION');
-    assert.strictEqual(err.actual, actual);
-    assert.strictEqual(err.operator, 'rejects');
-    assert(/rejects/.test(err.stack));
-    return true;
-  };
-  const err = new Error();
   promises.push(assert.rejects(
-    assert.rejects(Promise.reject(null), { code: 'FOO' }),
-    handler.bind(null, true, null)
-  ));
-  promises.push(assert.rejects(
-    assert.rejects(Promise.reject(5), { code: 'FOO' }, 'AAAAA'),
-    handler.bind(null, false, 5)
-  ));
-  promises.push(assert.rejects(
-    assert.rejects(Promise.reject(err), { code: 'FOO' }, 'AAAAA'),
-    handler.bind(null, false, err)
+    assert.doesNotReject(123),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: 'The "promiseFn" argument must be one of type ' +
+               'Function or Promise. Received type number'
+    }
   ));
+  /* eslint-enable no-restricted-syntax */
 }
 
 // Make sure all async code gets properly executed.

diff --git a/test/parallel/test-fs-readdir-types.js b/test/parallel/test-fs-readdir-types.js
@@ -72,13 +72,12 @@ fs.readdir(readdirDir, {
   assertDirents(dirents);
 }));
 
-// Check the promisified version
-assert.doesNotReject(async () => {
+(async () => {
   const dirents = await fs.promises.readdir(readdirDir, {
     withFileTypes: true
   });
   assertDirents(dirents);
-});
+})();
 
 // Check for correct types when the binding returns unknowns
 const UNKNOWN = constants.UV_DIRENT_UNKNOWN;