Skip to content

Commit 1248236

Browse files
shobhitchittoraMylesBorins
authored andcommitted
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 <jasnell@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent c98f039 commit 1248236

File tree

3 files changed

+52
-9
lines changed

3 files changed

+52
-9
lines changed

test/parallel/test-eslint-crypto-check.js

+22-8
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,34 @@ new RuleTester().run('crypto-check', rule, {
1515
'foo',
1616
'crypto',
1717
`
18-
if (!common.hasCrypto) {
19-
common.skip();
20-
}
21-
require('crypto');
18+
if (!common.hasCrypto) {
19+
common.skip("missing crypto");
20+
}
21+
require("crypto");
2222
`
2323
],
2424
invalid: [
2525
{
26-
code: 'require("crypto")',
27-
errors: [{ message }]
26+
code: 'require("common")\n' +
27+
'require("crypto")',
28+
errors: [{ message }],
29+
output: 'require("common")\n' +
30+
'if (!common.hasCrypto) {' +
31+
' common.skip("missing crypto");' +
32+
'}\n' +
33+
'require("crypto")'
2834
},
2935
{
30-
code: 'if (common.foo) {} require("crypto")',
31-
errors: [{ message }]
36+
code: 'require("common")\n' +
37+
'if (common.foo) {}\n' +
38+
'require("crypto")',
39+
errors: [{ message }],
40+
output: 'require("common")\n' +
41+
'if (!common.hasCrypto) {' +
42+
' common.skip("missing crypto");' +
43+
'}\n' +
44+
'if (common.foo) {}\n' +
45+
'require("crypto")'
3246
}
3347
]
3448
});

tools/eslint-rules/crypto-check.js

+19-1
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,18 @@ const bindingModules = cryptoModules.concat(['tls_wrap']);
2323
module.exports = function(context) {
2424
const missingCheckNodes = [];
2525
const requireNodes = [];
26+
var commonModuleNode = null;
2627
var hasSkipCall = false;
2728

2829
function testCryptoUsage(node) {
2930
if (utils.isRequired(node, requireModules) ||
3031
utils.isBinding(node, bindingModules)) {
3132
requireNodes.push(node);
3233
}
34+
35+
if (utils.isCommonModule(node)) {
36+
commonModuleNode = node;
37+
}
3338
}
3439

3540
function testIfStatement(node) {
@@ -75,7 +80,20 @@ module.exports = function(context) {
7580

7681
function report(nodes) {
7782
nodes.forEach((node) => {
78-
context.report(node, msg);
83+
context.report({
84+
node,
85+
message: msg,
86+
fix: (fixer) => {
87+
if (commonModuleNode) {
88+
return fixer.insertTextAfter(
89+
commonModuleNode,
90+
'\nif (!common.hasCrypto) {' +
91+
' common.skip("missing crypto");' +
92+
'}'
93+
);
94+
}
95+
}
96+
});
7997
});
8098
}
8199

tools/eslint-rules/rules-utils.js

+11
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,17 @@ module.exports.isRequired = function(node, modules) {
1212
modules.includes(node.arguments[0].value);
1313
};
1414

15+
/**
16+
* Return true if common module is required
17+
* in AST Node under inspection
18+
*/
19+
var commonModuleRegExp = new RegExp(/^(\.\.\/)*common(\.js)?$/);
20+
module.exports.isCommonModule = function(node) {
21+
return node.callee.name === 'require' &&
22+
node.arguments.length !== 0 &&
23+
commonModuleRegExp.test(node.arguments[0].value);
24+
};
25+
1526
/**
1627
* Returns true if any of the passed in modules are used in
1728
* binding calls.

0 commit comments

Comments
 (0)