From 8cae9b2ff808198b4451d9de634320fa5750c4cd Mon Sep 17 00:00:00 2001 From: Gus Caplan Date: Sat, 7 Jul 2018 23:32:23 -0500 Subject: [PATCH] tools: add no-duplicate-requires rule PR-URL: https://github.com/nodejs/node/pull/21712 Reviewed-By: Richard Lau Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Weijia Wang Reviewed-By: James M Snell Reviewed-By: Jon Moss --- .eslintrc.js | 2 + benchmark/streams/creation.js | 10 +-- doc/api/worker_threads.md | 4 +- lib/internal/http2/core.js | 8 ++- lib/internal/modules/esm/translators.js | 7 +- test/.eslintrc.yaml | 1 + .../test-eslint-duplicate-requires.js | 25 +++++++ tools/eslint-rules/no-duplicate-requires.js | 70 +++++++++++++++++++ 8 files changed, 116 insertions(+), 11 deletions(-) create mode 100644 test/parallel/test-eslint-duplicate-requires.js create mode 100644 tools/eslint-rules/no-duplicate-requires.js diff --git a/.eslintrc.js b/.eslintrc.js index 742f9ea6d38ef9..98568dd9bc4311 100644 --- a/.eslintrc.js +++ b/.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, diff --git a/benchmark/streams/creation.js b/benchmark/streams/creation.js index 67187f91bd9cb1..46a0a547907c45 100644 --- a/benchmark/streams/creation.js +++ b/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], diff --git a/doc/api/worker_threads.md b/doc/api/worker_threads.md index d20fd0fce57c80..8b301f42a87620 100644 --- a/doc/api/worker_threads.md +++ b/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(); diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 5a4fa63335581d..db251caf5f73ef 100644 --- a/lib/internal/http2/core.js +++ b/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: { diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 7785865d6573f9..618f1adac8deb7 100644 --- a/lib/internal/modules/esm/translators.js +++ b/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; diff --git a/test/.eslintrc.yaml b/test/.eslintrc.yaml index c57d7505c984ff..25026fec5a103a 100644 --- a/test/.eslintrc.yaml +++ b/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 diff --git a/test/parallel/test-eslint-duplicate-requires.js b/test/parallel/test-eslint-duplicate-requires.js new file mode 100644 index 00000000000000..c7edaeed2b6b6e --- /dev/null +++ b/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.' }], + }, + ], +}); diff --git a/tools/eslint-rules/no-duplicate-requires.js b/tools/eslint-rules/no-duplicate-requires.js new file mode 100644 index 00000000000000..595c22360112ca --- /dev/null +++ b/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; +};