lib,test: enforce use of assert.fail via a lint rule

PR-URL: https://github.com/nodejs/node/pull/61004
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>

doc/api/assert.md

@@ -1682,6 +1682,8 @@ If no arguments are passed in at all `message` will be set to the string:
 Be aware that in the `repl` the error message will be different to the one
 thrown in a file! See below for further details.
 
+<!-- eslint-skip -->
+
 ```mjs
 import assert from 'node:assert/strict';
 
@@ -1717,6 +1719,8 @@ assert.ok(0);
 //   assert.ok(0)
 ```
 
+<!-- eslint-skip -->
+
 ```cjs
 const assert = require('node:assert/strict');
 
@@ -1756,20 +1760,20 @@ assert.ok(0);
 import assert from 'node:assert/strict';
 
 // Using `assert()` works the same:
-assert(0);
+assert(2 + 2 > 5);
 // AssertionError: The expression evaluated to a falsy value:
 //
-//   assert(0)
+//   assert(2 + 2 > 5)
 ```
 
 ```cjs
 const assert = require('node:assert');
 
 // Using `assert()` works the same:
-assert(0);
+assert(2 + 2 > 5);
 // AssertionError: The expression evaluated to a falsy value:
 //
-//   assert(0)
+//   assert(2 + 2 > 5)
 ```
 
 ## `assert.rejects(asyncFn[, error][, message])`

doc/api/test.md

@@ -1786,7 +1786,7 @@ This function creates a hook that runs before executing a suite.
 describe('tests', async () => {
   before(() => console.log('about to run some test'));
   it('is a subtest', () => {
-    assert.ok('some relevant assertion here');
+    // Some relevant assertions here
   });
 });
 ```
@@ -1816,7 +1816,7 @@ This function creates a hook that runs after executing a suite.
 describe('tests', async () => {
   after(() => console.log('finished running tests'));
   it('is a subtest', () => {
-    assert.ok('some relevant assertion here');
+    // Some relevant assertion here
   });
 });
 ```
@@ -1849,7 +1849,7 @@ This function creates a hook that runs before each test in the current suite.
 describe('tests', async () => {
   beforeEach(() => console.log('about to run a test'));
   it('is a subtest', () => {
-    assert.ok('some relevant assertion here');
+    // Some relevant assertion here
   });
 });
 ```
@@ -1880,7 +1880,7 @@ The `afterEach()` hook is run even if the test fails.
 describe('tests', async () => {
   afterEach(() => console.log('finished running a test'));
   it('is a subtest', () => {
-    assert.ok('some relevant assertion here');
+    // Some relevant assertion here
   });
 });
 ```
@@ -3472,7 +3472,7 @@ test('top level test', async (t) => {
   await t.test(
     'This is a subtest',
     (t) => {
-      assert.ok('some relevant assertion here');
+      // Some relevant assertion here
     },
   );
 });
@@ -3503,7 +3503,7 @@ finishes.
 ```js
 test('top level test', async (t) => {
   t.after((t) => t.diagnostic(`finished running ${t.name}`));
-  assert.ok('some relevant assertion here');
+  // Some relevant assertion here
 });
 ```
 
@@ -3535,7 +3535,7 @@ test('top level test', async (t) => {
   await t.test(
     'This is a subtest',
     (t) => {
-      assert.ok('some relevant assertion here');
+      // Some relevant assertion here
     },
   );
 });

lib/internal/crypto/cipher.js

@@ -97,7 +97,7 @@ function getDecoder(decoder, encoding) {
     if (normalizedEncoding === undefined) {
       throw new ERR_UNKNOWN_ENCODING(encoding);
     }
-    assert(false, 'Cannot change encoding');
+    assert.fail('Cannot change encoding');
   }
   return decoder;
 }

test/abort/test-abort-fatal-error.js

@@ -36,7 +36,7 @@ exec(...cmdline, common.mustCall((err, stdout, stderr) => {
   if (!err) {
     console.log(stdout);
     console.log(stderr);
-    assert(false, 'this test should fail');
+    assert.fail('this test should fail');
   }
 
   assert(common.nodeProcessAborted(err.code, err.signal));

test/message/internal_assert.js

@@ -4,4 +4,4 @@
 require('../common');
 
 const assert = require('internal/assert');
-assert(false);
+assert(2 + 2 > 5);

test/parallel/test-cluster-bind-twice.js

@@ -102,7 +102,7 @@ if (!id) {
     }));
   }, 2));
 } else {
-  assert(0); // Bad command line argument
+  assert.fail('Bad command line argument');
 }
 
 function startWorker() {

test/parallel/test-cluster-eaddrinuse.js

@@ -58,5 +58,5 @@ if (id === 'undefined') {
     }));
   }));
 } else {
-  assert(0);  // Bad argument.
+  assert.fail('Bad argument');
 }

