From a4423ebaa1437534d4d81f5faaca71762fb35a02 Mon Sep 17 00:00:00 2001 From: cola119 Date: Sat, 6 Aug 2022 22:43:33 +0900 Subject: [PATCH] tools: add `require-order` rule This rule enforces `{a, b, c} = require()` is in alphabetical order. --- lib/.eslintrc.yaml | 1 + test/parallel/test-eslint-require-order.js | 61 ++++++++++++++++++ tools/eslint-rules/no-duplicate-requires.js | 19 +----- tools/eslint-rules/require-order.js | 71 +++++++++++++++++++++ tools/eslint-rules/rules-utils.js | 17 +++++ 5 files changed, 152 insertions(+), 17 deletions(-) create mode 100644 test/parallel/test-eslint-require-order.js create mode 100644 tools/eslint-rules/require-order.js diff --git a/lib/.eslintrc.yaml b/lib/.eslintrc.yaml index 2cfbf0894f2a3a2..0a3bac40e8f35b3 100644 --- a/lib/.eslintrc.yaml +++ b/lib/.eslintrc.yaml @@ -166,6 +166,7 @@ rules: node-core/lowercase-name-for-primitive: error node-core/non-ascii-character: error node-core/no-array-destructuring: error + node-core/require-order: error node-core/prefer-primordials: - error - name: AggregateError diff --git a/test/parallel/test-eslint-require-order.js b/test/parallel/test-eslint-require-order.js new file mode 100644 index 000000000000000..121586e6218c666 --- /dev/null +++ b/test/parallel/test-eslint-require-order.js @@ -0,0 +1,61 @@ +'use strict'; + +const common = require('../common'); +if ((!common.hasCrypto) || (!common.hasIntl)) { + common.skip('ESLint tests require crypto and Intl'); +} + +common.skipIfEslintMissing(); + +const RuleTester = require('../../tools/node_modules/eslint').RuleTester; +const rule = require('../../tools/eslint-rules/require-order'); + +new RuleTester({ + parserOptions: { ecmaVersion: 6 }, +}) + .run('require-order', rule, { + valid: [ + "const A = require('path')", + 'const A = primordials', + "const { A } = require('path')", + 'const { A } = primordials', + "const { A, B } = require('path')", + 'const { A, B } = primordials', + "const { B: a, A: b } = require('path')", + 'const { B: a, A: b } = primordials', + ], + invalid: [ + { + code: "const { B, A } = require('path')", + errors: [{ message: 'A should occur before B' }] + }, + { + code: 'const { B, A } = primordials', + errors: [{ message: 'A should occur before B' }] + }, + { + code: "const { A: b, A: a } = require('path')", + errors: [{ message: 'a should occur before b' }] + }, + { + code: 'const { A: b, A: a } = primordials', + errors: [{ message: 'a should occur before b' }] + }, + { + code: "const { C, B, A } = require('path')", + errors: [{ message: 'B should occur before C' }] + }, + { + code: 'const { C, B, A } = primordials', + errors: [{ message: 'B should occur before C' }] + }, + { + code: "const { C, A, B } = require('path')", + errors: [{ message: 'A should occur before C' }] + }, + { + code: 'const { C, A, B } = primordials', + errors: [{ message: 'A should occur before C' }] + }, + ] + }); diff --git a/tools/eslint-rules/no-duplicate-requires.js b/tools/eslint-rules/no-duplicate-requires.js index 6e149edceb8d6ee..d21903eababcfbd 100644 --- a/tools/eslint-rules/no-duplicate-requires.js +++ b/tools/eslint-rules/no-duplicate-requires.js @@ -4,27 +4,12 @@ */ 'use strict'; -const { isRequireCall, isString } = require('./rules-utils.js'); +const { isTopLevelRequireCall, isString } = require('./rules-utils.js'); //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ -const secondLevelTypes = [ - 'FunctionDeclaration', 'FunctionExpression', 'ArrowFunctionExpression', - 'ClassBody', 'MethodDefinition', -]; - -function isTopLevel(node) { - while (!secondLevelTypes.includes(node.type)) { - node = node.parent; - if (!node) { - return true; - } - } - return false; -} - module.exports = (context) => { if (context.parserOptions.sourceType === 'module') { return {}; @@ -43,7 +28,7 @@ module.exports = (context) => { const rules = { CallExpression: (node) => { - if (isRequireCall(node) && isTopLevel(node)) { + if (isTopLevelRequireCall(node)) { const moduleName = getRequiredModuleNameFromCall(node); if (moduleName === undefined) { return; diff --git a/tools/eslint-rules/require-order.js b/tools/eslint-rules/require-order.js new file mode 100644 index 000000000000000..7dd1eeb40028a08 --- /dev/null +++ b/tools/eslint-rules/require-order.js @@ -0,0 +1,71 @@ +'use strict'; + +const { isTopLevelRequireCall } = require('./rules-utils.js'); + +const getRequiredNodesFromRequireCallExpression = (node) => { + if ( + node && + node.parent && + node.parent.id && + node.parent.id.type === 'ObjectPattern' + ) { + return node.parent.id.properties.map((p) => p.value); + } +}; + +const isPrimordialsDeclarator = (node) => { + return ( + node && + node.type === 'VariableDeclarator' && + node.init?.name === 'primordials' + ); +}; + +const getRequiredNodesFromPrimordialsDeclarator = (node) => { + if ( + node && + node.id && + node.id.type === 'ObjectPattern' + ) { + return node.id.properties.map((p) => p.value); + } +}; + +const findOutOfOrderIndex = (nodes) => { + for (let i = 0; i < nodes.length - 1; i++) { + if (nodes[i].name > nodes[i + 1].name) return i; + } + return -1; +}; + +module.exports = { + create(context) { + const reportOutOfOrder = (target, requiredNodes, outOfOrderIndex) => { + const before = requiredNodes[outOfOrderIndex]; + const after = requiredNodes[outOfOrderIndex + 1]; + context.report( + target, + `${after.name} should occur before ${before.name}` + ); + }; + + return { + CallExpression(node) { + if (!isTopLevelRequireCall(node)) return; + const requiredNodes = getRequiredNodesFromRequireCallExpression(node); + if (!requiredNodes) return; + const outOfOrderIndex = findOutOfOrderIndex(requiredNodes); + if (outOfOrderIndex === -1) return; + reportOutOfOrder(node, requiredNodes, outOfOrderIndex); + }, + VariableDeclarator(node) { + if (!isPrimordialsDeclarator(node)) return; + const requiredNodes = getRequiredNodesFromPrimordialsDeclarator(node); + if (!requiredNodes) return; + const outOfOrderIndex = findOutOfOrderIndex(requiredNodes); + if (outOfOrderIndex === -1) return; + reportOutOfOrder(node, requiredNodes, outOfOrderIndex); + }, + }; + }, +}; diff --git a/tools/eslint-rules/rules-utils.js b/tools/eslint-rules/rules-utils.js index c5362c96cdaebe6..3be91300515a1c8 100644 --- a/tools/eslint-rules/rules-utils.js +++ b/tools/eslint-rules/rules-utils.js @@ -8,6 +8,23 @@ function isRequireCall(node) { } module.exports.isRequireCall = isRequireCall; +module.exports.isTopLevelRequireCall = function(node) { + const secondLevelTypes = [ + 'FunctionDeclaration', 'FunctionExpression', 'ArrowFunctionExpression', + 'ClassBody', 'MethodDefinition', + ]; + const isTopLevel = (node) => { + while (!secondLevelTypes.includes(node.type)) { + node = node.parent; + if (!node) { + return true; + } + } + return false; + }; + return isRequireCall(node) && isTopLevel(node); +}; + module.exports.isString = function(node) { return node && node.type === 'Literal' && typeof node.value === 'string'; };