tools: auto fix custom crypto-check eslint rule

1. Fixer for crypto-check.js 2. Extends tests PR-URL: #16647 Refs: #16636 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Khaidi Chu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
nodejs · Aug 16, 2018 · c32b087 · c32b087
1 parent c4716dc
commit c32b087
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 9 deletions.
diff --git a/test/parallel/test-eslint-crypto-check.js b/test/parallel/test-eslint-crypto-check.js
@@ -15,20 +15,34 @@ new RuleTester().run('crypto-check', rule, {
     'foo',
     'crypto',
     `
-      if (!common.hasCrypto) {
-        common.skip();
-      }
-      require('crypto');
+    if (!common.hasCrypto) {
+      common.skip("missing crypto");
+    }
+    require("crypto");
     `
   ],
   invalid: [
     {
-      code: 'require("crypto")',
-      errors: [{ message }]
+      code: 'require("common")\n' +
+            'require("crypto")',
+      errors: [{ message }],
+      output: 'require("common")\n' +
+              'if (!common.hasCrypto) {' +
+              ' common.skip("missing crypto");' +
+              '}\n' +
+              'require("crypto")'
     },
     {
-      code: 'if (common.foo) {} require("crypto")',
-      errors: [{ message }]
+      code: 'require("common")\n' +
+            'if (common.foo) {}\n' +
+            'require("crypto")',
+      errors: [{ message }],
+      output: 'require("common")\n' +
+              'if (!common.hasCrypto) {' +
+              ' common.skip("missing crypto");' +
+              '}\n' +
+              'if (common.foo) {}\n' +
+              'require("crypto")'
     }
   ]
 });
diff --git a/tools/eslint-rules/crypto-check.js b/tools/eslint-rules/crypto-check.js
@@ -23,13 +23,18 @@ const bindingModules = cryptoModules.concat(['tls_wrap']);
 module.exports = function(context) {
   const missingCheckNodes = [];
   const requireNodes = [];
+  var commonModuleNode = null;
   var hasSkipCall = false;
 
   function testCryptoUsage(node) {
     if (utils.isRequired(node, requireModules) ||
         utils.isBinding(node, bindingModules)) {
       requireNodes.push(node);
     }
+
+    if (utils.isCommonModule(node)) {
+      commonModuleNode = node;
+    }
   }
 
   function testIfStatement(node) {
@@ -75,7 +80,20 @@ module.exports = function(context) {
 
   function report(nodes) {
     nodes.forEach((node) => {
-      context.report(node, msg);
+      context.report({
+        node,
+        message: msg,
+        fix: (fixer) => {
+          if (commonModuleNode) {
+            return fixer.insertTextAfter(
+              commonModuleNode,
+              '\nif (!common.hasCrypto) {' +
+              ' common.skip("missing crypto");' +
+              '}'
+            );
+          }
+        }
+      });
     });
   }
 

diff --git a/tools/eslint-rules/rules-utils.js b/tools/eslint-rules/rules-utils.js
@@ -12,6 +12,17 @@ module.exports.isRequired = function(node, modules) {
     modules.includes(node.arguments[0].value);
 };
 
+/**
+* Return true if common module is required
+* in AST Node under inspection
+*/
+var commonModuleRegExp = new RegExp(/^(\.\.\/)*common(\.js)?$/);
+module.exports.isCommonModule = function(node) {
+  return node.callee.name === 'require' &&
+         node.arguments.length !== 0 &&
+         commonModuleRegExp.test(node.arguments[0].value);
+};
+
 /**
  * Returns true if any of the passed in modules are used in
  * binding calls.