test/parallel/test-fastutf8stream-sync.js

@@ -181,10 +181,6 @@ assert.throws(() => {
   assert.ok(stream.write('hello world 👀\n'));
   assert.ok(stream.write('another line 👀\n'));
 
-  // Check internal buffer length (may not be available in Utf8Stream)
-  // This is implementation-specific, so we just verify writes succeeded
-  assert.ok(true, 'writes completed successfully');
-
   stream.end();
 }
 
@@ -200,7 +196,6 @@ assert.throws(() => {
   }
 
   // Check internal buffer length (implementation-specific)
-  assert.ok(true, 'writes completed successfully');
   readFile(dest, 'utf8', common.mustSucceed((data) => {
     assert.strictEqual(data, str);
   }));

test/parallel/test-fs-glob.mjs

@@ -529,7 +529,6 @@ describe('glob - with restricted directory', function() {
       for await (const match of asyncGlob('*', { cwd: restrictedDir })) {
         results.push(match);
       }
-      assert.ok(true, 'glob completed without throwing on readdir error');
     } finally {
       try {
         chmodSync(restrictedDir, 0o755);

test/parallel/test-http.js

@@ -46,7 +46,7 @@ const server = http.Server(common.mustCall((req, res) => {
       assert.strictEqual(req.headers.cookie, 'abc=123; def=456; ghi=789');
       break;
     default:
-      assert(false, `Unexpected request for ${req.url}`);
+      assert.fail(`Unexpected request for ${req.url}`);
   }
 
   if (expectedRequests.length === 0)

test/parallel/test-inspector-network-http2.js

@@ -96,7 +96,7 @@ const handleStream = common.mustCallAtLeast((stream, headers) => {
       }));
       break;
     default:
-      assert(false, `Unexpected path: ${path}`);
+      assert.fail(`Unexpected path: ${path}`);
   }
 });
 

test/parallel/test-stream-duplex-readable-writable.js

@@ -39,7 +39,7 @@ const assert = require('assert');
   duplex.on('end', common.mustNotCall());
   async function run() {
     for await (const chunk of duplex) {
-      assert(false, chunk);
+      assert.fail(chunk);
     }
   }
   run().then(common.mustCall());

test/parallel/test-vm-module-basic.js

@@ -52,7 +52,7 @@ const util = require('util');
   const m = new SourceTextModule('while (true) {}');
   await m.link(common.mustNotCall());
   await m.evaluate({ timeout: 500 })
-    .then(() => assert(false), () => {});
+    .then(() => assert.fail(), () => {});
 })().then(common.mustCall());
 
 // Check the generated identifier for each module

test/pummel/test-fs-watch-file-slow.js

@@ -53,7 +53,7 @@ fs.watchFile(FILENAME, { interval: TIMEOUT - 250 }, common.mustCall((curr, prev)
       fs.unwatchFile(FILENAME);
       break;
     default:
-      assert(0);
+      assert.fail();
   }
 }, 4));
 

test/system-ca/test-set-default-ca-certificates-override-system.mjs

@@ -45,7 +45,7 @@ const server = https.createServer({
       res.end('bundled ca works');
       break;
     default:
-      assert(false, `Unexpected path: ${path}`);
+      assert.fail(`Unexpected path: ${path}`);
   }
 }, 1));
 

tools/eslint/eslint.config_utils.mjs

@@ -20,6 +20,13 @@ export const noRestrictedSyntaxCommonAll = [
     selector: "CallExpression[callee.property.name='substr']",
     message: 'Use String.prototype.slice() or String.prototype.substring() instead of String.prototype.substr()',
   },
+  {
+    selector: `CallExpression:matches(${[
+      '[callee.name="assert"]',
+      '[callee.object.name="assert"][callee.property.name="ok"]',
+    ]})[arguments.0.type="Literal"]`,
+    message: 'Do not use a literal for the first argument of assert(), use assert.fail() instead or remove the call',
+  },
 ];
 
 export const noRestrictedSyntaxCommonLib = [