Wrap fs.statSync and fs.lstatSync (#783)

Adds two new methods to src/common.js, common.statFollowLinks and common.statNoFollowLinks, which wrap fs.statSync and fs.lstatSync, respectively. This change is meant to improve readability and clarify intent. * Add common.statFollowLinks and common.statNoFollowLinks * Replace fs.statSync and fs.lstatSync in source files
shelljs · Oct 18, 2017 · a7d6df5 · a7d6df5
1 parent 7cbce88
commit a7d6df5
Show file tree

Hide file tree

Showing 28 changed files with 147 additions and 128 deletions.
diff --git a/src/cat.js b/src/cat.js
@@ -36,7 +36,7 @@ function _cat(options, files) {
   files.forEach(function (file) {
     if (!fs.existsSync(file)) {
       common.error('no such file or directory: ' + file);
-    } else if (fs.statSync(file).isDirectory()) {
+    } else if (common.statFollowLinks(file).isDirectory()) {
       common.error(file + ': Is a directory');
     }
 

diff --git a/src/cd.js b/src/cd.js
@@ -1,4 +1,3 @@
-var fs = require('fs');
 var os = require('os');
 var common = require('./common');
 
@@ -27,7 +26,7 @@ function _cd(options, dir) {
     // something went wrong, let's figure out the error
     var err;
     try {
-      fs.statSync(dir); // if this succeeds, it must be some sort of file
+      common.statFollowLinks(dir); // if this succeeds, it must be some sort of file
       err = 'not a directory: ' + dir;
     } catch (e2) {
       err = 'no such file or directory: ' + dir;

diff --git a/src/chmod.js b/src/chmod.js
@@ -85,7 +85,7 @@ function _chmod(options, mode, filePattern) {
   if (options.recursive) {
     files = [];
     filePattern.forEach(function addFile(expandedFile) {
-      var stat = fs.lstatSync(expandedFile);
+      var stat = common.statNoFollowLinks(expandedFile);
 
       if (!stat.isSymbolicLink()) {
         files.push(expandedFile);
@@ -108,11 +108,11 @@ function _chmod(options, mode, filePattern) {
     }
 
     // When recursing, don't follow symlinks.
-    if (options.recursive && fs.lstatSync(file).isSymbolicLink()) {
+    if (options.recursive && common.statNoFollowLinks(file).isSymbolicLink()) {
       return;
     }
 
-    var stat = fs.statSync(file);
+    var stat = common.statFollowLinks(file);
     var isDir = stat.isDirectory();
     var perms = stat.mode;
     var type = perms & PERMS.TYPE_MASK;
@@ -175,7 +175,7 @@ function _chmod(options, mode, filePattern) {
 
               // According to POSIX, when using = to explicitly set the
               // permissions, setuid and setgid can never be cleared.
-              if (fs.statSync(file).isDirectory()) {
+              if (common.statFollowLinks(file).isDirectory()) {
                 newPerms |= (PERMS.SETUID + PERMS.SETGID) & perms;
               }
               break;
@@ -204,7 +204,7 @@ function _chmod(options, mode, filePattern) {
 
       // POSIX rules are that setuid and setgid can only be added using numeric
       // form, but not cleared.
-      if (fs.statSync(file).isDirectory()) {
+      if (common.statFollowLinks(file).isDirectory()) {
         newPerms |= (PERMS.SETUID + PERMS.SETGID) & perms;
       }
 

diff --git a/src/common.js b/src/common.js
@@ -277,6 +277,18 @@ function unlinkSync(file) {
 }
 exports.unlinkSync = unlinkSync;
 
+// wrappers around common.statFollowLinks and common.statNoFollowLinks that clarify intent
+// and improve readability
+function statFollowLinks() {
+  return fs.statSync.apply(fs, arguments);
+}
+exports.statFollowLinks = statFollowLinks;
+
+function statNoFollowLinks() {
+  return fs.lstatSync.apply(fs, arguments);
+}
+exports.statNoFollowLinks = statNoFollowLinks;
+
 // e.g. 'shelljs_a5f185d0443ca...'
 function randomFileName() {
   function randomHash(count) {

diff --git a/src/cp.js b/src/cp.js
@@ -27,16 +27,16 @@ function copyFileSync(srcFile, destFile, options) {
 
   // Check the mtimes of the files if the '-u' flag is provided
   try {
-    if (options.update && fs.statSync(srcFile).mtime < fs.statSync(destFile).mtime) {
+    if (options.update && common.statFollowLinks(srcFile).mtime < fs.statSync(destFile).mtime) {
       return;
     }
   } catch (e) {
     // If we're here, destFile probably doesn't exist, so just do a normal copy
   }
 
-  if (fs.lstatSync(srcFile).isSymbolicLink() && !options.followsymlink) {
+  if (common.statNoFollowLinks(srcFile).isSymbolicLink() && !options.followsymlink) {
     try {
-      fs.lstatSync(destFile);
+      common.statNoFollowLinks(destFile);
       common.unlinkSync(destFile); // re-link it
     } catch (e) {
       // it doesn't exist, so no work needs to be done
@@ -75,7 +75,7 @@ function copyFileSync(srcFile, destFile, options) {
     fs.closeSync(fdr);
     fs.closeSync(fdw);
 
-    fs.chmodSync(destFile, fs.statSync(srcFile).mode);
+    fs.chmodSync(destFile, common.statFollowLinks(srcFile).mode);
   }
 }
 
@@ -99,7 +99,7 @@ function cpdirSyncRecursive(sourceDir, destDir, currentDepth, opts) {
   // Create the directory where all our junk is moving to; read the mode of the
   // source directory and mirror it
   try {
-    var checkDir = fs.statSync(sourceDir);
+    var checkDir = common.statFollowLinks(sourceDir);
     fs.mkdirSync(destDir, checkDir.mode);
   } catch (e) {
     // if the directory already exists, that's okay
@@ -111,7 +111,7 @@ function cpdirSyncRecursive(sourceDir, destDir, currentDepth, opts) {
   for (var i = 0; i < files.length; i++) {
     var srcFile = sourceDir + '/' + files[i];
     var destFile = destDir + '/' + files[i];
-    var srcFileStat = fs.lstatSync(srcFile);
+    var srcFileStat = common.statNoFollowLinks(srcFile);
 
     var symlinkFull;
     if (opts.followsymlink) {
@@ -129,14 +129,14 @@ function cpdirSyncRecursive(sourceDir, destDir, currentDepth, opts) {
     } else if (srcFileStat.isSymbolicLink() && !opts.followsymlink) {
       symlinkFull = fs.readlinkSync(srcFile);
       try {
-        fs.lstatSync(destFile);
+        common.statNoFollowLinks(destFile);
         common.unlinkSync(destFile); // re-link it
       } catch (e) {
         // it doesn't exist, so no work needs to be done
       }
       fs.symlinkSync(symlinkFull, destFile, isWindows ? 'junction' : null);
     } else if (srcFileStat.isSymbolicLink() && opts.followsymlink) {
-      srcFileStat = fs.statSync(srcFile);
+      srcFileStat = common.statFollowLinks(srcFile);
       if (srcFileStat.isDirectory()) {
         cpdirSyncRecursive(srcFile, destFile, currentDepth, opts);
       } else {
@@ -162,15 +162,15 @@ function checkRecentCreated(sources, index) {
 }
 
 function cpcheckcycle(sourceDir, srcFile) {
-  var srcFileStat = fs.lstatSync(srcFile);
+  var srcFileStat = common.statNoFollowLinks(srcFile);
   if (srcFileStat.isSymbolicLink()) {
     // Do cycle check. For example:
     //   $ mkdir -p 1/2/3/4
     //   $ cd  1/2/3/4
     //   $ ln -s ../../3 link
     //   $ cd ../../../..
     //   $ cp -RL 1 copy
-    var cyclecheck = fs.statSync(srcFile);
+    var cyclecheck = common.statFollowLinks(srcFile);
     if (cyclecheck.isDirectory()) {
       var sourcerealpath = fs.realpathSync(sourceDir);
       var symlinkrealpath = fs.realpathSync(srcFile);
@@ -223,7 +223,7 @@ function _cp(options, sources, dest) {
   }
 
   var destExists = fs.existsSync(dest);
-  var destStat = destExists && fs.statSync(dest);
+  var destStat = destExists && common.statFollowLinks(dest);
 
   // Dest is not existing dir, but multiple sources given
   if ((!destExists || !destStat.isDirectory()) && sources.length > 1) {
@@ -241,7 +241,7 @@ function _cp(options, sources, dest) {
       common.error('no such file or directory: ' + src, { continue: true });
       return; // skip file
     }
-    var srcStat = fs.statSync(src);
+    var srcStat = common.statFollowLinks(src);
     if (!options.noFollowsymlink && srcStat.isDirectory()) {
       if (!options.recursive) {
         // Non-Recursive
@@ -254,7 +254,7 @@ function _cp(options, sources, dest) {
             dest;
 
         try {
-          fs.statSync(path.dirname(dest));
+          common.statFollowLinks(path.dirname(dest));
           cpdirSyncRecursive(src, newDest, 0, { no_force: options.no_force, followsymlink: options.followsymlink });
         } catch (e) {
           /* istanbul ignore next */

diff --git a/src/find.js b/src/find.js
@@ -1,4 +1,3 @@
-var fs = require('fs');
 var path = require('path');
 var common = require('./common');
 var _ls = require('./ls');
@@ -42,7 +41,7 @@ function _find(options, paths) {
   paths.forEach(function (file) {
     var stat;
     try {
-      stat = fs.statSync(file);
+      stat = common.statFollowLinks(file);
     } catch (e) {
       common.error('no such file or directory: ' + file);
     }

diff --git a/src/head.js b/src/head.js
@@ -71,7 +71,7 @@ function _head(options, files) {
       if (!fs.existsSync(file)) {
         common.error('no such file or directory: ' + file, { continue: true });
         return;
-      } else if (fs.statSync(file).isDirectory()) {
+      } else if (common.statFollowLinks(file).isDirectory()) {
         common.error("error reading '" + file + "': Is a directory", {
           continue: true,
         });

diff --git a/src/ln.js b/src/ln.js
@@ -48,7 +48,7 @@ function _ln(options, source, dest) {
     var resolvedSourcePath = isAbsolute ? sourcePath : path.resolve(process.cwd(), path.dirname(dest), source);
     if (!fs.existsSync(resolvedSourcePath)) {
       common.error('Source file does not exist', { continue: true });
-    } else if (isWindows && fs.statSync(resolvedSourcePath).isDirectory()) {
+    } else if (isWindows && common.statFollowLinks(resolvedSourcePath).isDirectory()) {
       linkType = 'junction';
     }
 

diff --git a/src/ls.js b/src/ls.js
@@ -62,7 +62,7 @@ function _ls(options, paths) {
       relName = relName.replace(/\\/g, '/');
     }
     if (options.long) {
-      stat = stat || (options.link ? fs.statSync(abs) : fs.lstatSync(abs));
+      stat = stat || (options.link ? common.statFollowLinks(abs) : common.statNoFollowLinks(abs));
       list.push(addLsAttributes(relName, stat));
     } else {
       // list.push(path.relative(rel || '.', file));
@@ -74,11 +74,11 @@ function _ls(options, paths) {
     var stat;
 
     try {
-      stat = options.link ? fs.statSync(p) : fs.lstatSync(p);
+      stat = options.link ? common.statFollowLinks(p) : common.statNoFollowLinks(p);
       // follow links to directories by default
       if (stat.isSymbolicLink()) {
         try {
-          var _stat = fs.statSync(p);
+          var _stat = common.statFollowLinks(p);
           if (_stat.isDirectory()) {
             stat = _stat;
           }

diff --git a/src/mkdir.js b/src/mkdir.js
@@ -57,7 +57,7 @@ function _mkdir(options, dirs) {
 
   dirs.forEach(function (dir) {
     try {
-      var stat = fs.lstatSync(dir);
+      var stat = common.statNoFollowLinks(dir);
       if (!options.fullpath) {
         common.error('path already exists: ' + dir, { continue: true });
       } else if (stat.isFile()) {

diff --git a/src/mv.js b/src/mv.js
@@ -51,7 +51,7 @@ function _mv(options, sources, dest) {
   }
 
   var exists = fs.existsSync(dest);
-  var stats = exists && fs.statSync(dest);
+  var stats = exists && common.statFollowLinks(dest);
 
   // Dest is not existing dir, but multiple sources given
   if ((!exists || !stats.isDirectory()) && sources.length > 1) {
@@ -74,7 +74,7 @@ function _mv(options, sources, dest) {
     // When copying to '/path/dir':
     //    thisDest = '/path/dir/file1'
     var thisDest = dest;
-    if (fs.existsSync(dest) && fs.statSync(dest).isDirectory()) {
+    if (fs.existsSync(dest) && common.statFollowLinks(dest).isDirectory()) {
       thisDest = path.normalize(dest + '/' + path.basename(src));
     }
 

diff --git a/src/rm.js b/src/rm.js
@@ -25,7 +25,7 @@ function rmdirSyncRecursive(dir, force, fromSymlink) {
   // Loop through and delete everything in the sub-tree after checking it
   for (var i = 0; i < files.length; i++) {
     var file = dir + '/' + files[i];
-    var currFile = fs.lstatSync(file);
+    var currFile = common.statNoFollowLinks(file);
 
     if (currFile.isDirectory()) { // Recursive function back to the beginning
       rmdirSyncRecursive(file, force);
@@ -116,7 +116,7 @@ function handleDirectory(file, options) {
 function handleSymbolicLink(file, options) {
   var stats;
   try {
-    stats = fs.statSync(file);
+    stats = common.statFollowLinks(file);
   } catch (e) {
     // symlink is broken, so remove the symlink itself
     common.unlinkSync(file);
@@ -175,7 +175,7 @@ function _rm(options, files) {
       var filepath = (file[file.length - 1] === '/')
         ? file.slice(0, -1) // remove the '/' so lstatSync can detect symlinks
         : file;
-      lstats = fs.lstatSync(filepath); // test for existence
+      lstats = common.statNoFollowLinks(filepath); // test for existence
     } catch (e) {
       // Path does not exist, no force flag given
       if (!options.force) {

diff --git a/src/sort.js b/src/sort.js
@@ -72,7 +72,7 @@ function _sort(options, files) {
       if (!fs.existsSync(file)) {
         common.error('no such file or directory: ' + file, { continue: true });
         return accum;
-      } else if (fs.statSync(file).isDirectory()) {
+      } else if (common.statFollowLinks(file).isDirectory()) {
         common.error('read failed: ' + file + ': Is a directory', {
           continue: true,
         });

diff --git a/src/tail.js b/src/tail.js
@@ -50,7 +50,7 @@ function _tail(options, files) {
       if (!fs.existsSync(file)) {
         common.error('no such file or directory: ' + file, { continue: true });
         return;
-      } else if (fs.statSync(file).isDirectory()) {
+      } else if (common.statFollowLinks(file).isDirectory()) {
         common.error("error reading '" + file + "': Is a directory", {
           continue: true,
         });

diff --git a/src/tempdir.js b/src/tempdir.js
@@ -11,7 +11,7 @@ common.register('tempdir', _tempDir, {
 function writeableDir(dir) {
   if (!dir || !fs.existsSync(dir)) return false;
 
-  if (!fs.statSync(dir).isDirectory()) return false;
+  if (!common.statFollowLinks(dir).isDirectory()) return false;
 
   var testFile = dir + '/' + common.randomFileName();
   try {

diff --git a/src/test.js b/src/test.js
@@ -52,7 +52,7 @@ function _test(options, path) {
 
   if (options.link) {
     try {
-      return fs.lstatSync(path).isSymbolicLink();
+      return common.statNoFollowLinks(path).isSymbolicLink();
     } catch (e) {
       return false;
     }
@@ -62,7 +62,7 @@ function _test(options, path) {
 
   if (options.exists) return true;
 
-  var stats = fs.statSync(path);
+  var stats = common.statFollowLinks(path);
 
   if (options.block) return stats.isBlockDevice();
 

diff --git a/src/touch.js b/src/touch.js
@@ -103,7 +103,7 @@ module.exports = _touch;
 
 function tryStatFile(filePath) {
   try {
-    return fs.statSync(filePath);
+    return common.statFollowLinks(filePath);
   } catch (e) {
     return null;
   }

diff --git a/src/uniq.js b/src/uniq.js
@@ -45,11 +45,11 @@ function _uniq(options, input, output) {
 
     if (!fs.existsSync(input)) {
       common.error(input + ': No such file or directory');
-    } else if (fs.statSync(input).isDirectory()) {
+    } else if (common.statFollowLinks(input).isDirectory()) {
       common.error("error reading '" + input + "'");
     }
   }
-  if (output && fs.existsSync(output) && fs.statSync(output).isDirectory()) {
+  if (output && fs.existsSync(output) && common.statFollowLinks(output).isDirectory()) {
     common.error(output + ': Is a directory');
   }
 

diff --git a/src/which.js b/src/which.js
@@ -19,7 +19,7 @@ function splitPath(p) {
 }
 
 function checkPath(pathName) {
-  return fs.existsSync(pathName) && !fs.statSync(pathName).isDirectory();
+  return fs.existsSync(pathName) && !common.statFollowLinks(pathName).isDirectory();
 }
 
 //@