tools: add eslint rule for hasCrypto checking

test/.eslintrc.yaml

@@ -10,5 +10,7 @@ rules:
   prefer-assert-iferror: error
   prefer-assert-methods: error
   prefer-common-mustnotcall: error
+  crypto-check: error
+  inspector-check: error
   ## common module is mandatory in tests
   required-modules: [error, common]

test/common/index.js

@@ -19,7 +19,7 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-/* eslint-disable required-modules */
+/* eslint-disable required-modules, crypto-check */
 'use strict';
 const path = require('path');
 const fs = require('fs');

test/parallel/test-async-wrap-getasyncid.js

@@ -81,14 +81,14 @@ function testInitialized(req, ctor_name) {
 }
 
 
-if (common.hasCrypto) {
+if (common.hasCrypto) { // eslint-disable-line crypto-check
   const tls = require('tls');
   // SecurePair
   testInitialized(tls.createSecurePair().ssl, 'Connection');
 }
 
 
-if (common.hasCrypto) {
+if (common.hasCrypto) { // eslint-disable-line crypto-check
   const crypto = require('crypto');
 
   // The handle for PBKDF2 and RandomBytes isn't returned by the function call,
@@ -215,7 +215,7 @@ if (common.hasCrypto) {
 }
 
 
-if (common.hasCrypto) {
+if (common.hasCrypto) { // eslint-disable-line crypto-check
   const TCP = process.binding('tcp_wrap').TCP;
   const tcp = new TCP();
 

test/parallel/test-buffer-alloc.js

@@ -909,7 +909,7 @@ assert.throws(() => Buffer.from('', 'buffer'),
   }
 }
 
-if (common.hasCrypto) {
+if (common.hasCrypto) { // eslint-disable-line crypto-check
   // Test truncation after decode
   const crypto = require('crypto');
 

test/parallel/test-buffer-concat.js

@@ -62,6 +62,7 @@ function assertWrongList(value) {
   }));
 }
 
+// eslint-disable-next-line crypto-check
 const random10 = common.hasCrypto ?
   require('crypto').randomBytes(10) :
   Buffer.alloc(10, 1);

test/parallel/test-http2-noflag.js

@@ -4,5 +4,5 @@
 require('../common');
 const assert = require('assert');
 
-assert.throws(() => require('http2'),
+assert.throws(() => require('http2'), // eslint-disable-line crypto-check
               /^Error: Cannot find module 'http2'$/);

tools/eslint-rules/crypto-check.js

@@ -0,0 +1,83 @@
+/**
+ * @fileoverview Check that common.hasCrypto is used if crypto, tls,
+ * https, or http2 modules are required.
+ *
+ * This rule can be ignored using // eslint-disable-line crypto-check
+ *
+ * @author Daniel Bevenius <[email protected]>
+ */
+'use strict';
+
+const utils = require('./rules-utils.js');
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+const msg = 'Please add a hasCrypto check to allow this test to be skipped ' +
+            'when Node is built "--without-ssl".';
+
+module.exports = function(context) {
+  const missingCheckNodes = [];
+  const requireNodes = [];
+  var hasSkipCall = false;
+
+  function testCryptoUsage(node) {
+    if (utils.isRequired(node, ['crypto', 'tls', 'https', 'http2'])) {
+      requireNodes.push(node);
+    }
+  }
+
+  function testIfStatement(node) {
+    if (node.test.argument === undefined) {
+      return;
+    }
+    if (isCryptoCheck(node.test.argument)) {
+      checkCryptoCall(node);
+    }
+  }
+
+  function isCryptoCheck(node) {
+    return utils.usesCommonProperty(node, ['hasCrypto', 'hasFipsCrypto']);
+  }
+
+  function checkCryptoCall(node) {
+    if (utils.inSkipBlock(node)) {
+      hasSkipCall = true;
+    } else {
+      missingCheckNodes.push(node);
+    }
+  }
+
+  function testMemberExpression(node) {
+    if (isCryptoCheck(node)) {
+      checkCryptoCall(node);
+    }
+  }
+
+  function reportIfMissingCheck(node) {
+    if (hasSkipCall) {
+      return;
+    }
+
+    if (requireNodes.length > 0) {
+      if (missingCheckNodes.length > 0) {
+        report(missingCheckNodes);
+      } else {
+        report(requireNodes);
+      }
+    }
+  }
+
+  function report(nodes) {
+    nodes.forEach((node) => {
+      context.report(node, msg);
+    });
+  }
+
+  return {
+    'CallExpression': (node) => testCryptoUsage(node),
+    'IfStatement:exit': (node) => testIfStatement(node),
+    'MemberExpression:exit': (node) => testMemberExpression(node),
+    'Program:exit': (node) => reportIfMissingCheck(node)
+  };
+};

tools/eslint-rules/inspector-check.js

@@ -0,0 +1,43 @@
+/**
+ * @fileoverview Check that common.skipIfInspectorDisabled is used if
+ *               the inspector module is required.
+ * @author Daniel Bevenius <[email protected]>
+ */
+'use strict';
+
+const utils = require('./rules-utils.js');
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+const msg = 'Please add a skipIfInspectorDisabled() call to allow this ' +
+            'test to be skippped when Node is built \'--without-inspector\'.';
+
+module.exports = function(context) {
+  var usesInspector = false;
+  var hasInspectorCheck = false;
+
+  function testInspectorUsage(context, node) {
+    if (utils.isRequired(node, ['inspector'])) {
+      usesInspector = true;
+    }
+  }
+
+  function checkMemberExpression(context, node) {
+    if (utils.usesCommonProperty(node, ['skipIfInspectorDisabled'])) {
+      hasInspectorCheck = true;
+    }
+  }
+
+  function reportIfMissing(context, node) {
+    if (usesInspector && !hasInspectorCheck) {
+      context.report(node, msg);
+    }
+  }
+
+  return {
+    'CallExpression': (node) => testInspectorUsage(context, node),
+    'MemberExpression': (node) => checkMemberExpression(context, node),
+    'Program:exit': (node) => reportIfMissing(context, node)
+  };
+};

tools/eslint-rules/rules-utils.js

@@ -0,0 +1,61 @@
+/**
+ * Utility functions common to ESLint rules.
+ */
+'use strict';
+
+/**
+ * Returns true if any of the passed in modules are used in
+ * require calls.
+ */
+module.exports.isRequired = function(node, modules) {
+  return node.callee.name === 'require' &&
+    modules.includes(node.arguments[0].value);
+};
+
+/**
+ * Returns true is the node accesses any property in the properties
+ * array on the 'common' object.
+ */
+module.exports.usesCommonProperty = function(node, properties) {
+  if (node.name) {
+    return properties.includes(node.name);
+  }
+  if (node.property) {
+    return properties.includes(node.property.name);
+  }
+  return false;
+};
+
+/**
+ * Returns true if the passed in node is inside an if statement block,
+ * and the block also has a call to skip.
+ */
+module.exports.inSkipBlock = function(node) {
+  var hasSkipBlock = false;
+  if (node.test &&
+      node.test.type === 'UnaryExpression' &&
+      node.test.operator === '!') {
+    const consequent = node.consequent;
+    if (consequent.body) {
+      consequent.body.some(function(expressionStatement) {
+        if (hasSkip(expressionStatement.expression)) {
+          return hasSkipBlock = true;
+        }
+        return false;
+      });
+    } else {
+      if (hasSkip(consequent.expression)) {
+        hasSkipBlock = true;
+      }
+    }
+  }
+  return hasSkipBlock;
+};
+
+function hasSkip(expression) {
+  return expression &&
+         expression.callee &&
+         (expression.callee.name === 'skip' ||
+         expression.callee.property &&
+         expression.callee.property.name === 'skip');
+}