Skip to content

Commit

Permalink
fs: refactor "options" processing as a function
Browse files Browse the repository at this point in the history 
As it is, the "options" processing is repeated in all the functions
which need it. That introduces checks which are inconsistent with
other functions and produces slightly different error messages.

This patch moves the basic "options" validation and processing to a
seperate function.

PR-URL: #7165
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Nicu Micleușanu <micnic90@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@thefourtheye @jasnell
thefourtheye authored and jasnell committed Oct 10, 2016
1 parent 2c7019b commit 169f485
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 181 deletions.
218 changes: 51 additions & 167 deletions lib/fs.js
View file
Expand Up @@ -44,9 +44,24 @@ 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 getOptions(options, defaultOptions) {
if (options === null || options === undefined ||
typeof options === 'function') {
return defaultOptions;
}

if (typeof options === 'string') {
defaultOptions = util._extend({}, defaultOptions);
defaultOptions.encoding = options;
options = defaultOptions;
} else if (typeof options !== 'object') {
throw new TypeError('"options" must be a string or an object, got ' +
typeof options + ' instead.');
}

if (options.encoding !== 'buffer')
assertEncoding(options.encoding);
return options;
}

function rethrow() {
Expand Down Expand Up @@ -236,26 +251,14 @@ fs.existsSync = function(path) {
}
};

