diff --git a/test/.eslintrc.yaml b/test/.eslintrc.yaml index aeaf09fb0ff732..58e072c1951e27 100644 --- a/test/.eslintrc.yaml +++ b/test/.eslintrc.yaml @@ -10,5 +10,7 @@ rules: prefer-assert-iferror: error prefer-assert-methods: error prefer-common-mustnotcall: error + crypto-check: error + inspector-check: error ## common module is mandatory in tests required-modules: [error, common] diff --git a/test/common/index.js b/test/common/index.js index 8175474818b9dc..b79c1a57559e1a 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -19,7 +19,7 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. -/* eslint-disable required-modules */ +/* eslint-disable required-modules, crypto-check */ 'use strict'; const path = require('path'); const fs = require('fs'); diff --git a/test/parallel/test-async-wrap-getasyncid.js b/test/parallel/test-async-wrap-getasyncid.js index 4c1ea8e212e7ef..95317f711c9907 100644 --- a/test/parallel/test-async-wrap-getasyncid.js +++ b/test/parallel/test-async-wrap-getasyncid.js @@ -81,14 +81,14 @@ function testInitialized(req, ctor_name) { } -if (common.hasCrypto) { +if (common.hasCrypto) { // eslint-disable-line crypto-check const tls = require('tls'); // SecurePair testInitialized(tls.createSecurePair().ssl, 'Connection'); } -if (common.hasCrypto) { +if (common.hasCrypto) { // eslint-disable-line crypto-check const crypto = require('crypto'); // The handle for PBKDF2 and RandomBytes isn't returned by the function call, @@ -215,7 +215,7 @@ if (common.hasCrypto) { } -if (common.hasCrypto) { +if (common.hasCrypto) { // eslint-disable-line crypto-check const TCP = process.binding('tcp_wrap').TCP; const tcp = new TCP(); diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js index b62a192b126380..2843e9c9db727a 100644 --- a/test/parallel/test-buffer-alloc.js +++ b/test/parallel/test-buffer-alloc.js @@ -909,7 +909,7 @@ assert.throws(() => Buffer.from('', 'buffer'), } } -if (common.hasCrypto) { +if (common.hasCrypto) { // eslint-disable-line crypto-check // Test truncation after decode const crypto = require('crypto'); diff --git a/test/parallel/test-buffer-concat.js b/test/parallel/test-buffer-concat.js index 9f3acbc599c3eb..87ccb720b7497a 100644 --- a/test/parallel/test-buffer-concat.js +++ b/test/parallel/test-buffer-concat.js @@ -62,6 +62,7 @@ function assertWrongList(value) { })); } +// eslint-disable-next-line crypto-check const random10 = common.hasCrypto ? require('crypto').randomBytes(10) : Buffer.alloc(10, 1); diff --git a/test/parallel/test-http2-noflag.js b/test/parallel/test-http2-noflag.js index a1e0e8b72c79e9..903fd649c5e458 100644 --- a/test/parallel/test-http2-noflag.js +++ b/test/parallel/test-http2-noflag.js @@ -4,5 +4,5 @@ require('../common'); const assert = require('assert'); -assert.throws(() => require('http2'), +assert.throws(() => require('http2'), // eslint-disable-line crypto-check /^Error: Cannot find module 'http2'$/); diff --git a/tools/eslint-rules/crypto-check.js b/tools/eslint-rules/crypto-check.js new file mode 100644 index 00000000000000..b1b2a03f50e3b6 --- /dev/null +++ b/tools/eslint-rules/crypto-check.js @@ -0,0 +1,83 @@ +/** + * @fileoverview Check that common.hasCrypto is used if crypto, tls, + * https, or http2 modules are required. + * + * This rule can be ignored using // eslint-disable-line crypto-check + * + * @author Daniel Bevenius + */ +'use strict'; + +const utils = require('./rules-utils.js'); + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ +const msg = 'Please add a hasCrypto check to allow this test to be skipped ' + + 'when Node is built "--without-ssl".'; + +module.exports = function(context) { + const missingCheckNodes = []; + const requireNodes = []; + var hasSkipCall = false; + + function testCryptoUsage(node) { + if (utils.isRequired(node, ['crypto', 'tls', 'https', 'http2'])) { + requireNodes.push(node); + } + } + + function testIfStatement(node) { + if (node.test.argument === undefined) { + return; + } + if (isCryptoCheck(node.test.argument)) { + checkCryptoCall(node); + } + } + + function isCryptoCheck(node) { + return utils.usesCommonProperty(node, ['hasCrypto', 'hasFipsCrypto']); + } + + function checkCryptoCall(node) { + if (utils.inSkipBlock(node)) { + hasSkipCall = true; + } else { + missingCheckNodes.push(node); + } + } + + function testMemberExpression(node) { + if (isCryptoCheck(node)) { + checkCryptoCall(node); + } + } + + function reportIfMissingCheck(node) { + if (hasSkipCall) { + return; + } + + if (requireNodes.length > 0) { + if (missingCheckNodes.length > 0) { + report(missingCheckNodes); + } else { + report(requireNodes); + } + } + } + + function report(nodes) { + nodes.forEach((node) => { + context.report(node, msg); + }); + } + + return { + 'CallExpression': (node) => testCryptoUsage(node), + 'IfStatement:exit': (node) => testIfStatement(node), + 'MemberExpression:exit': (node) => testMemberExpression(node), + 'Program:exit': (node) => reportIfMissingCheck(node) + }; +}; diff --git a/tools/eslint-rules/inspector-check.js b/tools/eslint-rules/inspector-check.js new file mode 100644 index 00000000000000..f225b34cb6b0ca --- /dev/null +++ b/tools/eslint-rules/inspector-check.js @@ -0,0 +1,43 @@ +/** + * @fileoverview Check that common.skipIfInspectorDisabled is used if + * the inspector module is required. + * @author Daniel Bevenius + */ +'use strict'; + +const utils = require('./rules-utils.js'); + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ +const msg = 'Please add a skipIfInspectorDisabled() call to allow this ' + + 'test to be skippped when Node is built \'--without-inspector\'.'; + +module.exports = function(context) { + var usesInspector = false; + var hasInspectorCheck = false; + + function testInspectorUsage(context, node) { + if (utils.isRequired(node, ['inspector'])) { + usesInspector = true; + } + } + + function checkMemberExpression(context, node) { + if (utils.usesCommonProperty(node, ['skipIfInspectorDisabled'])) { + hasInspectorCheck = true; + } + } + + function reportIfMissing(context, node) { + if (usesInspector && !hasInspectorCheck) { + context.report(node, msg); + } + } + + return { + 'CallExpression': (node) => testInspectorUsage(context, node), + 'MemberExpression': (node) => checkMemberExpression(context, node), + 'Program:exit': (node) => reportIfMissing(context, node) + }; +}; diff --git a/tools/eslint-rules/rules-utils.js b/tools/eslint-rules/rules-utils.js new file mode 100644 index 00000000000000..e3e5e6e5ef9718 --- /dev/null +++ b/tools/eslint-rules/rules-utils.js @@ -0,0 +1,61 @@ +/** + * Utility functions common to ESLint rules. + */ +'use strict'; + +/** + * Returns true if any of the passed in modules are used in + * require calls. + */ +module.exports.isRequired = function(node, modules) { + return node.callee.name === 'require' && + modules.includes(node.arguments[0].value); +}; + +/** + * Returns true is the node accesses any property in the properties + * array on the 'common' object. + */ +module.exports.usesCommonProperty = function(node, properties) { + if (node.name) { + return properties.includes(node.name); + } + if (node.property) { + return properties.includes(node.property.name); + } + return false; +}; + +/** + * Returns true if the passed in node is inside an if statement block, + * and the block also has a call to skip. + */ +module.exports.inSkipBlock = function(node) { + var hasSkipBlock = false; + if (node.test && + node.test.type === 'UnaryExpression' && + node.test.operator === '!') { + const consequent = node.consequent; + if (consequent.body) { + consequent.body.some(function(expressionStatement) { + if (hasSkip(expressionStatement.expression)) { + return hasSkipBlock = true; + } + return false; + }); + } else { + if (hasSkip(consequent.expression)) { + hasSkipBlock = true; + } + } + } + return hasSkipBlock; +}; + +function hasSkip(expression) { + return expression && + expression.callee && + (expression.callee.name === 'skip' || + expression.callee.property && + expression.callee.property.name === 'skip'); +}