[v10.x backport] tools, test: forbid string literal as third argument for assert.strictEqual() and friends

.eslintrc.js

@@ -156,6 +156,7 @@ module.exports = {
     ],
     /* eslint-disable max-len */
     // If this list is modified, please copy the change to lib/.eslintrc.yaml
+    // and test/.eslintrc.yaml.
     'no-restricted-syntax': [
       'error',
       {
@@ -166,6 +167,10 @@ module.exports = {
         selector: "CallExpression[callee.object.name='assert'][callee.property.name='rejects'][arguments.length<2]",
         message: 'assert.rejects() must be invoked with at least two arguments.',
       },
+      {
+        selector: "CallExpression[callee.object.name='assert'][callee.property.name='strictEqual'][arguments.2.type='Literal']",
+        message: 'Do not use a literal for the third argument of assert.strictEqual()'
+      },
       {
         selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.1.type='Literal']:not([arguments.1.regex])",
         message: 'Use an object as second argument of assert.throws()',

lib/.eslintrc.yaml

@@ -2,10 +2,14 @@ rules:
   no-restricted-syntax:
     # Config copied from .eslintrc.js
     - error
+    - selector: "CallExpression[callee.object.name='assert'][callee.property.name='deepStrictEqual'][arguments.2.type='Literal']"
+      message: "Do not use a literal for the third argument of assert.deepStrictEqual()"
     - 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='rejects'][arguments.length<2]"
       message: "assert.rejects() must be invoked with at least two arguments."
+    - selector: "CallExpression[callee.object.name='assert'][callee.property.name='strictEqual'][arguments.2.type='Literal']"
+      message: "Do not use a literal for the third argument of assert.strictEqual()"
     - selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.1.type='Literal']:not([arguments.1.regex])"
       message: "Use an object as second argument of assert.throws()"
     - selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.length<2]"

test/.eslintrc.yaml

@@ -23,10 +23,14 @@ rules:
   no-restricted-syntax:
     # Config copied from .eslintrc.js
     - error
+    - selector: "CallExpression[callee.object.name='assert'][callee.property.name='deepStrictEqual'][arguments.2.type='Literal']"
+      message: "Do not use a literal for the third argument of assert.deepStrictEqual()"
     - 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='rejects'][arguments.length<2]"
       message: "assert.rejects() must be invoked with at least two arguments."
+    - selector: "CallExpression[callee.object.name='assert'][callee.property.name='strictEqual'][arguments.2.type='Literal']"
+      message: "Do not use a literal for the third argument of assert.strictEqual()"
     - selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.1.type='Literal']:not([arguments.1.regex])"
       message: "Use an object as second argument of assert.throws()"
     - selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.length<2]"

test/addons-napi/test_general/testFinalizer.js

@@ -29,5 +29,4 @@ test_general.wrap(finalizeAndWrap);
 test_general.addFinalizerOnly(finalizeAndWrap, common.mustCall());
 finalizeAndWrap = null;
 global.gc();
-assert.strictEqual(test_general.derefItemWasCalled(), true,
-                   'finalize callback was called');
+assert.strictEqual(test_general.derefItemWasCalled(), true);

test/async-hooks/test-async-await.js

@@ -64,15 +64,15 @@ const timeout = common.platformTimeout(10);
 
 function checkPromisesInitState() {
   for (const initState of promisesInitState.values()) {
-    assert.strictEqual(initState, 'resolved',
-                       'promise initialized without being resolved');
+    // Promise should not be initialized without being resolved.
+    assert.strictEqual(initState, 'resolved');
   }
 }
 
 function checkPromisesExecutionState() {
   for (const executionState of promisesExecutionState.values()) {
-    assert.strictEqual(executionState, 'after',
-                       'mismatch between before and after hook calls');
+    // Check for mismatch between before and after hook calls.
+    assert.strictEqual(executionState, 'after');
   }
 }
 

test/parallel/test-assert.js

@@ -304,11 +304,11 @@ try {
 }
 
 try {
-  assert.strictEqual(1, 2, 'oh no');
+  assert.strictEqual(1, 2, 'oh no'); // eslint-disable-line no-restricted-syntax
 } catch (e) {
   assert.strictEqual(e.message, 'oh no');
-  assert.strictEqual(e.generatedMessage, false,
-                     'Message incorrectly marked as generated');
+  // Message should not be marked as generated.
+  assert.strictEqual(e.generatedMessage, false);
 }
 
 {

test/parallel/test-dns-lookup.js

@@ -134,8 +134,8 @@ dns.lookup('example.com', common.mustCall((error, result, addressType) => {
   assert.strictEqual(tickValue, 1);
   assert.strictEqual(error.code, 'ENOENT');
   const descriptor = Object.getOwnPropertyDescriptor(error, 'message');
-  assert.strictEqual(descriptor.enumerable,
-                     false, 'The error message should be non-enumerable');
+  // The error message should be non-enumerable.
+  assert.strictEqual(descriptor.enumerable, false);
 }));
 
 // Make sure that the error callback is called

test/parallel/test-dns-resolveany-bad-ancount.js

@@ -40,8 +40,8 @@ server.bind(0, common.mustCall(async () => {
     assert.strictEqual(err.syscall, 'queryAny');
     assert.strictEqual(err.hostname, 'example.org');
     const descriptor = Object.getOwnPropertyDescriptor(err, 'message');
-    assert.strictEqual(descriptor.enumerable,
-                       false, 'The error message should be non-enumerable');
+    // The error message should be non-enumerable.
+    assert.strictEqual(descriptor.enumerable, false);
     server.close();
   }));
 }));

