test: expand linting rules around assert w literal messages

Expands the existing restrictions around not using literal messages to the `not` variants of `strictEqual` and `deepStrictEqual`, and forbids `assert()` where the second argument is a non-string literal, which almost always is a mis-typed `assert.strictEquals()`. PR-URL: #59147 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

Author: addaleax

test/addons/report-fatalerror/test.js

@@ -27,7 +27,7 @@ const ARGS = [
  tmpdir.refresh();
  const args = ['--report-on-fatalerror', ...ARGS];
  const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
- assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
+ assert.notStrictEqual(child.status, 0);
 
  const reports = helper.findReports(child.pid, tmpdir.path);
  assert.strictEqual(reports.length, 1);
@@ -46,7 +46,7 @@ const ARGS = [
  // Verify that --report-on-fatalerror is respected when not set.
  const args = ARGS;
  const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
- assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
+ assert.notStrictEqual(child.status, 0);
  const reports = helper.findReports(child.pid, tmpdir.path);
  assert.strictEqual(reports.length, 0);
 }

test/eslint.config_partial.mjs

@@ -36,6 +36,10 @@ export default [
  selector: "CallExpression:matches([callee.name='deepStrictEqual'], [callee.property.name='deepStrictEqual'])[arguments.2.type='Literal']",
  message: 'Do not use a literal for the third argument of assert.deepStrictEqual()',
  },
+ {
+ selector: "CallExpression:matches([callee.name='notDeepStrictEqual'], [callee.property.name='deepStrictEqual'])[arguments.2.type='Literal']",
+ message: 'Do not use a literal for the third argument of assert.notDeepStrictEqual()',
+ },
  {
  selector: "CallExpression:matches([callee.name='doesNotThrow'], [callee.property.name='doesNotThrow'])",
  message: 'Do not use `assert.doesNotThrow()`. Write the code without the wrapper and add a comment instead.',
@@ -48,10 +52,18 @@ export default [
  selector: "CallExpression:matches([callee.name='rejects'], [callee.property.name='rejects'])[arguments.length<2]",
  message: '`assert.rejects()` must be invoked with at least two arguments.',
  },
+ {
+ selector: "CallExpression[callee.property.name='notStrictEqual'][arguments.2.type='Literal']",
+ message: 'Do not use a literal for the third argument of assert.notStrictEqual()',
+ },
  {
  selector: "CallExpression[callee.property.name='strictEqual'][arguments.2.type='Literal']",
  message: 'Do not use a literal for the third argument of assert.strictEqual()',
  },
+ {
+ selector: "CallExpression[callee.name='assert'][arguments.1.type='Literal']:not([arguments.1.raw=/['\"`].*/])",
+ message: 'Do not use a non-string literal for the second argument of assert()',
+ },
  {
  selector: "CallExpression:matches([callee.name='throws'], [callee.property.name='throws'])[arguments.1.type='Literal']:not([arguments.1.regex])",
  message: 'Use an object as second argument of `assert.throws()`.',

test/js-native-api/test_object/test.js

@@ -140,7 +140,7 @@ assert.strictEqual(newObject.test_string, 'test string');
  test_object.Wrap(wrapper);
 
  assert(test_object.Unwrap(wrapper));
- assert(wrapper.protoA);
+ assert.strictEqual(wrapper.protoA, true);
 }
 
 {
@@ -155,8 +155,8 @@ assert.strictEqual(newObject.test_string, 'test string');
  Object.setPrototypeOf(wrapper, protoB);
 
  assert(test_object.Unwrap(wrapper));
- assert(wrapper.protoA, true);
- assert(wrapper.protoB, true);
+ assert.strictEqual(wrapper.protoA, true);
+ assert.strictEqual(wrapper.protoB, true);
 }
 
 {

test/parallel/test-buffer-write-fast.js

@@ -41,5 +41,5 @@ testFastUtf8Write();
 
 if (common.isDebug) {
  const { getV8FastApiCallCount } = internalBinding('debug');
- assert(getV8FastApiCallCount('buffer.writeString'), 4);
+ assert.strictEqual(getV8FastApiCallCount('buffer.writeString'), 4);
 }

test/parallel/test-net-listen-exclusive-random-ports.js

@@ -16,7 +16,7 @@ if (cluster.isPrimary) {
  worker2.on('message', function(port2) {
  assert.strictEqual(port2, port2 | 0,
  `second worker could not listen on port ${port2}`);
- assert.notStrictEqual(port1, port2, 'ports should not be equal');
+ assert.notStrictEqual(port1, port2);
  worker1.kill();
  worker2.kill();
  });

test/parallel/test-stream-toArray.js

@@ -59,7 +59,7 @@ const assert = require('assert');
  const ac = new AbortController();
  let stream;
  assert.rejects(async () => {
- stream = Readable.from([1, 2, 3]).map(async (x) => {
+ stream = Readable.from([1, 2, 3, 4]).map(async (x) => {
  if (x === 3) {
  await new Promise(() => {}); // Explicitly do not pass signal here
  }
@@ -69,8 +69,8 @@ const assert = require('assert');
  }, {
  name: 'AbortError',
  }).then(common.mustCall(() => {
- // Only stops toArray, does not destroy the stream
- assert(stream.destroyed, false);
+ // Stops toArray *and* destroys the stream
+ assert.strictEqual(stream.destroyed, true);
  }));
  ac.abort();
 }

test/report/test-report-fatalerror-oomerror-compact.js

@@ -25,7 +25,7 @@ const REPORT_FIELDS = [
  tmpdir.refresh();
  const args = ['--report-on-fatalerror', '--report-compact', ...ARGS];
  const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
- assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
+ assert.notStrictEqual(child.status, 0);
 
  const reports = helper.findReports(child.pid, tmpdir.path);
  assert.strictEqual(reports.length, 1);

test/report/test-report-fatalerror-oomerror-directory.js

@@ -27,7 +27,7 @@ const REPORT_FIELDS = [
  const dir = '--report-directory=' + tmpdir.path;
  const args = ['--report-on-fatalerror', dir, ...ARGS];
  const child = spawnSync(process.execPath, args, { });
- assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
+ assert.notStrictEqual(child.status, 0);
 
  const reports = helper.findReports(child.pid, tmpdir.path);
  assert.strictEqual(reports.length, 1);

test/report/test-report-fatalerror-oomerror-filename.js

@@ -30,7 +30,7 @@ const REPORT_FIELDS = [
  ...ARGS,
  ];
  const child = spawnSync(process.execPath, args, { encoding: 'utf8' });
- assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
+ assert.notStrictEqual(child.status, 0);
 
  const reports = helper.findReports(child.pid, tmpdir.path);
  assert.strictEqual(reports.length, 0);

test/report/test-report-fatalerror-oomerror-not-set.js

@@ -20,7 +20,7 @@ const ARGS = [
  // Verify that --report-on-fatalerror is respected when not set.
  const args = ARGS;
  const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
- assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
+ assert.notStrictEqual(child.status, 0);
  const reports = helper.findReports(child.pid, tmpdir.path);
  assert.strictEqual(reports.length, 0);
 }