lib,test: improve faulty assert usage detection

.eslintrc.js

@@ -170,32 +170,32 @@ module.exports = {
         message: '__defineSetter__ is deprecated.',
       },
     ],
-    // If this list is modified, please copy the change to lib/.eslintrc.yaml
-    // and test/.eslintrc.yaml.
+    // If this list is modified, please copy changes that should apply to ./lib
+    // as well to lib/.eslintrc.yaml.
     'no-restricted-syntax': [
       'error',
       {
-        selector: "CallExpression[callee.object.name='assert'][callee.property.name='deepStrictEqual'][arguments.2.type='Literal']",
+        selector: "CallExpression[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']",
+        selector: "CallExpression[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]",
+        selector: "CallExpression[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']",
+        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.object.name='assert'][callee.property.name='throws'][arguments.1.type='Literal']:not([arguments.1.regex])",
+        selector: "CallExpression[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]",
+        selector: "CallExpression[callee.property.name='throws'][arguments.length<2]",
         message: '`assert.throws()` must be invoked with at least two arguments.',
       },
       {
@@ -210,6 +210,38 @@ module.exports = {
         selector: 'ThrowStatement > CallExpression[callee.name=/Error$/]',
         message: 'Use `new` keyword when throwing an `Error`.',
       },
+      {
+        selector: "CallExpression[callee.property.name='notDeepStrictEqual'][arguments.length<2]",
+        message: 'assert.notDeepStrictEqual() must be invoked with at least two arguments.',
+      },
+      {
+        selector: "CallExpression[callee.property.name='deepStrictEqual'][arguments.length<2]",
+        message: 'assert.deepStrictEqual() must be invoked with at least two arguments.',
+      },
+      {
+        selector: "CallExpression[callee.property.name='notStrictEqual'][arguments.length<2]",
+        message: 'assert.notStrictEqual() must be invoked with at least two arguments.',
+      },
+      {
+        selector: "CallExpression[callee.property.name='strictEqual'][arguments.length<2]",
+        message: 'assert.strictEqual() must be invoked with at least two arguments.',
+      },
+      {
+        selector: "CallExpression[callee.property.name='notDeepStrictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression']):not([arguments.1.type='UnaryExpression'])",
+        message: 'The first argument should be the `actual`, not the `expected` value.',
+      },
+      {
+        selector: "CallExpression[callee.property.name='notStrictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression']):not([arguments.1.type='UnaryExpression'])",
+        message: 'The first argument should be the `actual`, not the `expected` value.',
+      },
+      {
+        selector: "CallExpression[callee.property.name='deepStrictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression']):not([arguments.1.type='UnaryExpression'])",
+        message: 'The first argument should be the `actual`, not the `expected` value.',
+      },
+      {
+        selector: "CallExpression[callee.property.name='strictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression']):not([arguments.1.type='UnaryExpression'])",
+        message: 'The first argument should be the `actual`, not the `expected` value.',
+      }
     ],
     /* eslint-enable max-len */
     'no-return-await': 'error',

lib/.eslintrc.yaml

@@ -4,18 +4,8 @@ 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]"
-      message: "assert.throws() must be invoked with at least two arguments."
+    - selector: "CallExpression[callee.object.name='assert']:not([callee.property.name='ok']):not([callee.property.name='fail']):not([callee.property.name='ifError'])"
+      message: "Please only use simply assertions in ./lib"
     - selector: "CallExpression[callee.name='setTimeout'][arguments.length<2]"
       message: "setTimeout() must be invoked with at least two arguments."
     - selector: "CallExpression[callee.name='setInterval'][arguments.length<2]"

lib/internal/js_stream_socket.js

@@ -1,6 +1,6 @@
 'use strict';
 
-const assert = require('assert');
+const assert = require('internal/assert');
 const util = require('util');
 const { Socket } = require('net');
 const { JSStream } = internalBinding('js_stream');
@@ -122,8 +122,8 @@ class JSStreamSocket extends Socket {
       this[kPendingShutdownRequest] = req;
       return 0;
     }
-    assert.strictEqual(this[kCurrentWriteRequest], null);
-    assert.strictEqual(this[kCurrentShutdownRequest], null);
+    assert(this[kCurrentWriteRequest] === null);
+    assert(this[kCurrentShutdownRequest] === null);
     this[kCurrentShutdownRequest] = req;
 
     const handle = this._handle;
