tools: add require-order rule

This rule enforces `{a, b, c} = require()` is in alphabetical order.
nodejs · Aug 6, 2022 · a4423eb · a4423eb
1 parent 8c35a4e
commit a4423eb
Show file tree

Hide file tree

Showing 5 changed files with 152 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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';
 };