[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');
 constdescriptor=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){