From cef144421c5ff6e9677ecf0b7a607000b744aa13 Mon Sep 17 00:00:00 2001 From: Leko Date: Wed, 30 Sep 2020 02:29:40 +0800 Subject: [PATCH] tools: add new ESLint rule: prefer-primordials MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I added a new custom ESLint rule to fix these problems. We have a lot of replaceable codes with primordials. Accessing built-in objects is restricted by existing rule (no-restricted-globals), but accessing property in the built-in objects is not restricted right now. We manually review codes that can be replaced by primordials, but there's a lot of code that actually needs to be fixed. We have often made pull requests to replace the primordials with. Restrict accessing global built-in objects such as `Promise`. Restrict calling static methods such as `Array.from` or `Symbol.for`. Don't restrict prototype methods to prevent false-positive. PR-URL: https://github.com/nodejs/node/pull/35448 Reviewed-By: Michaël Zasso Reviewed-By: Rich Trott Reviewed-By: Antoine du Hamel Reviewed-By: Daijiro Wachi Reviewed-By: Ben Coe --- lib/.eslintrc.yaml | 103 +++++------- lib/internal/event_target.js | 2 +- lib/internal/freeze_intrinsics.js | 130 +++++++++----- lib/internal/fs/utils.js | 4 +- lib/internal/http.js | 1 + lib/internal/main/print_help.js | 19 ++- lib/internal/per_context/primordials.js | 2 +- lib/internal/quic/core.js | 9 +- lib/internal/readline/utils.js | 4 +- lib/internal/timers.js | 3 +- lib/internal/util/iterable_weak_map.js | 4 +- lib/internal/vm/module.js | 3 +- lib/querystring.js | 5 +- lib/util.js | 1 + lib/v8.js | 1 + .../test-eslint-prefer-primordials.js | 158 ++++++++++++++++++ tools/eslint-rules/prefer-primordials.js | 156 +++++++++++++++++ 17 files changed, 477 insertions(+), 128 deletions(-) create mode 100644 test/parallel/test-eslint-prefer-primordials.js create mode 100644 tools/eslint-rules/prefer-primordials.js diff --git a/lib/.eslintrc.yaml b/lib/.eslintrc.yaml index 20e01898695507..37a43a223af2fd 100644 --- a/lib/.eslintrc.yaml +++ b/lib/.eslintrc.yaml @@ -7,105 +7,80 @@ rules: no-mixed-operators: - error - groups: [[ "&&", "||" ]] - no-restricted-globals: + no-restricted-syntax: + # Config copied from .eslintrc.js + - error + - selector: "CallExpression[callee.object.name='assert']:not([callee.property.name='ok']):not([callee.property.name='fail']):not([callee.property.name='ifError'])" + message: "Please only use simple assertions in ./lib" + - selector: "CallExpression[callee.name='setTimeout'][arguments.length<2]" + message: "setTimeout() must be invoked with at least two arguments." + - selector: "CallExpression[callee.name='setInterval'][arguments.length<2]" + message: "setInterval() must be invoked with at least 2 arguments." + - selector: "ThrowStatement > CallExpression[callee.name=/Error$/]" + message: "Use new keyword when throwing an Error." + # Config specific to lib + - selector: "NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError)$/])" + message: "Use an error exported by the internal/errors module." + - selector: "CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']" + message: "Please use `require('internal/errors').hideStackFrames()` instead." + - selector: "AssignmentExpression:matches([left.name='prepareStackTrace'], [left.property.name='prepareStackTrace'])" + message: "Use 'overrideStackTrace' from 'lib/internal/errors.js' instead of 'Error.prepareStackTrace'." + # Custom rules in tools/eslint-rules + node-core/lowercase-name-for-primitive: error + node-core/non-ascii-character: error + node-core/prefer-primordials: - error - name: Array - message: "Use `const { Array } = primordials;` instead of the global." - name: ArrayBuffer - message: "Use `const { ArrayBuffer } = primordials;` instead of the global." - name: BigInt - message: "Use `const { BigInt } = primordials;` instead of the global." - name: BigInt64Array - message: "Use `const { BigInt64Array } = primordials;` instead of the global." - name: BigUint64Array - message: "Use `const { BigUint64Array } = primordials;` instead of the global." - name: Boolean - message: "Use `const { Boolean } = primordials;` instead of the global." + - name: DataView + - name: Date - name: Error - message: "Use `const { Error } = primordials;` instead of the global." + ignore: + - stackTraceLimit + - captureStackTrace + - prepareStackTrace + - isPrototypeOf - name: EvalError - message: "Use `const { EvalError } = primordials;` instead of the global." - name: Float32Array - message: "Use `const { Float32Array } = primordials;` instead of the global." - name: Float64Array - message: "Use `const { Float64Array } = primordials;` instead of the global." + - name: Function - name: Int16Array - message: "Use `const { Int16Array } = primordials;` instead of the global." - name: Int32Array - message: "Use `const { Int32Array } = primordials;` instead of the global." - name: Int8Array - message: "Use `const { Int8Array } = primordials;` instead of the global." + - name: isFinite + into: Number + - name: isNaN + into: Number - name: JSON - message: "Use `const { JSON } = primordials;` instead of the global." - name: Map - message: "Use `const { Map } = primordials;` instead of the global." - name: Math - message: "Use `const { Math } = primordials;` instead of the global." - name: Number - message: "Use `const { Number } = primordials;` instead of the global." - name: Object - message: "Use `const { Object } = primordials;` instead of the global." + - name: parseFloat + into: Number + - name: parseInt + into: Number - name: Promise - message: "Use `const { Promise } = primordials;` instead of the global." - name: RangeError - message: "Use `const { RangeError } = primordials;` instead of the global." - name: ReferenceError - message: "Use `const { ReferenceError } = primordials;` instead of the global." - name: Reflect - message: "Use `const { Reflect } = primordials;` instead of the global." - name: RegExp - message: "Use `const { RegExp } = primordials;` instead of the global." - name: Set - message: "Use `const { Set } = primordials;` instead of the global." - name: String - message: "Use `const { String } = primordials;` instead of the global." - name: Symbol - message: "Use `const { Symbol } = primordials;` instead of the global." - name: SyntaxError - message: "Use `const { SyntaxError } = primordials;` instead of the global." - name: TypeError - message: "Use `const { TypeError } = primordials;` instead of the global." - - name: URIError - message: "Use `const { URIError } = primordials;` instead of the global." - name: Uint16Array - message: "Use `const { Uint16Array } = primordials;` instead of the global." - name: Uint32Array - message: "Use `const { Uint32Array } = primordials;` instead of the global." - name: Uint8Array - message: "Use `const { Uint8Array } = primordials;` instead of the global." - name: Uint8ClampedArray - message: "Use `const { Uint8ClampedArray } = primordials;` instead of the global." + - name: URIError - name: WeakMap - message: "Use `const { WeakMap } = primordials;` instead of the global." - name: WeakSet - message: "Use `const { WeakSet } = primordials;` instead of the global." - - name: parseFloat - message: "Use `const { NumberParseFloat } = primordials;` instead of the global." - - name: parseInt - message: "Use `const { NumberParseInt } = primordials;` instead of the global." - no-restricted-syntax: - # Config copied from .eslintrc.js - - error - - selector: "CallExpression[callee.object.name='assert']:not([callee.property.name='ok']):not([callee.property.name='fail']):not([callee.property.name='ifError'])" - message: "Please only use simple assertions in ./lib" - - selector: "CallExpression[callee.name='setTimeout'][arguments.length<2]" - message: "setTimeout() must be invoked with at least two arguments." - - selector: "CallExpression[callee.name='setInterval'][arguments.length<2]" - message: "setInterval() must be invoked with at least 2 arguments." - - selector: "ThrowStatement > CallExpression[callee.name=/Error$/]" - message: "Use new keyword when throwing an Error." - # Config specific to lib - - selector: "NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError)$/])" - message: "Use an error exported by the internal/errors module." - - selector: "CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']" - message: "Please use `require('internal/errors').hideStackFrames()` instead." - - selector: "AssignmentExpression:matches([left.name='prepareStackTrace'], [left.property.name='prepareStackTrace'])" - message: "Use 'overrideStackTrace' from 'lib/internal/errors.js' instead of 'Error.prepareStackTrace'." - - selector: "CallExpression[callee.name='isNaN']" - message: "Use NumberIsNaN() primordial instead of the global isNaN() function." - # Custom rules in tools/eslint-rules - node-core/lowercase-name-for-primitive: error - node-core/non-ascii-character: error globals: Intl: false # Parameters passed to internal modules diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index 12e04098320bc3..5fb9f4c6d62aa7 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -36,7 +36,7 @@ const kEvents = Symbol('kEvents'); const kStop = Symbol('kStop'); const kTarget = Symbol('kTarget'); -const kHybridDispatch = Symbol.for('nodejs.internal.kHybridDispatch'); +const kHybridDispatch = SymbolFor('nodejs.internal.kHybridDispatch'); const kCreateEvent = Symbol('kCreateEvent'); const kNewListener = Symbol('kNewListener'); const kRemoveListener = Symbol('kRemoveListener'); diff --git a/lib/internal/freeze_intrinsics.js b/lib/internal/freeze_intrinsics.js index dc64c13409b2f1..21612790ce749c 100644 --- a/lib/internal/freeze_intrinsics.js +++ b/lib/internal/freeze_intrinsics.js @@ -20,21 +20,61 @@ // https://github.com/tc39/proposal-ses/blob/e5271cc42a257a05dcae2fd94713ed2f46c08620/shim/src/freeze.js /* global WebAssembly, SharedArrayBuffer, console */ -/* eslint-disable no-restricted-globals */ 'use strict'; +const { + Array, + ArrayBuffer, + ArrayPrototypeForEach, + BigInt, + BigInt64Array, + BigUint64Array, + Boolean, + DataView, + Date, + Error, + EvalError, + Float32Array, + Float64Array, + Function, + Int16Array, + Int32Array, + Int8Array, + JSON, + Map, + Math, + Number, + Object, + ObjectDefineProperty, + ObjectFreeze, + ObjectGetOwnPropertyDescriptor, + ObjectGetOwnPropertyDescriptors, + ObjectGetOwnPropertyNames, + ObjectGetOwnPropertySymbols, + ObjectGetPrototypeOf, + ObjectPrototypeHasOwnProperty, + Promise, + RangeError, + ReferenceError, + Reflect, + ReflectOwnKeys, + RegExp, + Set, + String, + Symbol, + SymbolIterator, + SyntaxError, + TypeError, + Uint16Array, + Uint32Array, + Uint8Array, + Uint8ClampedArray, + URIError, + WeakMap, + WeakSet, +} = primordials; + module.exports = function() { - const { - defineProperty, - freeze, - getOwnPropertyDescriptor, - getOwnPropertyDescriptors, - getOwnPropertyNames, - getOwnPropertySymbols, - getPrototypeOf, - } = Object; - const objectHasOwnProperty = Object.prototype.hasOwnProperty; - const { ownKeys } = Reflect; const { clearImmediate, clearInterval, @@ -47,25 +87,25 @@ module.exports = function() { const intrinsicPrototypes = [ // Anonymous Intrinsics // IteratorPrototype - getPrototypeOf( - getPrototypeOf(new Array()[Symbol.iterator]()) + ObjectGetPrototypeOf( + ObjectGetPrototypeOf(new Array()[SymbolIterator]()) ), // ArrayIteratorPrototype - getPrototypeOf(new Array()[Symbol.iterator]()), + ObjectGetPrototypeOf(new Array()[SymbolIterator]()), // StringIteratorPrototype - getPrototypeOf(new String()[Symbol.iterator]()), + ObjectGetPrototypeOf(new String()[SymbolIterator]()), // MapIteratorPrototype - getPrototypeOf(new Map()[Symbol.iterator]()), + ObjectGetPrototypeOf(new Map()[SymbolIterator]()), // SetIteratorPrototype - getPrototypeOf(new Set()[Symbol.iterator]()), + ObjectGetPrototypeOf(new Set()[SymbolIterator]()), // GeneratorFunction - getPrototypeOf(function* () {}), + ObjectGetPrototypeOf(function* () {}), // AsyncFunction - getPrototypeOf(async function() {}), + ObjectGetPrototypeOf(async function() {}), // AsyncGeneratorFunction - getPrototypeOf(async function* () {}), + ObjectGetPrototypeOf(async function* () {}), // TypedArray - getPrototypeOf(Uint8Array), + ObjectGetPrototypeOf(Uint8Array), // 19 Fundamental Objects Object.prototype, // 19.1 @@ -129,33 +169,37 @@ module.exports = function() { const intrinsics = [ // Anonymous Intrinsics // ThrowTypeError - getOwnPropertyDescriptor(Function.prototype, 'caller').get, + ObjectGetOwnPropertyDescriptor(Function.prototype, 'caller').get, // IteratorPrototype - getPrototypeOf( - getPrototypeOf(new Array()[Symbol.iterator]()) + ObjectGetPrototypeOf( + ObjectGetPrototypeOf(new Array()[SymbolIterator]()) ), // ArrayIteratorPrototype - getPrototypeOf(new Array()[Symbol.iterator]()), + ObjectGetPrototypeOf(new Array()[SymbolIterator]()), // StringIteratorPrototype - getPrototypeOf(new String()[Symbol.iterator]()), + ObjectGetPrototypeOf(new String()[SymbolIterator]()), // MapIteratorPrototype - getPrototypeOf(new Map()[Symbol.iterator]()), + ObjectGetPrototypeOf(new Map()[SymbolIterator]()), // SetIteratorPrototype - getPrototypeOf(new Set()[Symbol.iterator]()), + ObjectGetPrototypeOf(new Set()[SymbolIterator]()), // GeneratorFunction - getPrototypeOf(function* () {}), + ObjectGetPrototypeOf(function* () {}), // AsyncFunction - getPrototypeOf(async function() {}), + ObjectGetPrototypeOf(async function() {}), // AsyncGeneratorFunction - getPrototypeOf(async function* () {}), + ObjectGetPrototypeOf(async function* () {}), // TypedArray - getPrototypeOf(Uint8Array), + ObjectGetPrototypeOf(Uint8Array), // 18 The Global Object eval, + // eslint-disable-next-line node-core/prefer-primordials isFinite, + // eslint-disable-next-line node-core/prefer-primordials isNaN, + // eslint-disable-next-line node-core/prefer-primordials parseFloat, + // eslint-disable-next-line node-core/prefer-primordials parseInt, decodeURI, decodeURIComponent, @@ -289,16 +333,16 @@ module.exports = function() { // Object are verified before being enqueued, // therefore this is a valid candidate. // Throws if this fails (strict mode). - freeze(obj); + ObjectFreeze(obj); // We rely upon certain commitments of Object.freeze and proxies here // Get stable/immutable outbound links before a Proxy has a chance to do // something sneaky. - const proto = getPrototypeOf(obj); - const descs = getOwnPropertyDescriptors(obj); + const proto = ObjectGetPrototypeOf(obj); + const descs = ObjectGetOwnPropertyDescriptors(obj); enqueue(proto); - ownKeys(descs).forEach((name) => { + ArrayPrototypeForEach(ReflectOwnKeys(descs), (name) => { // TODO: Uncurried form // TODO: getOwnPropertyDescriptors is guaranteed to return well-formed // descriptors, but they still inherit from Object.prototype. If @@ -378,10 +422,10 @@ module.exports = function() { `Cannot assign to read only property '${prop}' of object '${obj}'` ); } - if (objectHasOwnProperty.call(this, prop)) { + if (ObjectPrototypeHasOwnProperty(this, prop)) { this[prop] = newValue; } else { - defineProperty(this, prop, { + ObjectDefineProperty(this, prop, { value: newValue, writable: true, enumerable: true, @@ -390,7 +434,7 @@ module.exports = function() { } } - defineProperty(obj, prop, { + ObjectDefineProperty(obj, prop, { get: getter, set: setter, enumerable: desc.enumerable, @@ -403,14 +447,14 @@ module.exports = function() { if (!obj) { return; } - const descs = getOwnPropertyDescriptors(obj); + const descs = ObjectGetOwnPropertyDescriptors(obj); if (!descs) { return; } - getOwnPropertyNames(obj).forEach((prop) => { + ArrayPrototypeForEach(ObjectGetOwnPropertyNames(obj), (prop) => { return enableDerivedOverride(obj, prop, descs[prop]); }); - getOwnPropertySymbols(obj).forEach((prop) => { + ArrayPrototypeForEach(ObjectGetOwnPropertySymbols(obj), (prop) => { return enableDerivedOverride(obj, prop, descs[prop]); }); } diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 24e2224c2e3945..dcef28c556da46 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -3,11 +3,13 @@ const { ArrayIsArray, BigInt, + Date, DateNow, ErrorCaptureStackTrace, ObjectPrototypeHasOwnProperty, Number, NumberIsFinite, + NumberIsInteger, MathMin, ObjectSetPrototypeOf, ReflectOwnKeys, @@ -785,7 +787,7 @@ const getValidMode = hideStackFrames((mode, type) => { if (mode == null) { return def; } - if (Number.isInteger(mode) && mode >= min && mode <= max) { + if (NumberIsInteger(mode) && mode >= min && mode <= max) { return mode; } if (typeof mode !== 'number') { diff --git a/lib/internal/http.js b/lib/internal/http.js index aab4170a2f0e06..1d5973593e9ada 100644 --- a/lib/internal/http.js +++ b/lib/internal/http.js @@ -2,6 +2,7 @@ const { Symbol, + Date, } = primordials; const { setUnrefTimeout } = require('internal/timers'); diff --git a/lib/internal/main/print_help.js b/lib/internal/main/print_help.js index 34a8e19f08ec31..0850681882759f 100644 --- a/lib/internal/main/print_help.js +++ b/lib/internal/main/print_help.js @@ -1,6 +1,13 @@ 'use strict'; -/* eslint-disable no-restricted-globals */ +const { + Boolean, + Map, + MathFloor, + MathMax, + ObjectKeys, + RegExp, +} = primordials; const { types } = internalBinding('options'); const hasCrypto = Boolean(process.versions.openssl); @@ -10,7 +17,7 @@ const { } = require('internal/bootstrap/pre_execution'); const typeLookup = []; -for (const key of Object.keys(types)) +for (const key of ObjectKeys(types)) typeLookup[types[key]] = key; // Environment variables are parsed ad-hoc throughout the code base, @@ -127,7 +134,7 @@ function format({ options, aliases = new Map(), firstColumn, secondColumn }) { } text += displayName; - maxFirstColumnUsed = Math.max(maxFirstColumnUsed, displayName.length); + maxFirstColumnUsed = MathMax(maxFirstColumnUsed, displayName.length); if (displayName.length >= firstColumn) text += '\n' + ' '.repeat(firstColumn); else @@ -154,9 +161,9 @@ function print(stream) { const { options, aliases } = require('internal/options'); // Use 75 % of the available width, and at least 70 characters. - const width = Math.max(70, (stream.columns || 0) * 0.75); - const firstColumn = Math.floor(width * 0.4); - const secondColumn = Math.floor(width * 0.57); + const width = MathMax(70, (stream.columns || 0) * 0.75); + const firstColumn = MathFloor(width * 0.4); + const secondColumn = MathFloor(width * 0.57); options.set('-', { helpText: 'script read from stdin ' + '(default if no file name is provided, ' + diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index 8527e65f2ba7e1..ddecb18861c258 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -1,6 +1,6 @@ 'use strict'; -/* eslint-disable no-restricted-globals */ +/* eslint-disable node-core/prefer-primordials */ // This file subclasses and stores the JS builtins that come from the VM // so that Node.js's builtin modules do not need to later look these up from diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js index 2df63c2be67cdf..98a2861e1a76fe 100644 --- a/lib/internal/quic/core.js +++ b/lib/internal/quic/core.js @@ -10,7 +10,7 @@ const { assertCrypto(); const { - Array, + ArrayFrom, BigInt64Array, Boolean, Error, @@ -22,6 +22,7 @@ const { PromiseResolve, Set, Symbol, + SymbolFor, } = primordials; const { Buffer } = require('buffer'); @@ -242,7 +243,7 @@ const kUsePreferredAddress = Symbol('kUsePreferredAddress'); const kVersionNegotiation = Symbol('kVersionNegotiation'); const kWriteGeneric = Symbol('kWriteGeneric'); -const kRejections = Symbol.for('nodejs.rejection'); +const kRejections = SymbolFor('nodejs.rejection'); const kSocketUnbound = 0; const kSocketPending = 1; @@ -1020,7 +1021,7 @@ class QuicSocket extends EventEmitter { const state = this[kInternalState]; return customInspect(this, { endpoints: this.endpoints, - sessions: Array.from(state.sessions), + sessions: ArrayFrom(state.sessions), bound: this.bound, pending: this.pending, closing: this.closing, @@ -1470,7 +1471,7 @@ class QuicSocket extends EventEmitter { } get endpoints() { - return Array.from(this[kInternalState].endpoints); + return ArrayFrom(this[kInternalState].endpoints); } get serverSecureContext() { diff --git a/lib/internal/readline/utils.js b/lib/internal/readline/utils.js index 408d12d01e151d..cce4045a5a0b02 100644 --- a/lib/internal/readline/utils.js +++ b/lib/internal/readline/utils.js @@ -1,7 +1,7 @@ 'use strict'; const { - String, + StringFromCharCode, Symbol, } = primordials; @@ -325,7 +325,7 @@ function* emitKeys(stream) { key.meta = escaped; } else if (!escaped && ch <= '\x1a') { // ctrl+letter - key.name = String.fromCharCode(ch.charCodeAt(0) + 'a'.charCodeAt(0) - 1); + key.name = StringFromCharCode(ch.charCodeAt(0) + 'a'.charCodeAt(0) - 1); key.ctrl = true; } else if (/^[0-9A-Za-z]$/.test(ch)) { // Letter, number, shift+letter diff --git a/lib/internal/timers.js b/lib/internal/timers.js index d39bab9d074895..5009572f90f265 100644 --- a/lib/internal/timers.js +++ b/lib/internal/timers.js @@ -75,6 +75,7 @@ const { MathMax, MathTrunc, + NumberIsFinite, NumberMIN_SAFE_INTEGER, ObjectCreate, Symbol, @@ -380,7 +381,7 @@ function setUnrefTimeout(callback, after) { // Type checking used by timers.enroll() and Socket#setTimeout() function getTimerDuration(msecs, name) { validateNumber(msecs, name); - if (msecs < 0 || !isFinite(msecs)) { + if (msecs < 0 || !NumberIsFinite(msecs)) { throw new ERR_OUT_OF_RANGE(name, 'a non-negative finite number', msecs); } diff --git a/lib/internal/util/iterable_weak_map.js b/lib/internal/util/iterable_weak_map.js index 3fa139b23e4b4e..c9715a7e313b20 100644 --- a/lib/internal/util/iterable_weak_map.js +++ b/lib/internal/util/iterable_weak_map.js @@ -2,7 +2,7 @@ const { makeSafe, - Object, + ObjectFreeze, SafeSet, SafeWeakMap, SymbolIterator, @@ -79,7 +79,7 @@ function cleanup({ set, ref }) { set.delete(ref); } -Object.freeze(IterableWeakMap.prototype); +ObjectFreeze(IterableWeakMap.prototype); module.exports = { IterableWeakMap, diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index c10433a36f6dcb..ce37312d652943 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -9,6 +9,7 @@ const { ObjectSetPrototypeOf, SafePromise, Symbol, + SymbolToStringTag, TypeError, WeakMap, } = primordials; @@ -239,7 +240,7 @@ class Module { o.context = this.context; ObjectSetPrototypeOf(o, ObjectGetPrototypeOf(this)); - ObjectDefineProperty(o, Symbol.toStringTag, { + ObjectDefineProperty(o, SymbolToStringTag, { value: constructor.name, configurable: true }); diff --git a/lib/querystring.js b/lib/querystring.js index 04a21e8d07f24f..21ba41c7b4e1ad 100644 --- a/lib/querystring.js +++ b/lib/querystring.js @@ -27,6 +27,7 @@ const { Array, ArrayIsArray, MathAbs, + NumberIsFinite, ObjectCreate, ObjectKeys, String, @@ -156,7 +157,7 @@ function qsEscape(str) { function stringifyPrimitive(v) { if (typeof v === 'string') return v; - if (typeof v === 'number' && isFinite(v)) + if (typeof v === 'number' && NumberIsFinite(v)) return '' + v; if (typeof v === 'boolean') return v ? 'true' : 'false'; @@ -167,7 +168,7 @@ function stringifyPrimitive(v) { function encodeStringified(v, encode) { if (typeof v === 'string') return (v.length ? encode(v) : ''); - if (typeof v === 'number' && isFinite(v)) { + if (typeof v === 'number' && NumberIsFinite(v)) { // Values >= 1e21 automatically switch to scientific notation which requires // escaping due to the inclusion of a '+' in the output return (MathAbs(v) < 1e21 ? '' + v : encode('' + v)); diff --git a/lib/util.js b/lib/util.js index 1414c0d11d58a8..2d8687ac9c569a 100644 --- a/lib/util.js +++ b/lib/util.js @@ -23,6 +23,7 @@ const { ArrayIsArray, + Date, Error, NumberIsSafeInteger, ObjectDefineProperties, diff --git a/lib/v8.js b/lib/v8.js index 15fe57e63e36ce..9cc3412c341b54 100644 --- a/lib/v8.js +++ b/lib/v8.js @@ -17,6 +17,7 @@ const { Array, ArrayBuffer, + DataView, Error, Float32Array, Float64Array, diff --git a/test/parallel/test-eslint-prefer-primordials.js b/test/parallel/test-eslint-prefer-primordials.js new file mode 100644 index 00000000000000..d9417e857c2089 --- /dev/null +++ b/test/parallel/test-eslint-prefer-primordials.js @@ -0,0 +1,158 @@ +'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/prefer-primordials'); + +new RuleTester({ + parserOptions: { ecmaVersion: 6 }, + env: { es6: true } +}) + .run('prefer-primordials', rule, { + valid: [ + 'new Array()', + 'JSON.stringify({})', + 'class A { *[Symbol.iterator] () { yield "a"; } }', + 'const a = { *[Symbol.iterator] () { yield "a"; } }', + 'Object.defineProperty(o, Symbol.toStringTag, { value: "o" })', + 'parseInt("10")', + ` + const { Reflect } = primordials; + module.exports = function() { + const { ownKeys } = Reflect; + } + `, + { + code: 'const { Array } = primordials; new Array()', + options: [{ name: 'Array' }] + }, + { + code: 'const { JSONStringify } = primordials; JSONStringify({})', + options: [{ name: 'JSON' }] + }, + { + code: 'const { SymbolFor } = primordials; SymbolFor("xxx")', + options: [{ name: 'Symbol' }] + }, + { + code: ` + const { SymbolIterator } = primordials; + class A { *[SymbolIterator] () { yield "a"; } } + `, + options: [{ name: 'Symbol' }] + }, + { + code: ` + const { Symbol } = primordials; + const a = { *[Symbol.iterator] () { yield "a"; } } + `, + options: [{ name: 'Symbol', ignore: ['iterator'] }] + }, + { + code: ` + const { ObjectDefineProperty, Symbol } = primordials; + ObjectDefineProperty(o, Symbol.toStringTag, { value: "o" }) + `, + options: [{ name: 'Symbol', ignore: ['toStringTag'] }] + }, + { + code: 'const { Symbol } = primordials; Symbol.for("xxx")', + options: [{ name: 'Symbol', ignore: ['for'] }] + }, + { + code: 'const { NumberParseInt } = primordials; NumberParseInt("xxx")', + options: [{ name: 'parseInt' }] + }, + { + code: ` + const { ReflectOwnKeys } = primordials; + module.exports = function() { + ReflectOwnKeys({}) + } + `, + options: [{ name: 'Reflect' }], + }, + ], + invalid: [ + { + code: 'new Array()', + options: [{ name: 'Array' }], + errors: [{ message: /const { Array } = primordials/ }] + }, + { + code: 'JSON.parse("{}")', + options: [{ name: 'JSON' }], + errors: [ + { message: /const { JSONParse } = primordials/ }, + ] + }, + { + code: 'const { JSON } = primordials; JSON.parse("{}")', + options: [{ name: 'JSON' }], + errors: [{ message: /const { JSONParse } = primordials/ }] + }, + { + code: 'Symbol.for("xxx")', + options: [{ name: 'Symbol' }], + errors: [ + { message: /const { SymbolFor } = primordials/ }, + ] + }, + { + code: 'const { Symbol } = primordials; Symbol.for("xxx")', + options: [{ name: 'Symbol' }], + errors: [{ message: /const { SymbolFor } = primordials/ }] + }, + { + code: ` + const { Symbol } = primordials; + class A { *[Symbol.iterator] () { yield "a"; } } + `, + options: [{ name: 'Symbol' }], + errors: [{ message: /const { SymbolIterator } = primordials/ }] + }, + { + code: ` + const { Symbol } = primordials; + const a = { *[Symbol.iterator] () { yield "a"; } } + `, + options: [{ name: 'Symbol' }], + errors: [{ message: /const { SymbolIterator } = primordials/ }] + }, + { + code: ` + const { ObjectDefineProperty, Symbol } = primordials; + ObjectDefineProperty(o, Symbol.toStringTag, { value: "o" }) + `, + options: [{ name: 'Symbol' }], + errors: [{ message: /const { SymbolToStringTag } = primordials/ }] + }, + { + code: ` + const { Number } = primordials; + Number.parseInt('10') + `, + options: [{ name: 'Number', into: Number }], + errors: [{ message: /const { NumberParseInt } = primordials/ }] + }, + { + code: 'parseInt("10")', + options: [{ name: 'parseInt', into: 'Number' }], + errors: [{ message: /const { NumberParseInt } = primordials/ }] + }, + { + code: ` + module.exports = function() { + const { ownKeys } = Reflect; + } + `, + options: [{ name: 'Reflect' }], + errors: [{ message: /const { ReflectOwnKeys } = primordials/ }] + }, + ] + }); diff --git a/tools/eslint-rules/prefer-primordials.js b/tools/eslint-rules/prefer-primordials.js new file mode 100644 index 00000000000000..ffbb1e6e308c95 --- /dev/null +++ b/tools/eslint-rules/prefer-primordials.js @@ -0,0 +1,156 @@ +/** + * @fileoverview We shouldn't use global built-in object for security and + * performance reason. This linter rule reports replacable codes + * that can be replaced with primordials. + * @author Leko + */ +'use strict'; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +function toPrimordialsName(obj, prop) { + return obj + toUcFirst(prop); +} + +function toUcFirst(str) { + return str[0].toUpperCase() + str.slice(1); +} + +function isTarget(map, varName) { + return map.has(varName); +} + +function isIgnored(map, varName, propName) { + if (!map.has(varName) || !map.get(varName).has(propName)) { + return false; + } + return map.get(varName).get(propName).ignored; +} + +function getReportName({ name, parentName, into }) { + if (into) { + return toPrimordialsName(into, name); + } + if (parentName) { + return toPrimordialsName(parentName, name); + } + return name; +} + +/** + * Get identifier of object spread assignment + * + * code: 'const { ownKeys } = Reflect;' + * argument: 'ownKeys' + * return: 'Reflect' + */ +function getDestructuringAssignmentParent(scope, node) { + const declaration = scope.set.get(node.name); + if ( + !declaration || + !declaration.defs || + declaration.defs.length === 0 || + declaration.defs[0].type !== 'Variable' || + !declaration.defs[0].node.init + ) { + return null; + } + return declaration.defs[0].node.init.name; +} + +const identifierSelector = + '[type!=VariableDeclarator][type!=MemberExpression]>Identifier'; + +module.exports = { + meta: { + messages: { + error: 'Use `const { {{name}} } = primordials;` instead of the global.' + } + }, + create(context) { + const globalScope = context.getSourceCode().scopeManager.globalScope; + const nameMap = context.options.reduce((acc, option) => + acc.set( + option.name, + (option.ignore || []) + .concat(['prototype']) + .reduce((acc, name) => acc.set(name, { + ignored: true + }), new Map()) + ) + , new Map()); + const renameMap = context.options + .filter((option) => option.into) + .reduce((acc, option) => + acc.set(option.name, option.into) + , new Map()); + let reported; + + return { + Program() { + reported = new Map(); + }, + [identifierSelector](node) { + if (reported.has(node.range[0])) { + return; + } + const name = node.name; + const parentName = getDestructuringAssignmentParent( + context.getScope(), + node + ); + if (!isTarget(nameMap, name) && !isTarget(nameMap, parentName)) { + return; + } + + const defs = (globalScope.set.get(name) || {}).defs || null; + if (parentName && isTarget(nameMap, parentName)) { + if (!defs || defs[0].name.name !== 'primordials') { + reported.set(node.range[0], true); + const into = renameMap.get(name); + context.report({ + node, + messageId: 'error', + data: { + name: getReportName({ into, parentName, name }) + } + }); + } + return; + } + if (defs.length === 0 || defs[0].node.init.name !== 'primordials') { + reported.set(node.range[0], true); + const into = renameMap.get(name); + context.report({ + node, + messageId: 'error', + data: { + name: getReportName({ into, parentName, name }) + } + }); + } + }, + MemberExpression(node) { + const obj = node.object.name; + const prop = node.property.name; + if (!prop || !isTarget(nameMap, obj) || isIgnored(nameMap, obj, prop)) { + return; + } + + const variables = + context.getSourceCode().scopeManager.getDeclaredVariables(node); + if (variables.length === 0) { + context.report({ + node, + messageId: 'error', + data: { + name: toPrimordialsName(obj, prop), + } + }); + } + } + }; + } +};