fs.readFile = function(path, options, callback_) {
var callback = maybeCallback(arguments[arguments.length - 1]);

if (!options || typeof options === 'function') {
options = { encoding: null, flag: 'r' };
} else if (typeof options === 'string') {
options = { encoding: options, flag: 'r' };
} else if (typeof options !== 'object') {
throwOptionsError(options);
}

var encoding = options.encoding;
assertEncoding(encoding);

var flag = options.flag || 'r';
fs.readFile = function(path, options, callback) {
callback = maybeCallback(arguments[arguments.length - 1]);
options = getOptions(options, { flag: 'r' });

if (!nullCheck(path, callback))
return;

var context = new ReadFileContext(callback, encoding);
var context = new ReadFileContext(callback, options.encoding);
context.isUserFd = isFd(path); // file descriptor ownership
var req = new FSReqWrap();
req.context = context;
Expand All @@ -269,7 +272,7 @@ fs.readFile = function(path, options, callback_) {
}

binding.open(pathModule._makeLong(path),
stringToFlags(flag),
stringToFlags(options.flag || 'r'),
0o666,
req);
};
Expand Down Expand Up @@ -460,20 +463,9 @@ function tryReadSync(fd, isUserFd, buffer, pos, len) {
}

fs.readFileSync = function(path, options) {
if (!options) {
options = { encoding: null, flag: 'r' };
} else if (typeof options === 'string') {
options = { encoding: options, flag: 'r' };
} else if (typeof options !== 'object') {
throwOptionsError(options);
}

var encoding = options.encoding;
assertEncoding(encoding);

var flag = options.flag || 'r';
options = getOptions(options, { flag: 'r' });
var isUserFd = isFd(path); // file descriptor ownership
var fd = isUserFd ? path : fs.openSync(path, flag, 0o666);
var fd = isUserFd ? path : fs.openSync(path, options.flag || 'r', 0o666);

var st = tryStatSync(fd, isUserFd);
var size = st.isFile() ? st.size : 0;
Expand Down Expand Up @@ -517,7 +509,7 @@ fs.readFileSync = function(path, options) {
buffer = buffer.slice(0, pos);
}

if (encoding) buffer = buffer.toString(encoding);
if (options.encoding) buffer = buffer.toString(options.encoding);
return buffer;
};

Expand Down Expand Up @@ -899,29 +891,16 @@ fs.mkdirSync = function(path, mode) {
};

fs.readdir = function(path, options, callback) {
options = options || {};
if (typeof options === 'function') {
callback = options;
options = {};
} else if (typeof options === 'string') {
options = {encoding: options};
}
if (typeof options !== 'object')
throw new TypeError('"options" must be a string or an object');

callback = makeCallback(callback);
callback = makeCallback(typeof options === 'function' ? options : callback);
options = getOptions(options, {});
if (!nullCheck(path, callback)) return;
var req = new FSReqWrap();
req.oncomplete = callback;
binding.readdir(pathModule._makeLong(path), options.encoding, req);
};

fs.readdirSync = function(path, options) {
options = options || {};
if (typeof options === 'string')
options = {encoding: options};
if (typeof options !== 'object')
throw new TypeError('"options" must be a string or an object');
options = getOptions(options, {});
nullCheck(path);
return binding.readdir(pathModule._makeLong(path), options.encoding);
};
Expand Down Expand Up @@ -963,28 +942,16 @@ fs.statSync = function(path) {
};

fs.readlink = function(path, options, callback) {
options = options || {};
if (typeof options === 'function') {
callback = options;
options = {};
} else if (typeof options === 'string') {
options = {encoding: options};
}
if (typeof options !== 'object')
throw new TypeError('"options" must be a string or an object');
callback = makeCallback(callback);
callback = makeCallback(typeof options === 'function' ? options : callback);
options = getOptions(options, {});
if (!nullCheck(path, callback)) return;
var req = new FSReqWrap();
req.oncomplete = callback;
binding.readlink(pathModule._makeLong(path), options.encoding, req);
};

fs.readlinkSync = function(path, options) {
options = options || {};
if (typeof options === 'string')
options = {encoding: options};
if (typeof options !== 'object')
throw new TypeError('"options" must be a string or an object');
options = getOptions(options, {});
nullCheck(path);
return binding.readlink(pathModule._makeLong(path), options.encoding);
};
Expand Down Expand Up @@ -1255,20 +1222,10 @@ function writeAll(fd, isUserFd, buffer, offset, length, position, 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') {
throwOptionsError(options);
}

assertEncoding(options.encoding);

var flag = options.flag || 'w';
fs.writeFile = function(path, data, options, callback) {
callback = maybeCallback(arguments[arguments.length - 1]);
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
const flag = options.flag || 'w';

if (isFd(path)) {
writeFd(path, true);
Expand All @@ -1293,17 +1250,9 @@ fs.writeFile = function(path, data, options, callback_) {
};

fs.writeFileSync = function(path, data, options) {
if (!options) {
options = { encoding: 'utf8', mode: 0o666, flag: 'w' };
} else if (typeof options === 'string') {
options = { encoding: options, mode: 0o666, flag: 'w' };
} else if (typeof options !== 'object') {
throwOptionsError(options);
}

assertEncoding(options.encoding);
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
const flag = options.flag || 'w';

var flag = options.flag || 'w';
var isUserFd = isFd(path); // file descriptor ownership
var fd = isUserFd ? path : fs.openSync(path, flag, options.mode);

Expand All @@ -1327,16 +1276,9 @@ fs.writeFileSync = function(path, data, options) {
}
};

fs.appendFile = function(path, data, options, callback_) {
var callback = maybeCallback(arguments[arguments.length - 1]);

if (!options || typeof options === 'function') {
options = { encoding: 'utf8', mode: 0o666, flag: 'a' };
} else if (typeof options === 'string') {
options = { encoding: options, mode: 0o666, flag: 'a' };
} else if (typeof options !== 'object') {
throwOptionsError(options);
}
fs.appendFile = function(path, data, options, callback) {
callback = maybeCallback(arguments[arguments.length - 1]);
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' });

if (!options.flag)
options = util._extend({ flag: 'a' }, options);
Expand All @@ -1349,13 +1291,7 @@ fs.appendFile = function(path, data, options, callback_) {
};

fs.appendFileSync = function(path, data, options) {
if (!options) {
options = { encoding: 'utf8', mode: 0o666, flag: 'a' };
} else if (typeof options === 'string') {
options = { encoding: options, mode: 0o666, flag: 'a' };
} else if (typeof options !== 'object') {
throwOptionsError(options);
}
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' });

if (!options.flag)
options = util._extend({ flag: 'a' }, options);
Expand Down Expand Up @@ -1414,15 +1350,10 @@ FSWatcher.prototype.close = function() {
fs.watch = function(filename, options, listener) {
nullCheck(filename);

options = options || {};
if (typeof options === 'function') {
listener = options;
options = {};
} else if (typeof options === 'string') {
options = {encoding: options};
}
if (typeof options !== 'object')
throw new TypeError('"options" must be a string or an object');
options = getOptions(options, {});

if (options.persistent === undefined) options.persistent = true;
if (options.recursive === undefined) options.recursive = false;
Expand Down Expand Up @@ -1569,12 +1500,7 @@ function encodeRealpathResult(result, options, err) {
const realpathCacheKey = fs.realpathCacheKey = Symbol('realpathCacheKey');

fs.realpathSync = function realpathSync(p, options) {
if (!options)
options = {};
else if (typeof options === 'string')
options = {encoding: options};
else if (typeof options !== 'object')
throw new TypeError('"options" must be a string or an object');
options = getOptions(options, {});
nullCheck(p);

p = p.toString('utf8');
Expand Down Expand Up @@ -1675,20 +1601,8 @@ fs.realpathSync = function realpathSync(p, options) {


fs.realpath = function realpath(p, options, callback) {
if (typeof callback !== 'function') {
callback = maybeCallback(options);
options = {};
}

if (!options) {
options = {};
} else if (typeof options === 'function') {
options = {};
} else if (typeof options === 'string') {
options = {encoding: options};
} else if (typeof options !== 'object') {
throw new TypeError('"options" must be a string or an object');
}
callback = maybeCallback(typeof options === 'function' ? options : callback);
options = getOptions(options, {});
if (!nullCheck(p, callback))
return;

Expand Down Expand Up @@ -1797,20 +1711,10 @@ fs.realpath = function realpath(p, options, callback) {
};

fs.mkdtemp = function(prefix, options, callback) {
callback = makeCallback(typeof options === 'function' ? options : callback);
options = getOptions(options, {});
if (!prefix || typeof prefix !== 'string')
throw new TypeError('filename prefix is required');

options = options || {};
if (typeof options === 'function') {
callback = options;
options = {};
} else if (typeof options === 'string') {
options = {encoding: options};
}
if (typeof options !== 'object')
throw new TypeError('"options" must be a string or an object');

callback = makeCallback(callback);
if (!nullCheck(prefix, callback)) {
return;
}
Expand All @@ -1825,14 +1729,8 @@ fs.mkdtemp = function(prefix, options, callback) {
fs.mkdtempSync = function(prefix, options) {
if (!prefix || typeof prefix !== 'string')
throw new TypeError('filename prefix is required');

options = options || {};
if (typeof options === 'string')
options = {encoding: options};
if (typeof options !== 'object')
throw new TypeError('"options" must be a string or an object');
options = getOptions(options, {});
nullCheck(prefix);

return binding.mkdtemp(prefix + 'XXXXXX', options.encoding);
};

Expand All @@ -1856,15 +1754,8 @@ function ReadStream(path, options) {
if (!(this instanceof ReadStream))
return new ReadStream(path, options);

if (options === undefined)
options = {};
else if (typeof options === 'string')
options = { encoding: options };
else if (options === null || typeof options !== 'object')
throw new TypeError('"options" argument must be a string or an object');

// a little bit bigger buffer and water marks by default
options = Object.create(options);
options = Object.create(getOptions(options, {}));
if (options.highWaterMark === undefined)
options.highWaterMark = 64 * 1024;

Expand Down Expand Up @@ -2030,14 +1921,7 @@ function WriteStream(path, options) {
if (!(this instanceof WriteStream))
return new WriteStream(path, options);

if (options === undefined)
options = {};
else if (typeof options === 'string')
options = { encoding: options };
else if (options === null || typeof options !== 'object')
throw new TypeError('"options" argument must be a string or an object');

options = Object.create(options);
options = Object.create(getOptions(options, {}));

Writable.call(this, options);

Expand Down

0 comments on commit 169f485

Please sign in to comment.