lukechilds
/
node
mirror of https://github.com/lukechilds/node.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

fs: refactor "options" processing as a function 

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: https://github.com/nodejs/node/pull/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>
v6
Sakthipriyan Vairamani 9 years ago
committed by Sakthipriyan Vairamani (thefourtheye)
parent
1d4ba1be1a
commit
e8e969a5c1
3 changed files with 65 additions and 181 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 218
      lib/fs.js
  2. 14
      test/parallel/test-fs-read-stream-throw-type-error.js
  3. 14
      test/parallel/test-fs-write-stream-throw-type-error.js

218
lib/fs.js
View File

@ -36,9 +36,24 @@ const isWindows = process.platform === 'win32';
const DEBUG = process.env.NODE_DEBUG && /fs/.test(process.env.NODE_DEBUG); const DEBUG = process.env.NODE_DEBUG && /fs/.test(process.env.NODE_DEBUG);
const errnoException = util._errnoException; const errnoException = util._errnoException;
function throwOptionsError(options) { function getOptions(options, defaultOptions) {
throw new TypeError('Expected options to be either an object or a string, ' + if (options === null || options === undefined ||
'but got ' + typeof options + ' instead'); 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() { function rethrow() {
@ -228,26 +243,14 @@ fs.existsSync = function(path) {
} }
}; };
fs.readFile = function(path, options, callback_) { fs.readFile = function(path, options, callback) {
var callback = maybeCallback(arguments[arguments.length - 1]); callback = maybeCallback(arguments[arguments.length - 1]);
options = getOptions(options, { flag: 'r' });
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';
if (!nullCheck(path, callback)) if (!nullCheck(path, callback))
return; return;
var context = new ReadFileContext(callback, encoding); var context = new ReadFileContext(callback, options.encoding);
context.isUserFd = isFd(path); // file descriptor ownership context.isUserFd = isFd(path); // file descriptor ownership
var req = new FSReqWrap(); var req = new FSReqWrap();
req.context = context; req.context = context;
@ -261,7 +264,7 @@ fs.readFile = function(path, options, callback_) {
} }
binding.open(pathModule._makeLong(path), binding.open(pathModule._makeLong(path),
stringToFlags(flag), stringToFlags(options.flag || 'r'),
0o666, 0o666,
req); req);
}; };
@ -452,20 +455,9 @@ function tryReadSync(fd, isUserFd, buffer, pos, len) {
} }
fs.readFileSync = function(path, options) { fs.readFileSync = function(path, options) {
if (!options) { options = getOptions(options, { flag: 'r' });
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';
var isUserFd = isFd(path); // file descriptor ownership 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 st = tryStatSync(fd, isUserFd);
var size = st.isFile() ? st.size : 0; var size = st.isFile() ? st.size : 0;
@ -509,7 +501,7 @@ fs.readFileSync = function(path, options) {
buffer = buffer.slice(0, pos); buffer = buffer.slice(0, pos);
} }
if (encoding) buffer = buffer.toString(encoding); if (options.encoding) buffer = buffer.toString(options.encoding);
return buffer; return buffer;
}; };
@ -849,17 +841,8 @@ fs.mkdirSync = function(path, mode) {
}; };
fs.readdir = function(path, options, callback) { fs.readdir = function(path, options, callback) {
options = options || {}; callback = makeCallback(typeof options === 'function' ? options : callback);
if (typeof options === 'function') { options = getOptions(options, {});
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(path, callback)) return; if (!nullCheck(path, callback)) return;
var req = new FSReqWrap(); var req = new FSReqWrap();
req.oncomplete = callback; req.oncomplete = callback;
@ -867,11 +850,7 @@ fs.readdir = function(path, options, callback) {
}; };
fs.readdirSync = function(path, options) { fs.readdirSync = function(path, options) {
options = options || {}; options = getOptions(options, {});
if (typeof options === 'string')
options = {encoding: options};
if (typeof options !== 'object')
throw new TypeError('"options" must be a string or an object');
nullCheck(path); nullCheck(path);
return binding.readdir(pathModule._makeLong(path), options.encoding); return binding.readdir(pathModule._makeLong(path), options.encoding);
}; };
@ -913,16 +892,8 @@ fs.statSync = function(path) {
}; };
fs.readlink = function(path, options, callback) { fs.readlink = function(path, options, callback) {
options = options || {}; callback = makeCallback(typeof options === 'function' ? options : callback);
if (typeof options === 'function') { options = getOptions(options, {});
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(path, callback)) return; if (!nullCheck(path, callback)) return;
var req = new FSReqWrap(); var req = new FSReqWrap();
req.oncomplete = callback; req.oncomplete = callback;
@ -930,11 +901,7 @@ fs.readlink = function(path, options, callback) {
}; };
fs.readlinkSync = function(path, options) { fs.readlinkSync = function(path, options) {
options = options || {}; options = getOptions(options, {});
if (typeof options === 'string')
options = {encoding: options};
if (typeof options !== 'object')
throw new TypeError('"options" must be a string or an object');
nullCheck(path); nullCheck(path);
return binding.readlink(pathModule._makeLong(path), options.encoding); return binding.readlink(pathModule._makeLong(path), options.encoding);
}; };
@ -1205,20 +1172,10 @@ function writeAll(fd, isUserFd, 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]); callback = maybeCallback(arguments[arguments.length - 1]);
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
if (!options || typeof options === 'function') { const flag = options.flag || 'w';
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';
if (isFd(path)) { if (isFd(path)) {
writeFd(path, true); writeFd(path, true);
@ -1243,17 +1200,9 @@ fs.writeFile = function(path, data, options, callback_) {
}; };
fs.writeFileSync = function(path, data, options) { fs.writeFileSync = function(path, data, options) {
if (!options) { options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
options = { encoding: 'utf8', mode: 0o666, flag: 'w' }; const flag = options.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';
var isUserFd = isFd(path); // file descriptor ownership var isUserFd = isFd(path); // file descriptor ownership
var fd = isUserFd ? path : fs.openSync(path, flag, options.mode); var fd = isUserFd ? path : fs.openSync(path, flag, options.mode);
@ -1277,16 +1226,9 @@ fs.writeFileSync = function(path, data, options) {
} }
}; };
fs.appendFile = function(path, data, options, callback_) { fs.appendFile = function(path, data, options, callback) {
var callback = maybeCallback(arguments[arguments.length - 1]); callback = maybeCallback(arguments[arguments.length - 1]);
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' });
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);
}
if (!options.flag) if (!options.flag)
options = util._extend({ flag: 'a' }, options); options = util._extend({ flag: 'a' }, options);
@ -1299,13 +1241,7 @@ fs.appendFile = function(path, data, options, callback_) {
}; };
fs.appendFileSync = function(path, data, options) { fs.appendFileSync = function(path, data, options) {
if (!options) { options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' });
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);
}
if (!options.flag) if (!options.flag)
options = util._extend({ flag: 'a' }, options); options = util._extend({ flag: 'a' }, options);
@ -1364,15 +1300,10 @@ FSWatcher.prototype.close = function() {
fs.watch = function(filename, options, listener) { fs.watch = function(filename, options, listener) {
nullCheck(filename); nullCheck(filename);
options = options || {};
if (typeof options === 'function') { if (typeof options === 'function') {
listener = options; listener = options;
options = {};
} else if (typeof options === 'string') {
options = {encoding: options};
} }
if (typeof options !== 'object') options = getOptions(options, {});
throw new TypeError('"options" must be a string or an object');
if (options.persistent === undefined) options.persistent = true; if (options.persistent === undefined) options.persistent = true;
if (options.recursive === undefined) options.recursive = false; if (options.recursive === undefined) options.recursive = false;
@ -1519,12 +1450,7 @@ function encodeRealpathResult(result, options, err) {
const realpathCacheKey = fs.realpathCacheKey = Symbol('realpathCacheKey'); const realpathCacheKey = fs.realpathCacheKey = Symbol('realpathCacheKey');
fs.realpathSync = function realpathSync(p, options) { fs.realpathSync = function realpathSync(p, options) {
if (!options) options = getOptions(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');
nullCheck(p); nullCheck(p);
p = p.toString('utf8'); p = p.toString('utf8');
@ -1625,20 +1551,8 @@ fs.realpathSync = function realpathSync(p, options) {
fs.realpath = function realpath(p, options, callback) { fs.realpath = function realpath(p, options, callback) {
if (typeof callback !== 'function') { callback = maybeCallback(typeof options === 'function' ? options : callback);
callback = maybeCallback(options); options = getOptions(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');
}
if (!nullCheck(p, callback)) if (!nullCheck(p, callback))
return; return;
@ -1747,20 +1661,10 @@ fs.realpath = function realpath(p, options, callback) {
}; };
fs.mkdtemp = function(prefix, options, callback) { fs.mkdtemp = function(prefix, options, callback) {
callback = makeCallback(typeof options === 'function' ? options : callback);
options = getOptions(options, {});
if (!prefix || typeof prefix !== 'string') if (!prefix || typeof prefix !== 'string')
throw new TypeError('filename prefix is required'); 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)) { if (!nullCheck(prefix, callback)) {
return; return;
} }
@ -1775,14 +1679,8 @@ fs.mkdtemp = function(prefix, options, callback) {
fs.mkdtempSync = function(prefix, options) { fs.mkdtempSync = function(prefix, options) {
if (!prefix || typeof prefix !== 'string') if (!prefix || typeof prefix !== 'string')
throw new TypeError('filename prefix is required'); throw new TypeError('filename prefix is required');
options = getOptions(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');
nullCheck(prefix); nullCheck(prefix);
return binding.mkdtemp(prefix + 'XXXXXX', options.encoding); return binding.mkdtemp(prefix + 'XXXXXX', options.encoding);
}; };
@ -1806,15 +1704,8 @@ function ReadStream(path, options) {
if (!(this instanceof ReadStream)) if (!(this instanceof ReadStream))
return new ReadStream(path, options); 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 // a little bit bigger buffer and water marks by default
options = Object.create(options); options = Object.create(getOptions(options, {}));
if (options.highWaterMark === undefined) if (options.highWaterMark === undefined)
options.highWaterMark = 64 * 1024; options.highWaterMark = 64 * 1024;
@ -1980,14 +1871,7 @@ function WriteStream(path, options) {
if (!(this instanceof WriteStream)) if (!(this instanceof WriteStream))
return new WriteStream(path, options); return new WriteStream(path, options);
if (options === undefined) options = Object.create(getOptions(options, {}));
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);
Writable.call(this, options); Writable.call(this, options);

14
test/parallel/test-fs-read-stream-throw-type-error.js
View File

@ -9,6 +9,9 @@ const example = path.join(common.fixturesDir, 'x.txt');
assert.doesNotThrow(function() { assert.doesNotThrow(function() {
fs.createReadStream(example, undefined); fs.createReadStream(example, undefined);
}); });
assert.doesNotThrow(function() {
fs.createReadStream(example, null);
});
assert.doesNotThrow(function() { assert.doesNotThrow(function() {
fs.createReadStream(example, 'utf8'); fs.createReadStream(example, 'utf8');
}); });
@ -16,18 +19,15 @@ assert.doesNotThrow(function() {
fs.createReadStream(example, {encoding: 'utf8'}); fs.createReadStream(example, {encoding: 'utf8'});
}); });
assert.throws(function() {
fs.createReadStream(example, null);
}, /"options" argument must be a string or an object/);
assert.throws(function() { assert.throws(function() {
fs.createReadStream(example, 123); fs.createReadStream(example, 123);
}, /"options" argument must be a string or an object/); }, /"options" must be a string or an object/);
assert.throws(function() { assert.throws(function() {
fs.createReadStream(example, 0); fs.createReadStream(example, 0);
}, /"options" argument must be a string or an object/); }, /"options" must be a string or an object/);
assert.throws(function() { assert.throws(function() {
fs.createReadStream(example, true); fs.createReadStream(example, true);
}, /"options" argument must be a string or an object/); }, /"options" must be a string or an object/);
assert.throws(function() { assert.throws(function() {
fs.createReadStream(example, false); fs.createReadStream(example, false);
}, /"options" argument must be a string or an object/); }, /"options" must be a string or an object/);

14
test/parallel/test-fs-write-stream-throw-type-error.js
View File

@ -11,6 +11,9 @@ common.refreshTmpDir();
assert.doesNotThrow(function() { assert.doesNotThrow(function() {
fs.createWriteStream(example, undefined); fs.createWriteStream(example, undefined);
}); });
assert.doesNotThrow(function() {
fs.createWriteStream(example, null);
});
assert.doesNotThrow(function() { assert.doesNotThrow(function() {
fs.createWriteStream(example, 'utf8'); fs.createWriteStream(example, 'utf8');
}); });
@ -18,18 +21,15 @@ assert.doesNotThrow(function() {
fs.createWriteStream(example, {encoding: 'utf8'}); fs.createWriteStream(example, {encoding: 'utf8'});
}); });
assert.throws(function() {
fs.createWriteStream(example, null);
}, /"options" argument must be a string or an object/);
assert.throws(function() { assert.throws(function() {
fs.createWriteStream(example, 123); fs.createWriteStream(example, 123);
}, /"options" argument must be a string or an object/); }, /"options" must be a string or an object/);
assert.throws(function() { assert.throws(function() {
fs.createWriteStream(example, 0); fs.createWriteStream(example, 0);
}, /"options" argument must be a string or an object/); }, /"options" must be a string or an object/);
assert.throws(function() { assert.throws(function() {
fs.createWriteStream(example, true); fs.createWriteStream(example, true);
}, /"options" argument must be a string or an object/); }, /"options" must be a string or an object/);
assert.throws(function() { assert.throws(function() {
fs.createWriteStream(example, false); fs.createWriteStream(example, false);
}, /"options" argument must be a string or an object/); }, /"options" must be a string or an object/);

Write Preview
Loading…
Cancel
Save