From c04a2df1857b4fa004d6097ee871bfa9a601d97b Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 19 Nov 2020 23:20:26 +0100 Subject: [PATCH] fs: refactor to use more primordials PR-URL: https://github.com/nodejs/node/pull/36196 Reviewed-By: James M Snell --- lib/fs.js | 51 ++++++++++++++++------------ lib/internal/fs/dir.js | 23 ++++++++----- lib/internal/fs/promises.js | 37 ++++++++++++-------- lib/internal/fs/read_file_context.js | 6 ++-- lib/internal/fs/rimraf.js | 11 +++--- lib/internal/fs/streams.js | 9 ++--- lib/internal/fs/sync_write_stream.js | 3 +- lib/internal/fs/utils.js | 27 +++++++++------ lib/internal/fs/watchers.js | 5 +-- 9 files changed, 104 insertions(+), 68 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index cb9312c9d13fbe..39f35371393f4a 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -33,7 +33,8 @@ const kIoMaxLength = 2 ** 31 - 1; // in case they are created but never used due to an exception. const { - Map, + ArrayPrototypePush, + BigIntPrototypeToString, MathMax, Number, NumberIsSafeInteger, @@ -41,7 +42,13 @@ const { ObjectDefineProperties, ObjectDefineProperty, Promise, + ReflectApply, + RegExpPrototypeExec, + SafeMap, String, + StringPrototypeCharCodeAt, + StringPrototypeIndexOf, + StringPrototypeSlice, } = primordials; const { fs: constants } = internalBinding('constants'); @@ -329,7 +336,7 @@ function readFile(path, options, callback) { } if (context.isUserFd) { process.nextTick(function tick(context) { - readFileAfterOpen.call({ context }, null, path); + ReflectApply(readFileAfterOpen, { context }, [null, path]); }, context); return; } @@ -414,7 +421,7 @@ function readFileSync(path, options) { buffer = Buffer.allocUnsafe(8192); bytesRead = tryReadSync(fd, isUserFd, buffer, 0, 8192); if (bytesRead !== 0) { - buffers.push(buffer.slice(0, bytesRead)); + ArrayPrototypePush(buffers, buffer.slice(0, bytesRead)); } pos += bytesRead; } while (bytesRead !== 0); @@ -1580,7 +1587,7 @@ function watch(filename, options, listener) { } -const statWatchers = new Map(); +const statWatchers = new SafeMap(); function watchFile(filename, options, listener) { filename = getValidatedPath(filename); @@ -1652,13 +1659,13 @@ if (isWindows) { // slash. const splitRootRe = /^(?:[a-zA-Z]:|[\\/]{2}[^\\/]+[\\/][^\\/]+)?[\\/]*/; splitRoot = function splitRoot(str) { - return splitRootRe.exec(str)[0]; + return RegExpPrototypeExec(splitRootRe, str)[0]; }; } else { splitRoot = function splitRoot(str) { for (let i = 0; i < str.length; ++i) { - if (str.charCodeAt(i) !== CHAR_FORWARD_SLASH) - return str.slice(0, i); + if (StringPrototypeCharCodeAt(str, i) !== CHAR_FORWARD_SLASH) + return StringPrototypeSlice(str, 0, i); } return str; }; @@ -1679,7 +1686,7 @@ let nextPart; if (isWindows) { nextPart = function nextPart(p, i) { for (; i < p.length; ++i) { - const ch = p.charCodeAt(i); + const ch = StringPrototypeCharCodeAt(p, i); // Check for a separator character if (ch === CHAR_BACKWARD_SLASH || ch === CHAR_FORWARD_SLASH) @@ -1688,7 +1695,9 @@ if (isWindows) { return -1; }; } else { - nextPart = function nextPart(p, i) { return p.indexOf('/', i); }; + nextPart = function nextPart(p, i) { + return StringPrototypeIndexOf(p, '/', i); + }; } const emptyObj = ObjectCreate(null); @@ -1740,13 +1749,13 @@ function realpathSync(p, options) { const result = nextPart(p, pos); previous = current; if (result === -1) { - const last = p.slice(pos); + const last = StringPrototypeSlice(p, pos); current += last; base = previous + last; pos = p.length; } else { - current += p.slice(pos, result + 1); - base = previous + p.slice(pos, result); + current += StringPrototypeSlice(p, pos, result + 1); + base = previous + StringPrototypeSlice(p, pos, result); pos = result + 1; } @@ -1783,8 +1792,8 @@ function realpathSync(p, options) { let linkTarget = null; let id; if (!isWindows) { - const dev = stats[0].toString(32); - const ino = stats[7].toString(32); + const dev = BigIntPrototypeToString(stats[0], 32); + const ino = BigIntPrototypeToString(stats[7], 32); id = `${dev}:${ino}`; if (seenLinks[id]) { linkTarget = seenLinks[id]; @@ -1804,7 +1813,7 @@ function realpathSync(p, options) { } // Resolve the link, then start over - p = pathModule.resolve(resolvedLink, p.slice(pos)); + p = pathModule.resolve(resolvedLink, StringPrototypeSlice(p, pos)); // Skip over roots current = base = splitRoot(p); @@ -1883,13 +1892,13 @@ function realpath(p, options, callback) { const result = nextPart(p, pos); previous = current; if (result === -1) { - const last = p.slice(pos); + const last = StringPrototypeSlice(p, pos); current += last; base = previous + last; pos = p.length; } else { - current += p.slice(pos, result + 1); - base = previous + p.slice(pos, result); + current += StringPrototypeSlice(p, pos, result + 1); + base = previous + StringPrototypeSlice(p, pos, result); pos = result + 1; } @@ -1919,8 +1928,8 @@ function realpath(p, options, callback) { // `dev`/`ino` always return 0 on windows, so skip the check. let id; if (!isWindows) { - const dev = stats.dev.toString(32); - const ino = stats.ino.toString(32); + const dev = BigIntPrototypeToString(stats.dev, 32); + const ino = BigIntPrototypeToString(stats.ino, 32); id = `${dev}:${ino}`; if (seenLinks[id]) { return gotTarget(null, seenLinks[id]); @@ -1944,7 +1953,7 @@ function realpath(p, options, callback) { function gotResolvedLink(resolvedLink) { // Resolve the link, then start over - p = pathModule.resolve(resolvedLink, p.slice(pos)); + p = pathModule.resolve(resolvedLink, StringPrototypeSlice(p, pos)); current = base = splitRoot(p); pos = current.length; diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js index b4cd72d3668e25..d3e84500936176 100644 --- a/lib/internal/fs/dir.js +++ b/lib/internal/fs/dir.js @@ -1,6 +1,10 @@ 'use strict'; const { + ArrayPrototypePush, + ArrayPrototypeSlice, + ArrayPrototypeSplice, + FunctionPrototypeBind, ObjectDefineProperty, Symbol, SymbolAsyncIterator, @@ -61,9 +65,10 @@ class Dir { validateUint32(this[kDirOptions].bufferSize, 'options.bufferSize', true); - this[kDirReadPromisified] = - internalUtil.promisify(this[kDirReadImpl]).bind(this, false); - this[kDirClosePromisified] = internalUtil.promisify(this.close).bind(this); + this[kDirReadPromisified] = FunctionPrototypeBind( + internalUtil.promisify(this[kDirReadImpl]), this, false); + this[kDirClosePromisified] = FunctionPrototypeBind( + internalUtil.promisify(this.close), this); } get path() { @@ -86,14 +91,15 @@ class Dir { } if (this[kDirOperationQueue] !== null) { - this[kDirOperationQueue].push(() => { + ArrayPrototypePush(this[kDirOperationQueue], () => { this[kDirReadImpl](maybeSync, callback); }); return; } if (this[kDirBufferedEntries].length > 0) { - const [ name, type ] = this[kDirBufferedEntries].splice(0, 2); + const [ name, type ] = + ArrayPrototypeSplice(this[kDirBufferedEntries], 0, 2); if (maybeSync) process.nextTick(getDirent, this[kDirPath], name, type, callback); else @@ -113,7 +119,7 @@ class Dir { return callback(err, result); } - this[kDirBufferedEntries] = result.slice(2); + this[kDirBufferedEntries] = ArrayPrototypeSlice(result, 2); getDirent(this[kDirPath], result[0], result[1], callback); }; @@ -135,7 +141,8 @@ class Dir { } if (this[kDirBufferedEntries].length > 0) { - const [ name, type ] = this[kDirBufferedEntries].splice(0, 2); + const [ name, type ] = + ArrayPrototypeSplice(this[kDirBufferedEntries], 0, 2); return getDirent(this[kDirPath], name, type); } @@ -152,7 +159,7 @@ class Dir { return result; } - this[kDirBufferedEntries] = result.slice(2); + this[kDirBufferedEntries] = ArrayPrototypeSlice(result, 2); return getDirent(this[kDirPath], result[0], result[1]); } diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index bdd038b18cd6e5..93d37a07e751e6 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -10,12 +10,14 @@ const kReadFileMaxChunkSize = 2 ** 14; const kWriteFileMaxChunkSize = 2 ** 14; const { + ArrayPrototypePush, Error, MathMax, MathMin, NumberIsSafeInteger, Promise, PromisePrototypeFinally, + PromisePrototypeThen, PromiseResolve, Symbol, Uint8Array, @@ -178,18 +180,21 @@ class FileHandle extends JSTransferable { this[kRefs]--; if (this[kRefs] === 0) { this[kFd] = -1; - this[kClosePromise] = this[kHandle].close().finally(() => { - this[kClosePromise] = undefined; - }); + this[kClosePromise] = PromisePrototypeFinally( + this[kHandle].close(), + () => { this[kClosePromise] = undefined; } + ); } else { - this[kClosePromise] = new Promise((resolve, reject) => { - this[kCloseResolve] = resolve; - this[kCloseReject] = reject; - }).finally(() => { - this[kClosePromise] = undefined; - this[kCloseReject] = undefined; - this[kCloseResolve] = undefined; - }); + this[kClosePromise] = PromisePrototypeFinally( + new Promise((resolve, reject) => { + this[kCloseResolve] = resolve; + this[kCloseReject] = reject; + }), () => { + this[kClosePromise] = undefined; + this[kCloseReject] = undefined; + this[kCloseResolve] = undefined; + } + ); } return this[kClosePromise]; @@ -243,9 +248,11 @@ async function fsCall(fn, handle, ...args) { handle[kRefs]--; if (handle[kRefs] === 0) { handle[kFd] = -1; - handle[kHandle] - .close() - .then(handle[kCloseResolve], handle[kCloseReject]); + PromisePrototypeThen( + handle[kHandle].close(), + handle[kCloseResolve], + handle[kCloseReject] + ); } } } @@ -307,7 +314,7 @@ async function readFileHandle(filehandle, options) { await read(filehandle, buf, 0, chunkSize, -1); endOfFile = bytesRead === 0; if (bytesRead > 0) - chunks.push(buffer.slice(0, bytesRead)); + ArrayPrototypePush(chunks, buffer.slice(0, bytesRead)); } while (!endOfFile); const result = Buffer.concat(chunks); diff --git a/lib/internal/fs/read_file_context.js b/lib/internal/fs/read_file_context.js index faca0e0c331e39..10ba44e9c67bfc 100644 --- a/lib/internal/fs/read_file_context.js +++ b/lib/internal/fs/read_file_context.js @@ -1,7 +1,9 @@ 'use strict'; const { + ArrayPrototypePush, MathMin, + ReflectApply, } = primordials; const { Buffer } = require('buffer'); @@ -42,7 +44,7 @@ function readFileAfterRead(err, bytesRead) { // Unknown size, just read until we don't get bytes. const buffer = bytesRead === kReadFileUnknownBufferLength ? context.buffer : context.buffer.slice(0, bytesRead); - context.buffers.push(buffer); + ArrayPrototypePush(context.buffers, buffer); } context.read(); } @@ -118,7 +120,7 @@ class ReadFileContext { close(err) { if (this.isUserFd) { process.nextTick(function tick(context) { - readFileAfterClose.call({ context }, null); + ReflectApply(readFileAfterClose, { context }, [null]); }, this); return; } diff --git a/lib/internal/fs/rimraf.js b/lib/internal/fs/rimraf.js index e67b14a418c884..b0260be9ad1b51 100644 --- a/lib/internal/fs/rimraf.js +++ b/lib/internal/fs/rimraf.js @@ -7,8 +7,9 @@ 'use strict'; const { + ArrayPrototypeForEach, Promise, - Set, + SafeSet, } = primordials; const { Buffer } = require('buffer'); @@ -30,8 +31,8 @@ const { const { sep } = require('path'); const { setTimeout } = require('timers'); const { sleep } = require('internal/util'); -const notEmptyErrorCodes = new Set(['ENOTEMPTY', 'EEXIST', 'EPERM']); -const retryErrorCodes = new Set( +const notEmptyErrorCodes = new SafeSet(['ENOTEMPTY', 'EEXIST', 'EPERM']); +const retryErrorCodes = new SafeSet( ['EBUSY', 'EMFILE', 'ENFILE', 'ENOTEMPTY', 'EPERM']); const isWindows = process.platform === 'win32'; const epermHandler = isWindows ? fixWinEPERM : _rmdir; @@ -139,7 +140,7 @@ function _rmchildren(path, options, callback) { let done = false; - files.forEach((child) => { + ArrayPrototypeForEach(files, (child) => { const childPath = Buffer.concat([pathBuf, separator, child]); rimraf(childPath, options, (err) => { @@ -240,7 +241,7 @@ function _rmdirSync(path, options, originalErr) { // around that issue by retrying on Windows. const pathBuf = Buffer.from(path); - readdirSync(pathBuf, readdirEncoding).forEach((child) => { + ArrayPrototypeForEach(readdirSync(pathBuf, readdirEncoding), (child) => { const childPath = Buffer.concat([pathBuf, separator, child]); rimrafSync(childPath, options); diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js index 5944a3980088eb..9dfc7c73251e56 100644 --- a/lib/internal/fs/streams.js +++ b/lib/internal/fs/streams.js @@ -5,6 +5,7 @@ const { MathMin, ObjectDefineProperty, ObjectSetPrototypeOf, + ReflectApply, Symbol, } = primordials; @@ -42,12 +43,12 @@ function _construct(callback) { if (args[0] === 'open') { this.emit = orgEmit; callback(); - orgEmit.apply(this, args); + ReflectApply(orgEmit, this, args); } else if (args[0] === 'error') { this.emit = orgEmit; callback(args[1]); } else { - orgEmit.apply(this, args); + ReflectApply(orgEmit, this, args); } }; stream.open(); @@ -145,7 +146,7 @@ function ReadStream(path, options) { } } - Readable.call(this, options); + ReflectApply(Readable, this, [options]); } ObjectSetPrototypeOf(ReadStream.prototype, Readable.prototype); ObjectSetPrototypeOf(ReadStream, Readable); @@ -302,7 +303,7 @@ function WriteStream(path, options) { this.pos = this.start; } - Writable.call(this, options); + ReflectApply(Writable, this, [options]); if (options.encoding) this.setDefaultEncoding(options.encoding); diff --git a/lib/internal/fs/sync_write_stream.js b/lib/internal/fs/sync_write_stream.js index 7d1209ba2decb4..df366edce4716b 100644 --- a/lib/internal/fs/sync_write_stream.js +++ b/lib/internal/fs/sync_write_stream.js @@ -2,13 +2,14 @@ const { ObjectSetPrototypeOf, + ReflectApply, } = primordials; const { Writable } = require('stream'); const { closeSync, writeSync } = require('fs'); function SyncWriteStream(fd, options) { - Writable.call(this, { autoDestroy: true }); + ReflectApply(Writable, this, [{ autoDestroy: true }]); options = options || {}; diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index dcef28c556da46..d5f3b4c78f16a2 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -5,15 +5,22 @@ const { BigInt, Date, DateNow, + DatePrototypeGetTime, ErrorCaptureStackTrace, - ObjectPrototypeHasOwnProperty, + FunctionPrototypeCall, Number, NumberIsFinite, NumberIsInteger, MathMin, + ObjectPrototypeHasOwnProperty, ObjectSetPrototypeOf, + ReflectApply, ReflectOwnKeys, + StringPrototypeEndsWith, + StringPrototypeIncludes, + StringPrototypeReplace, Symbol, + TypedArrayPrototypeIncludes, } = primordials; const { Buffer } = require('buffer'); @@ -329,8 +336,8 @@ const nullCheck = hideStackFrames((path, propName, throwError = true) => { // We can only perform meaningful checks on strings and Uint8Arrays. if ((!pathIsString && !pathIsUint8Array) || - (pathIsString && !path.includes('\u0000')) || - (pathIsUint8Array && !path.includes(0))) { + (pathIsString && !StringPrototypeIncludes(path, '\u0000')) || + (pathIsUint8Array && !TypedArrayPrototypeIncludes(path, 0))) { return; } @@ -362,7 +369,7 @@ function preprocessSymlinkDestination(path, type, linkPath) { return pathModule.toNamespacedPath(path); } // Windows symlinks don't tolerate forward slashes. - return path.replace(/\//g, '\\'); + return StringPrototypeReplace(path, /\//g, '\\'); } // Constructor for file stats. @@ -433,8 +440,8 @@ function dateFromMs(ms) { function BigIntStats(dev, mode, nlink, uid, gid, rdev, blksize, ino, size, blocks, atimeNs, mtimeNs, ctimeNs, birthtimeNs) { - StatsBase.call(this, dev, mode, nlink, uid, gid, rdev, blksize, - ino, size, blocks); + ReflectApply(StatsBase, this, [dev, mode, nlink, uid, gid, rdev, blksize, + ino, size, blocks]); this.atimeMs = atimeNs / kNsPerMsBigInt; this.mtimeMs = mtimeNs / kNsPerMsBigInt; @@ -464,8 +471,8 @@ BigIntStats.prototype._checkModeProperty = function(property) { function Stats(dev, mode, nlink, uid, gid, rdev, blksize, ino, size, blocks, atimeMs, mtimeMs, ctimeMs, birthtimeMs) { - StatsBase.call(this, dev, mode, nlink, uid, gid, rdev, blksize, - ino, size, blocks); + FunctionPrototypeCall(StatsBase, this, dev, mode, nlink, uid, gid, rdev, + blksize, ino, size, blocks); this.atimeMs = atimeMs; this.mtimeMs = mtimeMs; this.ctimeMs = ctimeMs; @@ -590,7 +597,7 @@ function toUnixTimestamp(time, name = 'time') { } if (isDate(time)) { // Convert to 123.456 UNIX timestamp - return time.getTime() / 1000; + return DatePrototypeGetTime(time) / 1000; } throw new ERR_INVALID_ARG_TYPE(name, ['Date', 'Time in seconds'], time); } @@ -657,7 +664,7 @@ let nonPortableTemplateWarn = true; function warnOnNonPortableTemplate(template) { // Template strings passed to the mkdtemp() family of functions should not // end with 'X' because they are handled inconsistently across platforms. - if (nonPortableTemplateWarn && template.endsWith('X')) { + if (nonPortableTemplateWarn && StringPrototypeEndsWith(template, 'X')) { process.emitWarning('mkdtemp() templates ending with X are not portable. ' + 'For details see: https://nodejs.org/api/fs.html'); nonPortableTemplateWarn = false; diff --git a/lib/internal/fs/watchers.js b/lib/internal/fs/watchers.js index 01db66c551c9d3..43c6a8f9a938fa 100644 --- a/lib/internal/fs/watchers.js +++ b/lib/internal/fs/watchers.js @@ -1,6 +1,7 @@ 'use strict'; const { + FunctionPrototypeCall, ObjectDefineProperty, ObjectSetPrototypeOf, Symbol, @@ -40,7 +41,7 @@ function emitStop(self) { } function StatWatcher(bigint) { - EventEmitter.call(this); + FunctionPrototypeCall(EventEmitter, this); this._handle = null; this[kOldStatus] = -1; @@ -159,7 +160,7 @@ StatWatcher.prototype.unref = function() { function FSWatcher() { - EventEmitter.call(this); + FunctionPrototypeCall(EventEmitter, this); this._handle = new FSEvent(); this._handle[owner_symbol] = this;