tools: add no-duplicate-requires rule

PR-URL: #21712
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jon Moss <[email protected]>

Author: devsnek

.eslintrc.js

@@ -98,6 +98,7 @@ module.exports = {
     'no-dupe-class-members': 'error',
     'no-dupe-keys': 'error',
     'no-duplicate-case': 'error',
+    'no-duplicate-imports': 'error',
     'no-empty-character-class': 'error',
     'no-ex-assign': 'error',
     'no-extra-boolean-cast': 'error',
@@ -246,6 +247,7 @@ module.exports = {
 
     // Custom rules from eslint-plugin-node-core
     'node-core/no-unescaped-regexp-dot': 'error',
+    'node-core/no-duplicate-requires': 'error',
   },
   globals: {
     Atomics: false,

benchmark/streams/creation.js

@@ -1,9 +1,11 @@
 'use strict';
 const common = require('../common.js');
-const Duplex = require('stream').Duplex;
-const Readable = require('stream').Readable;
-const Transform = require('stream').Transform;
-const Writable = require('stream').Writable;
+const {
+  Duplex,
+  Readable,
+  Transform,
+  Writable,
+} = require('stream');
 
 const bench = common.createBenchmark(main, {
   n: [50e6],

doc/api/worker_threads.md

@@ -286,7 +286,7 @@ For example:
 ```js
 const assert = require('assert');
 const {
-  Worker, MessageChannel, MessagePort, isMainThread
+  Worker, MessageChannel, MessagePort, isMainThread, parentPort
 } = require('worker_threads');
 if (isMainThread) {
   const worker = new Worker(__filename);
@@ -296,7 +296,7 @@ if (isMainThread) {
     console.log('received:', value);
   });
 } else {
-  require('worker_threads').once('message', (value) => {
+  parentPort.once('message', (value) => {
     assert(value.hereIsYourPort instanceof MessagePort);
     value.hereIsYourPort.postMessage('the worker is sending this');
     value.hereIsYourPort.close();

lib/internal/http2/core.js

@@ -24,8 +24,12 @@ const { kIncomingMessage } = require('_http_common');
 const { kServerResponse } = require('_http_server');
 const { StreamWrap } = require('_stream_wrap');
 
-const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
-const { async_id_symbol } = require('internal/async_hooks').symbols;
+const {
+  defaultTriggerAsyncIdScope,
+  symbols: {
+    async_id_symbol,
+  },
+} = require('internal/async_hooks');
 const { internalBinding } = require('internal/bootstrap/loaders');
 const {
   codes: {

lib/internal/modules/esm/translators.js

@@ -14,13 +14,14 @@ const fs = require('fs');
 const { _makeLong } = require('path');
 const { SafeMap } = require('internal/safe_globals');
 const { URL } = require('url');
-const util = require('util');
-const debug = util.debuglog('esm');
-const readFileAsync = util.promisify(fs.readFile);
+const { debuglog, promisify } = require('util');
+const readFileAsync = promisify(fs.readFile);
 const readFileSync = fs.readFileSync;
 const StringReplace = Function.call.bind(String.prototype.replace);
 const JsonParse = JSON.parse;
 
+const debug = debuglog('esm');
+
 const translators = new SafeMap();
 module.exports = translators;
 

test/.eslintrc.yaml

@@ -18,6 +18,7 @@ rules:
   node-core/number-isnan: error
   ## common module is mandatory in tests
   node-core/required-modules: [error, common]
+  node-core/no-duplicate-requires: off
 
   no-restricted-syntax:
     # Config copied from .eslintrc.js

test/parallel/test-eslint-duplicate-requires.js

@@ -0,0 +1,25 @@
+'use strict';
+
+const common = require('../common');
+
+common.skipIfEslintMissing();
+
+const { RuleTester } = require('../../tools/node_modules/eslint');
+const rule = require('../../tools/eslint-rules/no-duplicate-requires');
+
+new RuleTester().run('no-duplicate-requires', rule, {
+  valid: [
+    {
+      code: 'require("a"); require("b"); (function() { require("a"); });',
+    },
+    {
+      code: 'require(a); require(a);',
+    },
+  ],
+  invalid: [
+    {
+      code: 'require("a"); require("a");',
+      errors: [{ message: '\'a\' require is duplicated.' }],
+    },
+  ],
+});

tools/eslint-rules/no-duplicate-requires.js

@@ -0,0 +1,70 @@
+/**
+ * @fileoverview Ensure modules are not required twice at top level of a module
+ * @author devsnek
+ */
+'use strict';
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+
+function isString(node) {
+  return node && node.type === 'Literal' && typeof node.value === 'string';
+}
+
+function isRequireCall(node) {
+  return node.callee.type === 'Identifier' && node.callee.name === 'require';
+}
+
+function isTopLevel(node) {
+  do {
+    if (node.type === 'FunctionDeclaration' ||
+        node.type === 'FunctionExpression' ||
+        node.type === 'ArrowFunctionExpression' ||
+        node.type === 'ClassBody' ||
+        node.type === 'MethodDefinition') {
+      return false;
+    }
+  } while (node = node.parent);
+  return true;
+}
+
+module.exports = (context) => {
+  if (context.parserOptions.sourceType === 'module') {
+    return {};
+  }
+
+  function getRequiredModuleNameFromCall(node) {
+    // node has arguments and first argument is string
+    if (node.arguments.length && isString(node.arguments[0])) {
+      return node.arguments[0].value.trim();
+    }
+
+    return undefined;
+  }
+
+  const required = new Set();
+
+  const rules = {
+    CallExpression: (node) => {
+      if (isRequireCall(node) && isTopLevel(node)) {
+        const moduleName = getRequiredModuleNameFromCall(node);
+        if (moduleName === undefined) {
+          return;
+        }
+        if (required.has(moduleName)) {
+          context.report(
+            node,
+            '\'{{moduleName}}\' require is duplicated.',
+            { moduleName }
+          );
+        } else {
+          required.add(moduleName);
+        }
+      }
+    },
+  };
+
+  return rules;
+};