Skip to content

Commit a7d365c

Browse files
targostargos
committed
tools: add new ESLint rule: prefer-primordials
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: #35448 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Ben Coe <[email protected]>
1 parent eee1d29 commit a7d365c

16 files changed

+473
-124
lines changed

‎lib/.eslintrc.yaml

+39-64
Original file line numberDiff line numberDiff line change
@@ -7,105 +7,80 @@ rules:
77
no-mixed-operators:
88
- error
99
- groups: [[ "&&", "||" ]]
10-
no-restricted-globals:
10+
no-restricted-syntax:
11+
# Config copied from .eslintrc.js
12+
- error
13+
- selector: "CallExpression[callee.object.name='assert']:not([callee.property.name='ok']):not([callee.property.name='fail']):not([callee.property.name='ifError'])"
14+
message: "Please only use simple assertions in ./lib"
15+
- selector: "CallExpression[callee.name='setTimeout'][arguments.length<2]"
16+
message: "setTimeout() must be invoked with at least two arguments."
17+
- selector: "CallExpression[callee.name='setInterval'][arguments.length<2]"
18+
message: "setInterval() must be invoked with at least 2 arguments."
19+
- selector: "ThrowStatement > CallExpression[callee.name=/Error$/]"
20+
message: "Use new keyword when throwing an Error."
21+
# Config specific to lib
22+
- selector: "NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError|AbortError)$/])"
23+
message: "Use an error exported by the internal/errors module."
24+
- selector: "CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']"
25+
message: "Please use `require('internal/errors').hideStackFrames()` instead."
26+
- selector: "AssignmentExpression:matches([left.name='prepareStackTrace'], [left.property.name='prepareStackTrace'])"
27+
message: "Use 'overrideStackTrace' from 'lib/internal/errors.js' instead of 'Error.prepareStackTrace'."
28+
# Custom rules in tools/eslint-rules
29+
node-core/lowercase-name-for-primitive: error
30+
node-core/non-ascii-character: error
31+
node-core/prefer-primordials:
1132
- error
1233
- name: Array
13-
message: "Use `const { Array } = primordials;` instead of the global."
1434
- name: ArrayBuffer
15-
message: "Use `const { ArrayBuffer } = primordials;` instead of the global."
1635
- name: BigInt
17-
message: "Use `const { BigInt } = primordials;` instead of the global."
1836
- name: BigInt64Array
19-
message: "Use `const { BigInt64Array } = primordials;` instead of the global."
2037
- name: BigUint64Array
21-
message: "Use `const { BigUint64Array } = primordials;` instead of the global."
2238
- name: Boolean
23-
message: "Use `const { Boolean } = primordials;` instead of the global."
39+
- name: DataView
40+
- name: Date
2441
- name: Error
25-
message: "Use `const { Error } = primordials;` instead of the global."
42+
ignore:
43+
- stackTraceLimit
44+
- captureStackTrace
45+
- prepareStackTrace
46+
- isPrototypeOf
2647
- name: EvalError
27-
message: "Use `const { EvalError } = primordials;` instead of the global."
2848
- name: Float32Array
29-
message: "Use `const { Float32Array } = primordials;` instead of the global."
3049
- name: Float64Array
31-
message: "Use `const { Float64Array } = primordials;` instead of the global."
50+
- name: Function
3251
- name: Int16Array
33-
message: "Use `const { Int16Array } = primordials;` instead of the global."
3452
- name: Int32Array
35-
message: "Use `const { Int32Array } = primordials;` instead of the global."
3653
- name: Int8Array
37-
message: "Use `const { Int8Array } = primordials;` instead of the global."
54+
- name: isFinite
55+
into: Number
56+
- name: isNaN
57+
into: Number
3858
- name: JSON
39-
message: "Use `const { JSON } = primordials;` instead of the global."
4059
- name: Map
41-
message: "Use `const { Map } = primordials;` instead of the global."
4260
- name: Math
43-
message: "Use `const { Math } = primordials;` instead of the global."
4461
- name: Number
45-
message: "Use `const { Number } = primordials;` instead of the global."
4662
- name: Object
47-
message: "Use `const { Object } = primordials;` instead of the global."
63+
- name: parseFloat
64+
into: Number
65+
- name: parseInt
66+
into: Number
4867
- name: Promise
49-
message: "Use `const { Promise } = primordials;` instead of the global."
5068
- name: RangeError
51-
message: "Use `const { RangeError } = primordials;` instead of the global."
5269
- name: ReferenceError
53-
message: "Use `const { ReferenceError } = primordials;` instead of the global."
5470
- name: Reflect
55-
message: "Use `const { Reflect } = primordials;` instead of the global."
5671
- name: RegExp
57-
message: "Use `const { RegExp } = primordials;` instead of the global."
5872
- name: Set
59-
message: "Use `const { Set } = primordials;` instead of the global."
6073
- name: String
61-
message: "Use `const { String } = primordials;` instead of the global."
6274
- name: Symbol
63-
message: "Use `const { Symbol } = primordials;` instead of the global."
6475
- name: SyntaxError
65-
message: "Use `const { SyntaxError } = primordials;` instead of the global."
6676
- name: TypeError
67-
message: "Use `const { TypeError } = primordials;` instead of the global."
68-
- name: URIError
69-
message: "Use `const { URIError } = primordials;` instead of the global."
7077
- name: Uint16Array
71-
message: "Use `const { Uint16Array } = primordials;` instead of the global."
7278
- name: Uint32Array
73-
message: "Use `const { Uint32Array } = primordials;` instead of the global."
7479
- name: Uint8Array
75-
message: "Use `const { Uint8Array } = primordials;` instead of the global."
7680
- name: Uint8ClampedArray
77-
message: "Use `const { Uint8ClampedArray } = primordials;` instead of the global."
81+
- name: URIError
7882
- name: WeakMap
79-
message: "Use `const { WeakMap } = primordials;` instead of the global."
8083
- name: WeakSet
81-
message: "Use `const { WeakSet } = primordials;` instead of the global."
82-
- name: parseFloat
83-
message: "Use `const { NumberParseFloat } = primordials;` instead of the global."
84-
- name: parseInt
85-
message: "Use `const { NumberParseInt } = primordials;` instead of the global."
86-
no-restricted-syntax:
87-
# Config copied from .eslintrc.js
88-
- error
89-
- selector: "CallExpression[callee.object.name='assert']:not([callee.property.name='ok']):not([callee.property.name='fail']):not([callee.property.name='ifError'])"
90-
message: "Please only use simple assertions in ./lib"
91-
- selector: "CallExpression[callee.name='setTimeout'][arguments.length<2]"
92-
message: "setTimeout() must be invoked with at least two arguments."
93-
- selector: "CallExpression[callee.name='setInterval'][arguments.length<2]"
94-
message: "setInterval() must be invoked with at least 2 arguments."
95-
- selector: "ThrowStatement > CallExpression[callee.name=/Error$/]"
96-
message: "Use new keyword when throwing an Error."
97-
# Config specific to lib
98-
- selector: "NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError|AbortError)$/])"
99-
message: "Use an error exported by the internal/errors module."
100-
- selector: "CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']"
101-
message: "Please use `require('internal/errors').hideStackFrames()` instead."
102-
- selector: "AssignmentExpression:matches([left.name='prepareStackTrace'], [left.property.name='prepareStackTrace'])"
103-
message: "Use 'overrideStackTrace' from 'lib/internal/errors.js' instead of 'Error.prepareStackTrace'."
104-
- selector: "CallExpression[callee.name='isNaN']"
105-
message: "Use NumberIsNaN() primordial instead of the global isNaN() function."
106-
# Custom rules in tools/eslint-rules
107-
node-core/lowercase-name-for-primitive: error
108-
node-core/non-ascii-character: error
10984
globals:
11085
Intl: false
11186
# Parameters passed to internal modules