@@ -148,8 +148,8 @@ class JSStreamSocket extends Socket {
   }
 
   doWrite(req, bufs) {
-    assert.strictEqual(this[kCurrentWriteRequest], null);
-    assert.strictEqual(this[kCurrentShutdownRequest], null);
+    assert(this[kCurrentWriteRequest] === null);
+    assert(this[kCurrentShutdownRequest] === null);
 
     const handle = this._handle;
     const self = this;
@@ -214,7 +214,7 @@ class JSStreamSocket extends Socket {
 
     setImmediate(() => {
       // Should be already set by net.js
-      assert.strictEqual(this._handle, null);
+      assert(this._handle === null);
 
       this.finishWrite(handle, uv.UV_ECANCELED);
       this.finishShutdown(handle, uv.UV_ECANCELED);

test/.eslintrc.yaml

@@ -20,36 +20,6 @@ rules:
   node-core/required-modules: [error, common]
   node-core/no-duplicate-requires: off
 
-  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]"
-      message: "assert.throws() must be invoked with at least two arguments."
-    - selector: "CallExpression[callee.name='setTimeout'][arguments.length<2]"
-      message: "setTimeout() must be invoked with at least two arguments."
-    - selector: "CallExpression[callee.name='setInterval'][arguments.length<2]"
-      message: "setInterval() must be invoked with at least 2 arguments."
-    - selector: "ThrowStatement > CallExpression[callee.name=/Error$/]"
-      message: "Use new keyword when throwing an Error."
-    # Specific to /test
-    - selector: "CallExpression[callee.object.name='assert'][callee.property.name='notDeepStrictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression'])"
-      message: "The first argument should be the `actual`, not the `expected` value."
-    - selector: "CallExpression[callee.object.name='assert'][callee.property.name='notStrictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression'])"
-      message: "The first argument should be the `actual`, not the `expected` value."
-    - selector: "CallExpression[callee.object.name='assert'][callee.property.name='deepStrictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression'])"
-      message: "The first argument should be the `actual`, not the `expected` value."
-    - selector: "CallExpression[callee.object.name='assert'][callee.property.name='strictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression'])"
-      message: "The first argument should be the `actual`, not the `expected` value."
 # Global scoped methods and vars
 globals:
   WebAssembly: false

test/parallel/test-assert.js

@@ -65,6 +65,7 @@ assert.throws(() => a.notEqual(true, true),
 assert.throws(() => a.strictEqual(2, '2'),
               a.AssertionError, 'strictEqual(2, \'2\')');
 
+/* eslint-disable no-restricted-syntax */
 assert.throws(() => a.strictEqual(null, undefined),
               a.AssertionError, 'strictEqual(null, undefined)');
 
@@ -95,11 +96,9 @@ function thrower(errorConstructor) {
 // The basic calls work.
 assert.throws(() => thrower(a.AssertionError), a.AssertionError, 'message');
 assert.throws(() => thrower(a.AssertionError), a.AssertionError);
-// eslint-disable-next-line no-restricted-syntax
 assert.throws(() => thrower(a.AssertionError));
 
 // If not passing an error, catch all.
-// eslint-disable-next-line no-restricted-syntax
 assert.throws(() => thrower(TypeError));
 
 // When passing a type, only catch errors of the appropriate type.
@@ -309,7 +308,7 @@ try {
 }
 
 try {
-  assert.strictEqual(1, 2, 'oh no'); // eslint-disable-line no-restricted-syntax
+  assert.strictEqual(1, 2, 'oh no');
 } catch (e) {
   assert.strictEqual(e.message, 'oh no');
   // Message should not be marked as generated.
@@ -833,7 +832,6 @@ common.expectsError(
 );
 
 common.expectsError(
-  // eslint-disable-next-line no-restricted-syntax
   () => assert.throws(() => {}, 'Error message', 'message'),
   {
     code: 'ERR_INVALID_ARG_TYPE',
@@ -1010,6 +1008,7 @@ assert.throws(
              'The error message "foo" is identical to the message.'
   }
 );
+/* eslint-enable no-restricted-syntax */
 
 // Should not throw.
 // eslint-disable-next-line no-restricted-syntax, no-throw-literal