fs: minor refactoring

lib/fs.js

@@ -35,6 +35,10 @@ const isWindows = process.platform === 'win32';
 const DEBUG = process.env.NODE_DEBUG && /fs/.test(process.env.NODE_DEBUG);
 const errnoException = util._errnoException;
 
+function throwOptionsError(options) {
+  throw new TypeError('Expected options to be either an object or a string, ' +
+    'but got ' + typeof options + ' instead');
+}
 
 function rethrow() {
   // Only enable in debug mode. A backtrace uses ~1000 bytes of heap space and
@@ -45,8 +49,7 @@ function rethrow() {
       if (err) {
         backtrace.stack = err.name + ': ' + err.message +
                           backtrace.stack.substr(backtrace.name.length);
-        err = backtrace;
-        throw err;
+        throw backtrace;
       }
     };
   }
@@ -91,16 +94,12 @@ function nullCheck(path, callback) {
     er.code = 'ENOENT';
     if (typeof callback !== 'function')
       throw er;
-    process.nextTick(nullCheckCallNT, callback, er);
+    process.nextTick(callback, er);
     return false;
   }
   return true;
 }
 
-function nullCheckCallNT(callback, er) {
-  callback(er);
-}
-
 // Static method to set the stats properties on a Stats object.
 fs.Stats = function(
     dev,
@@ -226,12 +225,13 @@ fs.existsSync = function(path) {
 fs.readFile = function(path, options, callback_) {
   var callback = maybeCallback(arguments[arguments.length - 1]);
 
-  if (!options || typeof options === 'function')
+  if (!options || typeof options === 'function') {
     options = { encoding: null, flag: 'r' };
-  else if (typeof options === 'string')
+  } else if (typeof options === 'string') {
     options = { encoding: options, flag: 'r' };
-  else if (typeof options !== 'object')
-    throw new TypeError('Bad arguments');
+  } else if (typeof options !== 'object') {
+    throwOptionsError(options);
+  }
 
   var encoding = options.encoding;
   assertEncoding(encoding);
@@ -266,27 +266,25 @@ function ReadFileContext(callback, encoding) {
 }
 
 ReadFileContext.prototype.read = function() {
-  var fd = this.fd;
-  var size = this.size;
   var buffer;
   var offset;
   var length;
 
-  if (size === 0) {
+  if (this.size === 0) {
     buffer = this.buffer = new SlowBuffer(kReadFileBufferLength);
     offset = 0;
     length = kReadFileBufferLength;
   } else {
     buffer = this.buffer;
     offset = this.pos;
-    length = size - this.pos;
+    length = this.size - this.pos;
   }
 
   var req = new FSReqWrap();
   req.oncomplete = readFileAfterRead;
   req.context = this;
 
-  binding.read(fd, buffer, offset, length, -1, req);
+  binding.read(this.fd, buffer, offset, length, -1, req);
 };
 
 ReadFileContext.prototype.close = function(err) {
@@ -301,8 +299,7 @@ function readFileAfterOpen(err, fd) {
   var context = this.context;
 
   if (err) {
-    var callback = context.callback;
-    callback(err);
+    context.callback(err);
     return;
   }
 
@@ -389,7 +386,7 @@ fs.readFileSync = function(path, options) {
   } else if (typeof options === 'string') {
     options = { encoding: options, flag: 'r' };
   } else if (typeof options !== 'object') {
-    throw new TypeError('Bad arguments');
+    throwOptionsError(options);
   }
 
   var encoding = options.encoding;
@@ -416,7 +413,7 @@ fs.readFileSync = function(path, options) {
   if (size === 0) {
     buffers = [];
   } else {
-    var threw = true;
+    threw = true;
     try {
       buffer = new Buffer(size);
       threw = false;
@@ -426,16 +423,18 @@ fs.readFileSync = function(path, options) {
   }
 
   var done = false;
+  var bytesRead;
+
   while (!done) {
-    var threw = true;
+    threw = true;
     try {
       if (size !== 0) {
-        var bytesRead = fs.readSync(fd, buffer, pos, size - pos);
+        bytesRead = fs.readSync(fd, buffer, pos, size - pos);
       } else {
         // the kernel lies about many files.
         // Go ahead and try to read some bytes.
         buffer = new Buffer(8192);
-        var bytesRead = fs.readSync(fd, buffer, 0, 8192);
+        bytesRead = fs.readSync(fd, buffer, 0, 8192);
         if (bytesRead) {
           buffers.push(buffer.slice(0, bytesRead));
         }
@@ -528,8 +527,8 @@ function modeNum(m, def) {
   return undefined;
 }
 
-fs.open = function(path, flags, mode, callback) {
-  callback = makeCallback(arguments[arguments.length - 1]);
+fs.open = function(path, flags, mode, callback_) {
+  var callback = makeCallback(arguments[arguments.length - 1]);
   mode = modeNum(mode, 0o666);
 
   if (!nullCheck(path, callback)) return;
@@ -584,10 +583,12 @@ fs.read = function(fd, buffer, offset, length, position, callback) {
 
 fs.readSync = function(fd, buffer, offset, length, position) {
   var legacy = false;
+  var encoding;
+
   if (!(buffer instanceof Buffer)) {
     // legacy string interface (fd, length, position, encoding, callback)
     legacy = true;
-    var encoding = arguments[3];
+    encoding = arguments[3];
 
     assertEncoding(encoding);
 
@@ -622,14 +623,14 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
     callback(err, written || 0, buffer);
   }
 
+  var req = new FSReqWrap();
   if (buffer instanceof Buffer) {
     // if no position is passed then assume null
     if (typeof position === 'function') {
       callback = position;
       position = null;
     }
     callback = maybeCallback(callback);
-    var req = new FSReqWrap();
     req.oncomplete = strWrapper;
     return binding.writeBuffer(fd, buffer, offset, length, position, req);
   }
@@ -646,7 +647,6 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
     length = 'utf8';
   }
   callback = maybeCallback(position);
-  var req = new FSReqWrap();
   req.oncomplete = bufWrapper;
   return binding.writeString(fd, buffer, offset, length, req);
 };
@@ -720,8 +720,10 @@ fs.truncateSync = function(path, len) {
   }
   // allow error to be thrown, but still close fd.
   var fd = fs.openSync(path, 'r+');
+  var ret;
+
   try {
-    var ret = fs.ftruncateSync(fd, len);
+    ret = fs.ftruncateSync(fd, len);
   } finally {
     fs.closeSync(fd);
   }
@@ -874,7 +876,7 @@ function preprocessSymlinkDestination(path, type, linkPath) {
   }
 }
 
-fs.symlink = function(destination, path, type_, callback) {
+fs.symlink = function(destination, path, type_, callback_) {
   var type = (typeof type_ === 'string' ? type_ : null);
   var callback = makeCallback(arguments[arguments.length - 1]);
 
@@ -967,9 +969,9 @@ if (constants.hasOwnProperty('O_SYMLINK')) {
 
     // prefer to return the chmod error, if one occurs,
     // but still try to close, and report closing errors if they occur.
-    var err, err2;
+    var err, err2, ret;
     try {
-      var ret = fs.fchmodSync(fd, mode);
+      ret = fs.fchmodSync(fd, mode);
     } catch (er) {
       err = er;
     }
@@ -1087,8 +1089,8 @@ fs.futimesSync = function(fd, atime, mtime) {
   binding.futimes(fd, atime, mtime);
 };
 
-function writeAll(fd, buffer, offset, length, position, callback) {
-  callback = maybeCallback(arguments[arguments.length - 1]);
+function writeAll(fd, buffer, offset, length, position, callback_) {
+  var callback = maybeCallback(arguments[arguments.length - 1]);
 
   // write(fd, buffer, offset, length, position, callback)
   fs.write(fd, buffer, offset, length, position, function(writeErr, written) {
@@ -1111,15 +1113,15 @@ function writeAll(fd, buffer, offset, length, position, callback) {
   });
 }
 
-fs.writeFile = function(path, data, options, callback) {
+fs.writeFile = function(path, data, options, callback_) {
   var callback = maybeCallback(arguments[arguments.length - 1]);
 
   if (!options || typeof options === 'function') {
     options = { encoding: 'utf8', mode: 0o666, flag: 'w' };
   } else if (typeof options === 'string') {
     options = { encoding: options, mode: 0o666, flag: 'w' };
   } else if (typeof options !== 'object') {
-    throw new TypeError('Bad arguments');
+    throwOptionsError(options);
   }
 
   assertEncoding(options.encoding);
@@ -1143,7 +1145,7 @@ fs.writeFileSync = function(path, data, options) {
   } else if (typeof options === 'string') {
     options = { encoding: options, mode: 0o666, flag: 'w' };
   } else if (typeof options !== 'object') {
-    throw new TypeError('Bad arguments');
+    throwOptionsError(options);
   }
 
   assertEncoding(options.encoding);
@@ -1178,7 +1180,7 @@ fs.appendFile = function(path, data, options, callback_) {
   } else if (typeof options === 'string') {
     options = { encoding: options, mode: 0o666, flag: 'a' };
   } else if (typeof options !== 'object') {
-    throw new TypeError('Bad arguments');
+    throwOptionsError(options);
   }
 
   if (!options.flag)
@@ -1192,7 +1194,7 @@ fs.appendFileSync = function(path, data, options) {
   } else if (typeof options === 'string') {
     options = { encoding: options, mode: 0o666, flag: 'a' };
   } else if (typeof options !== 'object') {
-    throw new TypeError('Bad arguments');
+    throwOptionsError(options);
   }
   if (!options.flag)
     options = util._extend({ flag: 'a' }, options);
@@ -1300,12 +1302,7 @@ StatWatcher.prototype.stop = function() {
 };
 
 
-var statWatchers = {};
-function inStatWatchers(filename) {
-  return Object.prototype.hasOwnProperty.call(statWatchers, filename) &&
-      statWatchers[filename];
-}
-
+const statWatchers = new Map();
 
 fs.watchFile = function(filename) {
   nullCheck(filename);
@@ -1332,22 +1329,24 @@ fs.watchFile = function(filename) {
     throw new Error('watchFile requires a listener function');
   }
 
-  if (inStatWatchers(filename)) {
-    stat = statWatchers[filename];
-  } else {
-    stat = statWatchers[filename] = new StatWatcher();
+  stat = statWatchers.get(filename);
+
+  if (stat === undefined) {
+    stat = new StatWatcher();
     stat.start(filename, options.persistent, options.interval);
+    statWatchers.set(filename, stat);
   }
+
   stat.addListener('change', listener);
   return stat;
 };
 
 fs.unwatchFile = function(filename, listener) {
   nullCheck(filename);
   filename = pathModule.resolve(filename);
-  if (!inStatWatchers(filename)) return;
+  var stat = statWatchers.get(filename);
 
-  var stat = statWatchers[filename];
+  if (stat === undefined) return;
 
   if (typeof listener === 'function') {
     stat.removeListener('change', listener);
@@ -1357,7 +1356,7 @@ fs.unwatchFile = function(filename, listener) {
 
   if (EventEmitter.listenerCount(stat, 'change') === 0) {
     stat.stop();
-    statWatchers[filename] = undefined;
+    statWatchers.delete(filename);
   }
 };
 
@@ -1629,8 +1628,8 @@ function ReadStream(path, options) {
   this.flags = options.flags === undefined ? 'r' : options.flags;
   this.mode = options.mode === undefined ? 0o666 : options.mode;
 
-  this.start = options.start === undefined ? undefined : options.start;
-  this.end = options.end === undefined ? undefined : options.end;
+  this.start = options.start;
+  this.end = options.end;
   this.autoClose = options.autoClose === undefined ? true : options.autoClose;
   this.pos = undefined;
 
@@ -1792,7 +1791,7 @@ function WriteStream(path, options) {
   this.flags = options.flags === undefined ? 'w' : options.flags;
   this.mode = options.mode === undefined ? 0o666 : options.mode;
 
-  this.start = options.start === undefined ? undefined : options.start;
+  this.start = options.start;
   this.pos = undefined;
   this.bytesWritten = 0;
 
@@ -1879,8 +1878,11 @@ util.inherits(SyncWriteStream, Stream);
 
 
 // Export
-fs.SyncWriteStream = SyncWriteStream;
-
+Object.defineProperty(fs, 'SyncWriteStream', {
+    configurable: true,
+    writable: true,
+    value: SyncWriteStream
+});
 
 SyncWriteStream.prototype.write = function(data, arg1, arg2) {
   var encoding, cb;

src/fs_event_wrap.cc

@@ -86,7 +86,7 @@ void FSEventWrap::Start(const FunctionCallbackInfo<Value>& args) {
   FSEventWrap* wrap = Unwrap<FSEventWrap>(args.Holder());
 
   if (args.Length() < 1 || !args[0]->IsString()) {
-    return env->ThrowTypeError("Bad arguments");
+    return env->ThrowTypeError("filename must be a valid string");
   }
 
   node::Utf8Value path(env->isolate(), args[0]);