Skip to content

Commit

Permalink
tools: auto fix custom crypto-check eslint rule
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
shobhitchittora authored and rvagg committed Aug 16, 2018
1 parent c4716dc commit c32b087
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 9 deletions.
30 changes: 22 additions & 8 deletions test/parallel/test-eslint-crypto-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -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")'
}
]
});
20 changes: 19 additions & 1 deletion tools/eslint-rules/crypto-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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");' +
'}'
);
}
}
});
});
}

Expand Down
11 changes: 11 additions & 0 deletions tools/eslint-rules/rules-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit c32b087

Please sign in to comment.