‎lib/internal/event_target.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ const kStop = Symbol('kStop');
4343
const kTarget = Symbol('kTarget');
4444
const kHandlers = Symbol('khandlers');
4545

46-
const kHybridDispatch = Symbol.for('nodejs.internal.kHybridDispatch');
46+
const kHybridDispatch = SymbolFor('nodejs.internal.kHybridDispatch');
4747
const kCreateEvent = Symbol('kCreateEvent');
4848
const kNewListener = Symbol('kNewListener');
4949
const kRemoveListener = Symbol('kRemoveListener');

‎lib/internal/freeze_intrinsics.js

+87-43
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,61 @@
2020
// https://github.com/tc39/proposal-ses/blob/e5271cc42a257a05dcae2fd94713ed2f46c08620/shim/src/freeze.js
2121

2222
/* global WebAssembly, SharedArrayBuffer, console */
23-
/* eslint-disable no-restricted-globals */
2423
'use strict';
2524

25+
const {
26+
Array,
27+
ArrayBuffer,
28+
ArrayPrototypeForEach,
29+
BigInt,
30+
BigInt64Array,
31+
BigUint64Array,
32+
Boolean,
33+
DataView,
34+
Date,
35+
Error,
36+
EvalError,
37+
Float32Array,
38+
Float64Array,
39+
Function,
40+
Int16Array,
41+
Int32Array,
42+
Int8Array,
43+
JSON,
44+
Map,
45+
Math,
46+
Number,
47+
Object,
48+
ObjectDefineProperty,
49+
ObjectFreeze,
50+
ObjectGetOwnPropertyDescriptor,
51+
ObjectGetOwnPropertyDescriptors,
52+
ObjectGetOwnPropertyNames,
53+
ObjectGetOwnPropertySymbols,
54+
ObjectGetPrototypeOf,
55+
ObjectPrototypeHasOwnProperty,
56+
Promise,
57+
RangeError,
58+
ReferenceError,
59+
Reflect,
60+
ReflectOwnKeys,
61+
RegExp,
62+
Set,
63+
String,
64+
Symbol,
65+
SymbolIterator,
66+
SyntaxError,
67+
TypeError,
68+
Uint16Array,
69+
Uint32Array,
70+
Uint8Array,
71+
Uint8ClampedArray,
72+
URIError,
73+
WeakMap,
74+
WeakSet,
75+
} = primordials;
76+
2677
module.exports = function() {
27-
const {
28-
defineProperty,
29-
freeze,
30-
getOwnPropertyDescriptor,
31-
getOwnPropertyDescriptors,
32-
getOwnPropertyNames,
33-
getOwnPropertySymbols,
34-
getPrototypeOf,
35-
} = Object;
36-
const objectHasOwnProperty = Object.prototype.hasOwnProperty;
37-
const { ownKeys } = Reflect;
3878
const {
3979
clearImmediate,
4080
clearInterval,
@@ -47,25 +87,25 @@ module.exports = function() {
4787
const intrinsicPrototypes = [
4888
// Anonymous Intrinsics
4989
// IteratorPrototype
50-
getPrototypeOf(
51-
getPrototypeOf(new Array()[Symbol.iterator]())
90+
ObjectGetPrototypeOf(
91+
ObjectGetPrototypeOf(new Array()[SymbolIterator]())
5292
),
5393
// ArrayIteratorPrototype
54-
getPrototypeOf(new Array()[Symbol.iterator]()),
94+
ObjectGetPrototypeOf(new Array()[SymbolIterator]()),
5595
// StringIteratorPrototype
56-
getPrototypeOf(new String()[Symbol.iterator]()),
96+
ObjectGetPrototypeOf(new String()[SymbolIterator]()),
5797
// MapIteratorPrototype
58-
getPrototypeOf(new Map()[Symbol.iterator]()),
98+
ObjectGetPrototypeOf(new Map()[SymbolIterator]()),
5999
// SetIteratorPrototype
60-
getPrototypeOf(new Set()[Symbol.iterator]()),
100+
ObjectGetPrototypeOf(new Set()[SymbolIterator]()),
61101
// GeneratorFunction
62-
getPrototypeOf(function* () {}),
102+
ObjectGetPrototypeOf(function* () {}),
63103
// AsyncFunction
64-
getPrototypeOf(async function() {}),
104+
ObjectGetPrototypeOf(async function() {}),
65105
// AsyncGeneratorFunction
66-
getPrototypeOf(async function* () {}),
106+
ObjectGetPrototypeOf(async function* () {}),
67107
// TypedArray
68-
getPrototypeOf(Uint8Array),
108+
ObjectGetPrototypeOf(Uint8Array),
69109

70110
// 19 Fundamental Objects
71111
Object.prototype, // 19.1
@@ -129,33 +169,37 @@ module.exports = function() {
129169
const intrinsics = [
130170
// Anonymous Intrinsics
131171
// ThrowTypeError
132-
getOwnPropertyDescriptor(Function.prototype, 'caller').get,
172+
ObjectGetOwnPropertyDescriptor(Function.prototype, 'caller').get,
133173
// IteratorPrototype
134-
getPrototypeOf(
135-
getPrototypeOf(new Array()[Symbol.iterator]())
174+
ObjectGetPrototypeOf(
175+
ObjectGetPrototypeOf(new Array()[SymbolIterator]())
136176
),
137177
// ArrayIteratorPrototype
138-
getPrototypeOf(new Array()[Symbol.iterator]()),
178+
ObjectGetPrototypeOf(new Array()[SymbolIterator]()),
139179
// StringIteratorPrototype
140-
getPrototypeOf(new String()[Symbol.iterator]()),
180+
ObjectGetPrototypeOf(new String()[SymbolIterator]()),
141181
// MapIteratorPrototype
142-
getPrototypeOf(new Map()[Symbol.iterator]()),
182+
ObjectGetPrototypeOf(new Map()[SymbolIterator]()),
143183
// SetIteratorPrototype
144-
getPrototypeOf(new Set()[Symbol.iterator]()),
184+
ObjectGetPrototypeOf(new Set()[SymbolIterator]()),
145185
// GeneratorFunction
146-
getPrototypeOf(function* () {}),
186+
ObjectGetPrototypeOf(function* () {}),
147187
// AsyncFunction
148-
getPrototypeOf(async function() {}),
188+
ObjectGetPrototypeOf(async function() {}),
149189
// AsyncGeneratorFunction
150-
getPrototypeOf(async function* () {}),
190+
ObjectGetPrototypeOf(async function* () {}),
151191
// TypedArray
152-
getPrototypeOf(Uint8Array),
192+
ObjectGetPrototypeOf(Uint8Array),
153193

154194
// 18 The Global Object
155195
eval,
196+
// eslint-disable-next-line node-core/prefer-primordials
156197
isFinite,
198+
// eslint-disable-next-line node-core/prefer-primordials
157199
isNaN,
200+
// eslint-disable-next-line node-core/prefer-primordials
158201
parseFloat,
202+
// eslint-disable-next-line node-core/prefer-primordials
159203
parseInt,
160204
decodeURI,
161205
decodeURIComponent,
@@ -289,16 +333,16 @@ module.exports = function() {
289333
// Object are verified before being enqueued,
290334
// therefore this is a valid candidate.
291335
// Throws if this fails (strict mode).
292-
freeze(obj);
336+
ObjectFreeze(obj);
293337

294338
// We rely upon certain commitments of Object.freeze and proxies here
295339

296340
// Get stable/immutable outbound links before a Proxy has a chance to do
297341
// something sneaky.
298-
const proto = getPrototypeOf(obj);
299-
const descs = getOwnPropertyDescriptors(obj);
342+
const proto = ObjectGetPrototypeOf(obj);
343+
const descs = ObjectGetOwnPropertyDescriptors(obj);
300344
enqueue(proto);
301-
ownKeys(descs).forEach((name) => {
345+
ArrayPrototypeForEach(ReflectOwnKeys(descs), (name) => {
302346
// TODO: Uncurried form
303347
// TODO: getOwnPropertyDescriptors is guaranteed to return well-formed
304348
// descriptors, but they still inherit from Object.prototype. If
@@ -378,10 +422,10 @@ module.exports = function() {
378422
`Cannot assign to read only property '${prop}' of object '${obj}'`
379423
);
380424
}
381-
if (objectHasOwnProperty.call(this, prop)) {
425+
if (ObjectPrototypeHasOwnProperty(this, prop)) {
382426
this[prop] = newValue;
383427
} else {
384-
defineProperty(this, prop, {
428+
ObjectDefineProperty(this, prop, {
385429
value: newValue,
386430
writable: true,
387431
enumerable: true,
@@ -390,7 +434,7 @@ module.exports = function() {
390434
}
391435
}
392436

393-
defineProperty(obj, prop, {
437+
ObjectDefineProperty(obj, prop, {
394438
get: getter,
395439
set: setter,
396440
enumerable: desc.enumerable,
@@ -403,14 +447,14 @@ module.exports = function() {
403447
if (!obj) {
404448
return;
405449
}
406-
const descs = getOwnPropertyDescriptors(obj);
450+
const descs = ObjectGetOwnPropertyDescriptors(obj);
407451
if (!descs) {
408452
return;
409453
}
410-
getOwnPropertyNames(obj).forEach((prop) => {
454+
ArrayPrototypeForEach(ObjectGetOwnPropertyNames(obj), (prop) => {
411455
return enableDerivedOverride(obj, prop, descs[prop]);
412456
});
413-
getOwnPropertySymbols(obj).forEach((prop) => {
457+
ArrayPrototypeForEach(ObjectGetOwnPropertySymbols(obj), (prop) => {
414458
return enableDerivedOverride(obj, prop, descs[prop]);
415459
});
416460
}

‎lib/internal/fs/utils.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33
const {
44
ArrayIsArray,
55
BigInt,
6+
Date,
67
DateNow,
78
ErrorCaptureStackTrace,
89
ObjectPrototypeHasOwnProperty,
910
Number,
1011
NumberIsFinite,
12+
NumberIsInteger,
1113
MathMin,
1214
ObjectSetPrototypeOf,
1315
ReflectOwnKeys,
@@ -758,7 +760,7 @@ const getValidMode = hideStackFrames((mode, type) => {
758760
if (mode == null) {
759761
return def;
760762
}
761-
if (Number.isInteger(mode) && mode >= min && mode <= max) {
763+
if (NumberIsInteger(mode) && mode >= min && mode <= max) {
762764
return mode;
763765
}
764766
if (typeof mode !== 'number') {

0 commit comments

Comments
 (0)