From de230555369087282213d337ecc1c315fbb74230 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 4 Apr 2019 11:36:41 +0800 Subject: [PATCH] lib: remove `env: node` in eslint config for lib files This patch removes the redundant `require-globals` custom eslint rule by removing `env: node` in the eslint config and whitelist the globals that can be accessed in native modules instead of black listing them. This makes sense for our `lib/` files because here we are creating the Node.js environment instead of running in a normal user land Node.js environment. PR-URL: https://github.com/nodejs/node/pull/27082 Reviewed-By: Luigi Pinca Reviewed-By: Ruben Bridgewater --- .eslintrc.js | 3 +- benchmark/.eslintrc.yaml | 4 ++ lib/.eslintrc.yaml | 22 ++++++++- lib/buffer.js | 23 ++++++---- lib/crypto.js | 4 +- lib/internal/bootstrap/loaders.js | 1 + lib/internal/modules/cjs/helpers.js | 2 +- lib/util.js | 6 +-- test/.eslintrc.yaml | 4 ++ test/parallel/test-eslint-require-buffer.js | 51 --------------------- tools/.eslintrc.yaml | 4 ++ tools/eslint-rules/require-globals.js | 50 -------------------- 12 files changed, 57 insertions(+), 117 deletions(-) delete mode 100644 test/parallel/test-eslint-require-buffer.js delete mode 100644 tools/eslint-rules/require-globals.js diff --git a/.eslintrc.js b/.eslintrc.js index d3a6c73241f1ff..9aa90c6366232d 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -1,5 +1,7 @@ 'use strict'; +/* eslint-env node */ + const Module = require('module'); const path = require('path'); @@ -31,7 +33,6 @@ Module._findPath = (request, paths, isMain) => { module.exports = { root: true, plugins: ['markdown', 'node-core'], - env: { node: true, es6: true }, parser: 'babel-eslint', parserOptions: { sourceType: 'script' }, overrides: [ diff --git a/benchmark/.eslintrc.yaml b/benchmark/.eslintrc.yaml index f8585fd7065f88..7de962dc9002af 100644 --- a/benchmark/.eslintrc.yaml +++ b/benchmark/.eslintrc.yaml @@ -1,5 +1,9 @@ ## Benchmark-specific linter rules +env: + node: true + es6: true + rules: comma-dangle: - error diff --git a/lib/.eslintrc.yaml b/lib/.eslintrc.yaml index 029d7ad177f468..2ec4dc80480e83 100644 --- a/lib/.eslintrc.yaml +++ b/lib/.eslintrc.yaml @@ -1,3 +1,6 @@ +env: + es6: true + rules: prefer-object-spread: error no-buffer-constructor: error @@ -19,10 +22,11 @@ rules: - selector: "CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']" message: "Please use `require('internal/errors').hideStackFrames()` instead." # Custom rules in tools/eslint-rules - node-core/require-globals: error node-core/lowercase-name-for-primitive: error node-core/non-ascii-character: error globals: + Intl: false + # Assertions CHECK: false CHECK_EQ: false CHECK_GE: false @@ -37,5 +41,21 @@ globals: DCHECK_LE: false DCHECK_LT: false DCHECK_NE: false + # Parameters passed to internal modules + global: false + require: false + process: false + exports: false + module: false internalBinding: false primordials: false + # Globals + # TODO(joyeecheung): if possible, get these in native modules + # through `require` instead of grabbing them from the global proxy. + clearTimeout: false + setTimeout: false + clearInterval: false + setInterval: false + setImmediate: false + clearImmediate: false + console: false diff --git a/lib/buffer.js b/lib/buffer.js index 24a24f369be17f..ae4ef870723f67 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -654,9 +654,10 @@ Buffer.prototype.equals = function equals(otherBuffer) { return _compare(this, otherBuffer) === 0; }; +let INSPECT_MAX_BYTES = 50; // Override how buffers are presented by util.inspect(). Buffer.prototype[customInspectSymbol] = function inspect(recurseTimes, ctx) { - const max = exports.INSPECT_MAX_BYTES; + const max = INSPECT_MAX_BYTES; const actualMax = Math.min(max, this.length); const remaining = this.length - max; let str = this.hexSlice(0, actualMax).replace(/(.{2})/g, '$1 ').trim(); @@ -1089,19 +1090,25 @@ if (internalBinding('config').hasIntl) { }; } -module.exports = exports = { +module.exports = { Buffer, SlowBuffer, transcode, - INSPECT_MAX_BYTES: 50, - // Legacy kMaxLength, kStringMaxLength }; -Object.defineProperty(exports, 'constants', { - configurable: false, - enumerable: true, - value: constants +Object.defineProperties(module.exports, { + constants: { + configurable: false, + enumerable: true, + value: constants + }, + INSPECT_MAX_BYTES: { + configurable: true, + enumerable: true, + get() { return INSPECT_MAX_BYTES; }, + set(val) { INSPECT_MAX_BYTES = val; } + } }); diff --git a/lib/crypto.js b/lib/crypto.js index e80c7a8327df98..44d4800624e291 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -145,7 +145,7 @@ function createVerify(algorithm, options) { return new Verify(algorithm, options); } -module.exports = exports = { +module.exports = { // Methods createCipheriv, createDecipheriv, @@ -218,7 +218,7 @@ function getFipsForced() { return 1; } -Object.defineProperties(exports, { +Object.defineProperties(module.exports, { createCipher: { enumerable: false, value: deprecate(createCipher, diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index d8dcd653920818..ffcc931334f861 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -124,6 +124,7 @@ const internalBindingWhitelist = new SafeSet([ let internalBinding; { const bindingObj = Object.create(null); + // eslint-disable-next-line no-global-assign internalBinding = function internalBinding(module) { let mod = bindingObj[module]; if (typeof mod !== 'object') { diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js index df727ff3c88a45..85c3557a812152 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -129,7 +129,7 @@ function normalizeReferrerURL(referrer) { return new URL(referrer).href; } -module.exports = exports = { +module.exports = { addBuiltinLibsToObject, builtinLibs, makeRequireFunction, diff --git a/lib/util.js b/lib/util.js index a508c3cead113c..f6b3082760dbe2 100644 --- a/lib/util.js +++ b/lib/util.js @@ -113,8 +113,8 @@ function timestamp() { } // Log is just a thin wrapper to console.log that prepends a timestamp -function log() { - console.log('%s - %s', timestamp(), exports.format.apply(exports, arguments)); +function log(...args) { + console.log('%s - %s', timestamp(), format(...args)); } /** @@ -218,7 +218,7 @@ function getSystemErrorName(err) { } // Keep the `exports =` so that various functions can still be monkeypatched -module.exports = exports = { +module.exports = { _errnoException: errnoException, _exceptionWithHostPort: exceptionWithHostPort, _extend, diff --git a/test/.eslintrc.yaml b/test/.eslintrc.yaml index 1140cff966a5c4..49c55de6b8ad0c 100644 --- a/test/.eslintrc.yaml +++ b/test/.eslintrc.yaml @@ -1,5 +1,9 @@ ## Test-specific linter rules +env: + node: true + es6: true + rules: # ECMAScript 6 # http://eslint.org/docs/rules/#ecmascript-6 diff --git a/test/parallel/test-eslint-require-buffer.js b/test/parallel/test-eslint-require-buffer.js deleted file mode 100644 index e0abd0c2d8d524..00000000000000 --- a/test/parallel/test-eslint-require-buffer.js +++ /dev/null @@ -1,51 +0,0 @@ -'use strict'; - -const common = require('../common'); -if (!common.hasCrypto) - common.skip('missing crypto'); - -common.skipIfEslintMissing(); - -const RuleTester = require('../../tools/node_modules/eslint').RuleTester; -const rule = require('../../tools/eslint-rules/require-globals'); -const ruleTester = new RuleTester({ - parserOptions: { ecmaVersion: 6 }, - env: { node: true } -}); - -const message = "Use const { Buffer } = require('buffer'); " + - 'at the beginning of this file'; - -const useStrict = '\'use strict\';\n\n'; -const bufferModule = 'const { Buffer } = require(\'buffer\');\n'; -const mockComment = '// Some Comment\n//\n// Another Comment\n\n'; -const useBuffer = 'Buffer;'; -ruleTester.run('require-globals', rule, { - valid: [ - 'foo', - 'const Buffer = require("Buffer"); Buffer;', - 'const { Buffer } = require(\'buffer\'); Buffer;', - ], - invalid: [ - { - code: useBuffer, - errors: [{ message }], - output: bufferModule + useBuffer, - }, - { - code: useStrict + useBuffer, - errors: [{ message }], - output: useStrict + bufferModule + useBuffer, - }, - { - code: mockComment + useBuffer, - errors: [{ message }], - output: mockComment + bufferModule + useBuffer, - }, - { - code: mockComment + useStrict + useBuffer, - errors: [{ message }], - output: mockComment + useStrict + bufferModule + useBuffer, - }, - ] -}); diff --git a/tools/.eslintrc.yaml b/tools/.eslintrc.yaml index ea518d2afb83ad..ffc36c66bdfc9d 100644 --- a/tools/.eslintrc.yaml +++ b/tools/.eslintrc.yaml @@ -1,3 +1,7 @@ +env: + node: true + es6: true + rules: comma-dangle: - error diff --git a/tools/eslint-rules/require-globals.js b/tools/eslint-rules/require-globals.js deleted file mode 100644 index bc49ff6c8746ed..00000000000000 --- a/tools/eslint-rules/require-globals.js +++ /dev/null @@ -1,50 +0,0 @@ -'use strict'; - -// This rule makes sure that no Globals are going to be used in /lib. -// That could otherwise result in problems with the repl. - -module.exports = function(context) { - - function flagIt(msg, fix) { - return (reference) => { - context.report({ - node: reference.identifier, - message: msg, - fix: (fixer) => { - const sourceCode = context.getSourceCode(); - - const useStrict = /'use strict';\n\n?/g; - const hasUseStrict = !!useStrict.exec(sourceCode.text); - const firstLOC = sourceCode.ast.range[0]; - const rangeNeedle = hasUseStrict ? useStrict.lastIndex : firstLOC; - - return fixer.insertTextBeforeRange([rangeNeedle], `${fix}\n`); - } - }); - }; - } - - return { - 'Program:exit': function() { - const globalScope = context.getScope(); - let variable = globalScope.set.get('Buffer'); - if (variable) { - const fix = "const { Buffer } = require('buffer');"; - const msg = `Use ${fix} at the beginning of this file`; - variable.references.forEach(flagIt(msg, fix)); - } - variable = globalScope.set.get('URL'); - if (variable) { - const fix = "const { URL } = require('url');"; - const msg = `Use ${fix} at the beginning of this file`; - variable.references.forEach(flagIt(msg, fix)); - } - variable = globalScope.set.get('URLSearchParams'); - if (variable) { - const fix = "const { URLSearchParams } = require('url');"; - const msg = `Use ${fix} at the beginning of this file`; - variable.references.forEach(flagIt(msg, fix)); - } - } - }; -};