Skip to content
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -13,20 +13,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");
`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: unnecessary change.

],
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
10 changes: 10 additions & 0 deletions tools/eslint-rules/rules-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ 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' &&
commonModuleRegExp.test(node.arguments[0].value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will produce access errors due to accessing nodes without arguments. Please guard against them the same as I suggested in the other PR.

};

/**
* Returns true if any of the passed in modules are used in
* binding calls.
Expand Down