Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new ESLint rule: prefer-primordials #35448

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
103 changes: 39 additions & 64 deletions lib/.eslintrc.yaml
Expand Up @@ -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
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
- 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
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/event_target.js
Expand Up @@ -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');
Expand Down
130 changes: 87 additions & 43 deletions lib/internal/freeze_intrinsics.js
Expand Up @@ -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 */
Leko marked this conversation as resolved.
Show resolved Hide resolved
'use strict';

const {
Array,
ArrayBuffer,
ArrayPrototypeForEach,
BigInt,
BigInt64Array,
BigUint64Array,
Boolean,
DataView,
Date,
Error,
EvalError,
Float32Array,
Float64Array,
Function,
Int16Array,
Int32Array,
Int8Array,
JSON,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSON, Math and Reflect don’t actually exist on primordials, only their methods.


I’m fixing that in #36016.

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,
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -390,7 +434,7 @@ module.exports = function() {
}
}

defineProperty(obj, prop, {
ObjectDefineProperty(obj, prop, {
get: getter,
set: setter,
enumerable: desc.enumerable,
Expand All @@ -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]);
});
}
Expand Down
4 changes: 3 additions & 1 deletion lib/internal/fs/utils.js
Expand Up @@ -3,11 +3,13 @@
const {
ArrayIsArray,
BigInt,
Date,
DateNow,
ErrorCaptureStackTrace,
ObjectPrototypeHasOwnProperty,
Number,
NumberIsFinite,
NumberIsInteger,
MathMin,
ObjectSetPrototypeOf,
ReflectOwnKeys,
Expand Down Expand Up @@ -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') {
Expand Down