tools: add must-call-assert lint rule

Author: aduh95

test/eslint.config_partial.mjs

@@ -126,6 +126,7 @@ export default [
       'node-core/eslint-check': 'error',
       'node-core/async-iife-no-unused-result': 'error',
       'node-core/inspector-check': 'error',
+      'node-core/must-call-assert': 'error',
       // `common` module is mandatory in tests.
       'node-core/required-modules': [
         'error',
@@ -152,6 +153,7 @@ export default [
           objects: 'only-multiline',
         },
       ],
+      'node-core/must-call-assert': 'off',
     },
   },
   {

test/parallel/test-eslint-must-call-assert.js

@@ -0,0 +1,114 @@
+'use strict';
+const common = require('../common');
+if ((!common.hasCrypto) || (!common.hasIntl)) {
+  common.skip('ESLint tests require crypto and Intl');
+}
+common.skipIfEslintMissing();
+
+const RuleTester = require('../../tools/eslint/node_modules/eslint').RuleTester;
+const rule = require('../../tools/eslint-rules/must-call-assert');
+
+const message = 'Assertions must be wrapped into `common.mustCall` or `common.mustCallAtLeast`';
+
+const tester = new RuleTester();
+tester.run('must-call-assert', rule, {
+  valid: [
+    'assert.strictEqual(2+2, 4)',
+    'process.on("exit", common.mustCallAtLeast((code) => {assert.strictEqual(code, 0)}));',
+    'process.once("exit", common.mustCall((code) => {assert.strictEqual(code, 0)}));',
+    'process.once("exit", common.mustCall((code) => {if(2+2 === 5) { assert.strictEqual(code, 0)} }));',
+    'process.once("exit", common.mustCall((code) => { (() => assert.strictEqual(code, 0))(); }));',
+    '(async () => {await assert.rejects(fun())})().then()',
+    '[1, true].forEach((val) => assert.strictEqual(fun(val), 0));',
+    'const assert = require("node:assert")',
+    'const assert = require("assert")',
+    'const assert = require("assert/strict")',
+    'const assert = require("node:assert/strict")',
+    'import assert from "node:assert"',
+    'import * as assert from "node:assert"',
+    'import assert from "node:assert/strict"',
+    'import * as assert from "node:assert/strict"',
+  ],
+  invalid: [
+    {
+      code: 'process.on("exit", (code) => assert.strictEqual(code, 0))',
+      errors: [{ message }],
+    },
+    {
+      code: 'function test() { process.on("exit", (code) => assert.strictEqual(code, 0)) }',
+      errors: [{ message }],
+    },
+    {
+      code: 'process.once("exit", (code) => {if(2+2 === 5) { assert.strictEqual(code, 0)} });',
+      errors: [{ message }],
+    },
+    {
+      code: 'process.once("exit", (code) => { (() => { assert.strictEqual(code, 0)})(); });',
+      errors: [{ message }],
+    },
+    {
+      code: 'process.once("exit", common.mustCall((code) => {setImmediate(() => { assert.strictEqual(code, 0)}); }));',
+      errors: [{ message }],
+    },
+    {
+      code: 'require("node:assert").strictEqual(2+2, 5)',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'const { strictEqual } = require("node:assert")',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'const { strictEqual } = require("node:assert/strict")',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'const { strictEqual } = require("assert")',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'const { strictEqual } = require("assert/strict")',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'const someOtherName = require("assert")',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'import assert, { strictEqual } from "assert"',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'import * as someOtherName from "assert"',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'import someOtherName from "assert"',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'import "assert"',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'import { strictEqual } from "node:assert"',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'import assert, { strictEqual } from "node:assert"',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'import * as someOtherName from "node:assert"',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'import someOtherName from "node:assert"',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+    {
+      code: 'import "node:assert"',
+      errors: [{ message: 'Only assign `node:assert` to `assert`' }],
+    },
+  ]
+});

test/v8-updates/test-trace-gc-flag.js

@@ -2,7 +2,7 @@
 
 // This test verifies that `--trace-gc` flag is well integrated.
 // We'll check here, that the console outputs gc events properly.
-require('../common');
+const common = require('../common');
 
 const assert = require('assert');
 const { spawnSync } = require('child_process');
@@ -21,10 +21,10 @@ const fixtures = require('../common/fixtures');
   const scavengeRegex = /\bScavenge\b/;
   const eofRegex = /\bMark-Compact\b/;
 
-  lines.forEach((line, index) => {
+  lines.forEach(common.mustCallAtLeast((line, index) => {
     const expected = index !== lines.length - 1 ? scavengeRegex : eofRegex;
     assert.match(line, expected);
-  });
+  }));
 }
 
 /**

test/wasi/test-wasi-stdio.js

@@ -1,7 +1,7 @@
 'use strict';
 const common = require('../common');
 const tmpdir = require('../common/tmpdir');
-const { strictEqual } = require('assert');
+const assert = require('assert');
 const { closeSync, openSync, readFileSync, writeFileSync } = require('fs');
 const { join } = require('path');
 const { WASI } = require('wasi');
@@ -24,10 +24,10 @@ const importObject = { wasi_snapshot_preview1: wasi.wasiImport };
 (async () => {
   const { instance } = await WebAssembly.instantiate(buffer, importObject);
 
-  strictEqual(wasi.start(instance), 0);
+  assert.strictEqual(wasi.start(instance), 0);
   closeSync(stdin);
   closeSync(stdout);
   closeSync(stderr);
-  strictEqual(readFileSync(stdoutFile, 'utf8').trim(), 'x'.repeat(31));
-  strictEqual(readFileSync(stderrFile, 'utf8').trim(), '');
+  assert.strictEqual(readFileSync(stdoutFile, 'utf8').trim(), 'x'.repeat(31));
+  assert.strictEqual(readFileSync(stderrFile, 'utf8').trim(), '');
 })().then(common.mustCall());

test/wasi/test-wasi-worker-terminate.js

@@ -25,10 +25,10 @@ const bytecode = new Uint8Array([
 if (!process.env.HAS_STARTED_WORKER) {
   process.env.HAS_STARTED_WORKER = 1;
   const worker = new Worker(__filename);
-  worker.once('message', (message) => {
+  worker.once('message', common.mustCall((message) => {
     assert.strictEqual(message, 'start');
     setTimeout(() => worker.terminate(), common.platformTimeout(50));
-  });
+  }));
 } else {
   go();
 }

tools/eslint-rules/must-call-assert.js

@@ -0,0 +1,77 @@
+'use strict';
+
+const message =
+  'Assertions must be wrapped into `common.mustCall` or `common.mustCallAtLeast`';
+
+
+const requireCall = 'CallExpression[callee.name="require"]';
+const assertModuleSpecifier = '/^(node:)?assert(.strict)?$/';
+
+function findEnclosingFunction(node) {
+  while (node) {
+    node = node.parent;
+    if (!node) break;
+
+    if (node.type === 'ArrowFunctionExpression' || node.type === 'FunctionExpression') {
+      // We want to exit the loop only if it's not an IIFE nor a `[].forEach` call.
+      if (node.parent.type !== 'CallExpression' || (
+        node.parent.callee !== node && // IIFE
+        !(
+          node.parent.callee.type === 'MemberExpression' &&
+          node.parent.callee.object.type === 'ArrayExpression' &&
+          node.parent.callee.property.name === 'forEach'
+        )
+      )) {
+        break;
+      }
+    }
+  }
+  return node;
+}
+
+function isMustCallOrMustCallAtLeast(str) {
+  return str === 'mustCall' || str === 'mustCallAtLeast';
+}
+
+function isInMustCallFunction(node) {
+  const parent = findEnclosingFunction(node)?.parent;
+  if (!parent) return true;
+  return (
+    parent.type === 'CallExpression' &&
+    (
+      parent.callee.type === 'MemberExpression' ?
+        parent.callee.object.name === 'common' && isMustCallOrMustCallAtLeast(parent.callee.property.name) :
+        parent.callee.type === 'Identifier' && isMustCallOrMustCallAtLeast(parent.callee.name)
+    )
+  );
+}
+
+module.exports = {
+  meta: {
+    fixable: 'code',
+  },
+  create: function(context) {
+    return {
+      ':function CallExpression[callee.object.name="assert"]': (node) => {
+        if (!isInMustCallFunction(node)) {
+          context.report({
+            node,
+            message,
+          });
+        }
+      },
+
+      [[
+        `:not(VariableDeclarator[id.name="assert"])>${requireCall}[arguments.0.value=${assertModuleSpecifier}]`,
+        `ImportDeclaration[source.value=${assertModuleSpecifier}]:not(
+          [specifiers.length=1][specifiers.0.type=/^Import(Default|Namespace)Specifier$/][specifiers.0.local.name="assert"]
+        )`,
+      ].join(',')]: (node) => {
+        context.report({
+          node,
+          message: 'Only assign `node:assert` to `assert`',
+        });
+      },
+    };
+  },
+};