From c32b0871613376a44c51740b37ed33cfc877af57 Mon Sep 17 00:00:00 2001 From: Shobhit Chittora Date: Wed, 1 Nov 2017 01:08:45 +0530 Subject: [PATCH] tools: auto fix custom crypto-check eslint rule 1. Fixer for crypto-check.js 2. Extends tests PR-URL: https://github.com/nodejs/node/pull/16647 Refs: https://github.com/nodejs/node/issues/16636 Reviewed-By: James M Snell Reviewed-By: Khaidi Chu Reviewed-By: Ruben Bridgewater --- test/parallel/test-eslint-crypto-check.js | 30 +++++++++++++++++------ tools/eslint-rules/crypto-check.js | 20 ++++++++++++++- tools/eslint-rules/rules-utils.js | 11 +++++++++ 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/test/parallel/test-eslint-crypto-check.js b/test/parallel/test-eslint-crypto-check.js index 7feea0d8715131..ccd4992b64d302 100644 --- 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 index 9570c24c030ef4..42b17b0c80a225 100644 --- a/tools/eslint-rules/crypto-check.js +++ b/tools/eslint-rules/crypto-check.js @@ -23,6 +23,7 @@ const bindingModules = cryptoModules.concat(['tls_wrap']); module.exports = function(context) { const missingCheckNodes = []; const requireNodes = []; + var commonModuleNode = null; var hasSkipCall = false; function testCryptoUsage(node) { @@ -30,6 +31,10 @@ module.exports = function(context) { 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 index 0fda705fb290b7..88ecf658ce37f0 100644 --- 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.