fs: consistent constants use and cleanup

* only require('buffer') once
* use constants directly
* fix incorrect constants

PR-URL: #20765
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

lib/fs.js

@@ -24,16 +24,31 @@
 
 'use strict';
 
-const constants = process.binding('constants').fs;
-const { S_IFIFO, S_IFLNK, S_IFMT, S_IFREG, S_IFSOCK } = constants;
+const { fs: constants } = process.binding('constants');
+const {
+  S_IFIFO,
+  S_IFLNK,
+  S_IFMT,
+  S_IFREG,
+  S_IFSOCK,
+  F_OK,
+  R_OK,
+  W_OK,
+  X_OK,
+  O_WRONLY,
+  O_SYMLINK,
+  UV_FS_COPYFILE_EXCL,
+  UV_FS_COPYFILE_FICLONE,
+  UV_FS_COPYFILE_FICLONE_FORCE
+} = constants;
 const util = require('util');
 const pathModule = require('path');
 const { isUint8Array } = require('internal/util/types');
 const { createPromise, promiseResolve } = process.binding('util');
 
 const binding = process.binding('fs');
 const fs = exports;
-const { Buffer } = require('buffer');
+const { Buffer, kMaxLength } = require('buffer');
 const errors = require('internal/errors');
 const {
   ERR_FS_FILE_TOO_LARGE,
@@ -58,6 +73,7 @@ const {
   preprocessSymlinkDestination,
   Stats,
   getStatsFromBinding,
+  realpathCacheKey,
   stringToFlags,
   stringToSymlinkType,
   toUnixTimestamp,
@@ -103,7 +119,6 @@ function lazyAssert() {
 }
 
 const kMinPoolSpace = 128;
-const { kMaxLength } = require('buffer');
 
 const isWindows = process.platform === 'win32';
 
@@ -179,16 +194,16 @@ function isFileType(stats, fileType) {
 
 // Don't allow mode to accidentally be overwritten.
 Object.defineProperties(fs, {
-  F_OK: { enumerable: true, value: constants.F_OK || 0 },
-  R_OK: { enumerable: true, value: constants.R_OK || 0 },
-  W_OK: { enumerable: true, value: constants.W_OK || 0 },
-  X_OK: { enumerable: true, value: constants.X_OK || 0 },
+  F_OK: { enumerable: true, value: F_OK || 0 },
+  R_OK: { enumerable: true, value: R_OK || 0 },
+  W_OK: { enumerable: true, value: W_OK || 0 },
+  X_OK: { enumerable: true, value: X_OK || 0 },
 });
 
 fs.access = function(path, mode, callback) {
   if (typeof mode === 'function') {
     callback = mode;
-    mode = fs.F_OK;
+    mode = F_OK;
   }
 
   path = getPathFromURL(path);
@@ -205,7 +220,7 @@ fs.accessSync = function(path, mode) {
   validatePath(path);
 
   if (mode === undefined)
-    mode = fs.F_OK;
+    mode = F_OK;
   else
     mode = mode | 0;
 
@@ -222,7 +237,7 @@ fs.exists = function(path, callback) {
   }
 
   try {
-    fs.access(path, fs.FS_OK, suppressedCallback);
+    fs.access(path, F_OK, suppressedCallback);
   } catch (err) {
     return callback(false);
   }
@@ -244,7 +259,7 @@ Object.defineProperty(fs.exists, internalUtil.promisify.custom, {
 // TODO(joyeecheung): deprecate the never-throw-on-invalid-arguments behavior
 fs.existsSync = function(path) {
   try {
-    fs.accessSync(path, fs.FS_OK);
+    fs.accessSync(path, F_OK);
     return true;
   } catch (e) {
     return false;
@@ -1054,10 +1069,10 @@ fs.fchmodSync = function(fd, mode) {
   handleErrorFromBinding(ctx);
 };
 
-if (constants.O_SYMLINK !== undefined) {
+if (O_SYMLINK !== undefined) {
   fs.lchmod = function(path, mode, callback) {
     callback = maybeCallback(callback);
-    fs.open(path, constants.O_WRONLY | constants.O_SYMLINK, function(err, fd) {
+    fs.open(path, O_WRONLY | O_SYMLINK, function(err, fd) {
       if (err) {
         callback(err);
         return;
@@ -1073,7 +1088,7 @@ if (constants.O_SYMLINK !== undefined) {
   };
 
   fs.lchmodSync = function(path, mode) {
-    const fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
+    const fd = fs.openSync(path, O_WRONLY | O_SYMLINK);
 
     // Prefer to return the chmod error, if one occurs,
     // but still try to close, and report closing errors if they occur.
@@ -1110,10 +1125,10 @@ fs.chmodSync = function(path, mode) {
   handleErrorFromBinding(ctx);
 };
 
-if (constants.O_SYMLINK !== undefined) {
+if (O_SYMLINK !== undefined) {
   fs.lchown = function(path, uid, gid, callback) {
     callback = maybeCallback(callback);
-    fs.open(path, constants.O_WRONLY | constants.O_SYMLINK, function(err, fd) {
+    fs.open(path, O_WRONLY | O_SYMLINK, function(err, fd) {
       if (err) {
         callback(err);
         return;
@@ -1129,7 +1144,7 @@ if (constants.O_SYMLINK !== undefined) {
   };
 
   fs.lchownSync = function(path, uid, gid) {
-    const fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
+    const fd = fs.openSync(path, O_WRONLY | O_SYMLINK);
     let ret;
     try {
       ret = fs.fchownSync(fd, uid, gid);
@@ -1631,7 +1646,7 @@ fs.realpathSync = function realpathSync(p, options) {
   validatePath(p);
   p = pathModule.resolve(p);
 
-  const cache = options[internalFS.realpathCacheKey];
+  const cache = options[realpathCacheKey];
   const maybeCachedResult = cache && cache.get(p);
   if (maybeCachedResult) {
     return maybeCachedResult;
@@ -1936,14 +1951,14 @@ fs.mkdtempSync = function(prefix, options) {
 
 // Define copyFile() flags.
 Object.defineProperties(fs.constants, {
-  COPYFILE_EXCL: { enumerable: true, value: constants.UV_FS_COPYFILE_EXCL },
+  COPYFILE_EXCL: { enumerable: true, value: UV_FS_COPYFILE_EXCL },
   COPYFILE_FICLONE: {
     enumerable: true,
-    value: constants.UV_FS_COPYFILE_FICLONE
+    value: UV_FS_COPYFILE_FICLONE
   },
   COPYFILE_FICLONE_FORCE: {
     enumerable: true,
-    value: constants.UV_FS_COPYFILE_FICLONE_FORCE
+    value: UV_FS_COPYFILE_FICLONE_FORCE
   }
 });