test/parallel/test-fs-readfile.js

@@ -54,6 +54,6 @@ for (const e of fileInfo) {
   fs.readFile(e.name, common.mustCall((err, buf) => {
     console.log(`Validating readFile on file ${e.name} of length ${e.len}`);
     assert.ifError(err);
-    assert.deepStrictEqual(buf, e.contents, 'Incorrect file contents');
+    assert.deepStrictEqual(buf, e.contents);
   }));
 }

test/parallel/test-http-information-processing.js

@@ -36,8 +36,8 @@ server.listen(0, function() {
   });
 
   req.on('response', function(res) {
-    assert.strictEqual(countdown.remaining, 1,
-                       'Full response received before all 102 Processing');
+    // Check that all 102 Processing received before full response received.
+    assert.strictEqual(countdown.remaining, 1);
     assert.strictEqual(200, res.statusCode,
                        `Final status code was ${res.statusCode}, not 200.`);
     res.setEncoding('utf8');

test/parallel/test-next-tick-domain.js

@@ -27,5 +27,5 @@ const origNextTick = process.nextTick;
 
 require('domain');
 
-assert.strictEqual(origNextTick, process.nextTick,
-                   'Requiring domain should not change nextTick');
+// Requiring domain should not change nextTick.
+assert.strictEqual(origNextTick, process.nextTick);

test/parallel/test-timers-same-timeout-wrong-list-deleted.js

@@ -37,7 +37,7 @@ const handle1 = setTimeout(common.mustCall(function() {
 
         // Make sure our clearTimeout succeeded. One timer finished and
         // the other was canceled, so none should be active.
-        assert.strictEqual(activeTimers.length, 0, 'Timers remain.');
+        assert.strictEqual(activeTimers.length, 0);
       }));
     }));
   }), 1);
@@ -53,11 +53,9 @@ const handle1 = setTimeout(common.mustCall(function() {
 
   // Make sure our clearTimeout succeeded. One timer finished and
   // the other was canceled, so none should be active.
-  assert.strictEqual(activeTimers.length, 3,
-                     'There should be 3 timers in the list.');
+  assert.strictEqual(activeTimers.length, 3);
   assert(shortTimer instanceof Timer, 'The shorter timer is not in the list.');
-  assert.strictEqual(longTimers.length, 2,
-                     'Both longer timers should be in the list.');
+  assert.strictEqual(longTimers.length, 2);
 
   // When this callback completes, `listOnTimeout` should now look at the
   // correct list and refrain from removing the new TIMEOUT list which

test/parallel/test-timers-unref-reuse-no-exposed-list.js

@@ -4,11 +4,9 @@ require('../common');
 const assert = require('assert');
 
 const timer1 = setTimeout(() => {}, 1).unref();
-assert.strictEqual(timer1._handle._list, undefined,
-                   'timer1._handle._list should be undefined');
+assert.strictEqual(timer1._handle._list, undefined);
 
 // Check that everything works even if the handle was not re-used.
 setTimeout(() => {}, 1);
 const timer2 = setTimeout(() => {}, 1).unref();
-assert.strictEqual(timer2._handle._list, undefined,
-                   'timer2._handle._list should be undefined');
+assert.strictEqual(timer2._handle._list, undefined);

test/parallel/test-vm-run-in-new-context.js

@@ -26,8 +26,8 @@ const common = require('../common');
 const assert = require('assert');
 const vm = require('vm');
 
-assert.strictEqual(typeof global.gc, 'function',
-                   'Run this test with --expose-gc');
+if (typeof global.gc !== 'function')
+  assert.fail('Run this test with --expose-gc');
 
 // Run a string
 const result = vm.runInNewContext('\'passed\';');

test/sequential/test-http2-timeout-large-write-file.js

@@ -48,7 +48,7 @@ server.on('stream', common.mustCall((stream) => {
 }));
 server.setTimeout(serverTimeout);
 server.on('timeout', () => {
-  assert.strictEqual(didReceiveData, false, 'Should not timeout');
+  assert.ok(!didReceiveData, 'Should not timeout');
 });
 
 server.listen(0, common.mustCall(() => {

test/sequential/test-http2-timeout-large-write.js

@@ -40,13 +40,13 @@ server.on('stream', common.mustCall((stream) => {
   stream.write(content);
   stream.setTimeout(serverTimeout);
   stream.on('timeout', () => {
-    assert.strictEqual(didReceiveData, false, 'Should not timeout');
+    assert.ok(!didReceiveData, 'Should not timeout');
   });
   stream.end();
 }));
 server.setTimeout(serverTimeout);
 server.on('timeout', () => {
-  assert.strictEqual(didReceiveData, false, 'Should not timeout');
+  assert.ok(!didReceiveData, 'Should not timeout');
 });
 
 server.listen(0, common.mustCall(() => {

test/sequential/test-inspector.js

@@ -33,8 +33,7 @@ function checkBadPath(err) {
 }
 
 function checkException(message) {
-  assert.strictEqual(message.exceptionDetails, undefined,
-                     'An exception occurred during execution');
+  assert.strictEqual(message.exceptionDetails, undefined);
 }
 
 function assertScopeValues({ result }, expected) {