Skip to content

Commit e8e969a

Browse files
thefourtheyethefourtheye
committed
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: #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>
1 parent 1d4ba1b commit e8e969a

File tree

3 files changed

+65
-181
lines changed

3 files changed

+65
-181
lines changed

‎lib/fs.js‎

Lines changed: 51 additions & 167 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,24 @@ const isWindows = process.platform === 'win32';
3636
const DEBUG = process.env.NODE_DEBUG && /fs/.test(process.env.NODE_DEBUG);
3737
const errnoException = util._errnoException;
3838

39-
function throwOptionsError(options) {
40-
throw new TypeError('Expected options to be either an object or a string, ' +
41-
'but got ' + typeof options + ' instead');
39+
function getOptions(options, defaultOptions) {
40+
if (options === null || options === undefined ||
41+
typeof options === 'function') {
42+
return defaultOptions;
43+
}
44+
45+
if (typeof options === 'string') {
46+
defaultOptions = util._extend({}, defaultOptions);
47+
defaultOptions.encoding = options;
48+
options = defaultOptions;
49+
} else if (typeof options !== 'object') {
50+
throw new TypeError('"options" must be a string or an object, got ' +
51+
typeof options + ' instead.');
52+
}
53+
54+
if (options.encoding !== 'buffer')
55+
assertEncoding(options.encoding);
56+
return options;
4257
}
4358

4459
function rethrow() {
@@ -228,26 +243,14 @@ fs.existsSync = function(path) {
228243
}
229244
};
230245

231-
fs.readFile = function(path, options, callback_) {
232-
var callback = maybeCallback(arguments[arguments.length - 1]);
233-
234-
if (!options || typeof options === 'function') {
235-
options = { encoding: null, flag: 'r' };
236-
} else if (typeof options === 'string') {
237-
options = { encoding: options, flag: 'r' };
238-
} else if (typeof options !== 'object') {
239-
throwOptionsError(options);
240-
}
241-
242-
var encoding = options.encoding;
243-
assertEncoding(encoding);
244-
245-
var flag = options.flag || 'r';
246+
fs.readFile = function(path, options, callback) {
247+
callback = maybeCallback(arguments[arguments.length - 1]);
248+
options = getOptions(options, { flag: 'r' });
246249

247250
if (!nullCheck(path, callback))
248251
return;
249252

250-
var context = new ReadFileContext(callback, encoding);
253+
var context = new ReadFileContext(callback, options.encoding);
251254
context.isUserFd = isFd(path); // file descriptor ownership
252255
var req = new FSReqWrap();
253256
req.context = context;
@@ -261,7 +264,7 @@ fs.readFile = function(path, options, callback_) {
261264
}
262265

263266
binding.open(pathModule._makeLong(path),
264-
stringToFlags(flag),
267+
stringToFlags(options.flag || 'r'),
265268
0o666,
266269
req);
267270
};
@@ -452,20 +455,9 @@ function tryReadSync(fd, isUserFd, buffer, pos, len) {
452455
}
453456

454457
fs.readFileSync = function(path, options) {
455-
if (!options) {
456-
options = { encoding: null, flag: 'r' };
457-
} else if (typeof options === 'string') {
458-
options = { encoding: options, flag: 'r' };
459-
} else if (typeof options !== 'object') {
460-
throwOptionsError(options);
461-
}
462-
463-
var encoding = options.encoding;
464-
assertEncoding(encoding);
465-
466-
var flag = options.flag || 'r';
458+
options = getOptions(options, { flag: 'r' });
467459
var isUserFd = isFd(path); // file descriptor ownership
468-
var fd = isUserFd ? path : fs.openSync(path, flag, 0o666);
460+
var fd = isUserFd ? path : fs.openSync(path, options.flag || 'r', 0o666);
469461

470462
var st = tryStatSync(fd, isUserFd);
471463
var size = st.isFile() ? st.size : 0;
@@ -509,7 +501,7 @@ fs.readFileSync = function(path, options) {
509501
buffer = buffer.slice(0, pos);
510502
}
511503

512-
if (encoding) buffer = buffer.toString(encoding);
504+
if (options.encoding) buffer = buffer.toString(options.encoding);
513505
return buffer;
514506
};
515507

@@ -849,29 +841,16 @@ fs.mkdirSync = function(path, mode) {
849841
};
850842

851843
fs.readdir = function(path, options, callback) {
852-
options = options || {};
853-
if (typeof options === 'function') {
854-
callback = options;
855-
options = {};
856-
} else if (typeof options === 'string') {
857-
options = {encoding: options};
858-
}
859-
if (typeof options !== 'object')
860-
throw new TypeError('"options" must be a string or an object');
861-
862-
callback = makeCallback(callback);
844+
callback = makeCallback(typeof options === 'function' ? options : callback);
845+
options = getOptions(options, {});
863846
if (!nullCheck(path, callback)) return;
864847
var req = new FSReqWrap();
865848
req.oncomplete = callback;
866849
binding.readdir(pathModule._makeLong(path), options.encoding, req);
867850
};
868851

869852
fs.readdirSync = function(path, options) {
870-
options = options || {};
871-
if (typeof options === 'string')
872-
options = {encoding: options};
873-
if (typeof options !== 'object')
874-
throw new TypeError('"options" must be a string or an object');
853+
options = getOptions(options, {});
875854
nullCheck(path);
876855
return binding.readdir(pathModule._makeLong(path), options.encoding);
877856
};
@@ -913,28 +892,16 @@ fs.statSync = function(path) {
913892
};
914893

915894
fs.readlink = function(path, options, callback) {
916-
options = options || {};
917-
if (typeof options === 'function') {
918-
callback = options;
919-
options = {};
920-
} else if (typeof options === 'string') {
921-
options = {encoding: options};
922-
}
923-
if (typeof options !== 'object')
924-
throw new TypeError('"options" must be a string or an object');
925-
callback = makeCallback(callback);
895+
callback = makeCallback(typeof options === 'function' ? options : callback);
896+
options = getOptions(options, {});
926897
if (!nullCheck(path, callback)) return;
927898
var req = new FSReqWrap();
928899
req.oncomplete = callback;
929900
binding.readlink(pathModule._makeLong(path), options.encoding, req);
930901
};
931902

932903
fs.readlinkSync = function(path, options) {
933-
options = options || {};
934-
if (typeof options === 'string')
935-
options = {encoding: options};
936-
if (typeof options !== 'object')
937-
throw new TypeError('"options" must be a string or an object');
904+
options = getOptions(options, {});
938905
nullCheck(path);
939906
return binding.readlink(pathModule._makeLong(path), options.encoding);
940907
};
@@ -1205,20 +1172,10 @@ function writeAll(fd, isUserFd, buffer, offset, length, position, callback_) {
12051172
});
12061173
}
12071174

1208-
fs.writeFile = function(path, data, options, callback_) {
1209-
var callback = maybeCallback(arguments[arguments.length - 1]);
1210-
1211-
if (!options || typeof options === 'function') {
1212-
options = { encoding: 'utf8', mode: 0o666, flag: 'w' };
1213-
} else if (typeof options === 'string') {
1214-
options = { encoding: options, mode: 0o666, flag: 'w' };
1215-
} else if (typeof options !== 'object') {
1216-
throwOptionsError(options);
1217-
}
1218-
1219-
assertEncoding(options.encoding);
1220-
1221-
var flag = options.flag || 'w';
1175+
fs.writeFile = function(path, data, options, callback) {
1176+
callback = maybeCallback(arguments[arguments.length - 1]);
1177+
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
1178+
const flag = options.flag || 'w';
12221179

12231180
if (isFd(path)) {
12241181
writeFd(path, true);
@@ -1243,17 +1200,9 @@ fs.writeFile = function(path, data, options, callback_) {
12431200
};
12441201

12451202
fs.writeFileSync = function(path, data, options) {
1246-
if (!options) {
1247-
options = { encoding: 'utf8', mode: 0o666, flag: 'w' };
1248-
} else if (typeof options === 'string') {
1249-
options = { encoding: options, mode: 0o666, flag: 'w' };
1250-
} else if (typeof options !== 'object') {
1251-
throwOptionsError(options);
1252-
}
1253-
1254-
assertEncoding(options.encoding);
1203+
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
1204+
const flag = options.flag || 'w';
12551205

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

@@ -1277,16 +1226,9 @@ fs.writeFileSync = function(path, data, options) {
12771226
}
12781227
};
12791228

1280-
fs.appendFile = function(path, data, options, callback_) {
1281-
var callback = maybeCallback(arguments[arguments.length - 1]);
1282-
1283-
if (!options || typeof options === 'function') {
1284-
options = { encoding: 'utf8', mode: 0o666, flag: 'a' };
1285-
} else if (typeof options === 'string') {
1286-
options = { encoding: options, mode: 0o666, flag: 'a' };
1287-
} else if (typeof options !== 'object') {
1288-
throwOptionsError(options);
1289-
}
1229+
fs.appendFile = function(path, data, options, callback) {
1230+
callback = maybeCallback(arguments[arguments.length - 1]);
1231+
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' });
12901232

12911233
if (!options.flag)
12921234
options = util._extend({ flag: 'a' }, options);
@@ -1299,13 +1241,7 @@ fs.appendFile = function(path, data, options, callback_) {
12991241
};
13001242

13011243
fs.appendFileSync = function(path, data, options) {
1302-
if (!options) {
1303-
options = { encoding: 'utf8', mode: 0o666, flag: 'a' };
1304-
} else if (typeof options === 'string') {
1305-
options = { encoding: options, mode: 0o666, flag: 'a' };
1306-
} else if (typeof options !== 'object') {
1307-
throwOptionsError(options);
1308-
}
1244+
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' });
13091245

13101246
if (!options.flag)
13111247
options = util._extend({ flag: 'a' }, options);
@@ -1364,15 +1300,10 @@ FSWatcher.prototype.close = function() {
13641300
fs.watch = function(filename, options, listener) {
13651301
nullCheck(filename);
13661302

1367-
options = options || {};
13681303
if (typeof options === 'function') {
13691304
listener = options;
1370-
options = {};
1371-
} else if (typeof options === 'string') {
1372-
options = {encoding: options};
13731305
}
1374-
if (typeof options !== 'object')
1375-
throw new TypeError('"options" must be a string or an object');
1306+
options = getOptions(options, {});
13761307

13771308
if (options.persistent === undefined) options.persistent = true;
13781309
if (options.recursive === undefined) options.recursive = false;
@@ -1519,12 +1450,7 @@ function encodeRealpathResult(result, options, err) {
15191450
const realpathCacheKey = fs.realpathCacheKey = Symbol('realpathCacheKey');
15201451

15211452
fs.realpathSync = function realpathSync(p, options) {
1522-
if (!options)
1523-
options = {};
1524-
else if (typeof options === 'string')
1525-
options = {encoding: options};
1526-
else if (typeof options !== 'object')
1527-
throw new TypeError('"options" must be a string or an object');
1453+
options = getOptions(options, {});
15281454
nullCheck(p);
15291455

15301456
p = p.toString('utf8');
@@ -1625,20 +1551,8 @@ fs.realpathSync = function realpathSync(p, options) {
16251551

16261552

16271553
fs.realpath = function realpath(p, options, callback) {
1628-
if (typeof callback !== 'function') {
1629-
callback = maybeCallback(options);
1630-
options = {};
1631-
}
1632-
1633-
if (!options) {
1634-
options = {};
1635-
} else if (typeof options === 'function') {
1636-
options = {};
1637-
} else if (typeof options === 'string') {
1638-
options = {encoding: options};
1639-
} else if (typeof options !== 'object') {
1640-
throw new TypeError('"options" must be a string or an object');
1641-
}
1554+
callback = maybeCallback(typeof options === 'function' ? options : callback);
1555+
options = getOptions(options, {});
16421556
if (!nullCheck(p, callback))
16431557
return;
16441558

@@ -1747,20 +1661,10 @@ fs.realpath = function realpath(p, options, callback) {
17471661
};
17481662

17491663
fs.mkdtemp = function(prefix, options, callback) {
1664+
callback = makeCallback(typeof options === 'function' ? options : callback);
1665+
options = getOptions(options, {});
17501666
if (!prefix || typeof prefix !== 'string')
17511667
throw new TypeError('filename prefix is required');
1752-
1753-
options = options || {};
1754-
if (typeof options === 'function') {
1755-
callback = options;
1756-
options = {};
1757-
} else if (typeof options === 'string') {
1758-
options = {encoding: options};
1759-
}
1760-
if (typeof options !== 'object')
1761-
throw new TypeError('"options" must be a string or an object');
1762-
1763-
callback = makeCallback(callback);
17641668
if (!nullCheck(prefix, callback)) {
17651669
return;
17661670
}
@@ -1775,14 +1679,8 @@ fs.mkdtemp = function(prefix, options, callback) {
17751679
fs.mkdtempSync = function(prefix, options) {
17761680
if (!prefix || typeof prefix !== 'string')
17771681
throw new TypeError('filename prefix is required');
1778-
1779-
options = options || {};
1780-
if (typeof options === 'string')
1781-
options = {encoding: options};
1782-
if (typeof options !== 'object')
1783-
throw new TypeError('"options" must be a string or an object');
1682+
options = getOptions(options, {});
17841683
nullCheck(prefix);
1785-
17861684
return binding.mkdtemp(prefix + 'XXXXXX', options.encoding);
17871685
};
17881686

@@ -1806,15 +1704,8 @@ function ReadStream(path, options) {
18061704
if (!(this instanceof ReadStream))
18071705
return new ReadStream(path, options);
18081706

1809-
if (options === undefined)
1810-
options = {};
1811-
else if (typeof options === 'string')
1812-
options = { encoding: options };
1813-
else if (options === null || typeof options !== 'object')
1814-
throw new TypeError('"options" argument must be a string or an object');
1815-
18161707
// a little bit bigger buffer and water marks by default
1817-
options = Object.create(options);
1708+
options = Object.create(getOptions(options, {}));
18181709
if (options.highWaterMark === undefined)
18191710
options.highWaterMark = 64 * 1024;
18201711

@@ -1980,14 +1871,7 @@ function WriteStream(path, options) {
19801871
if (!(this instanceof WriteStream))
19811872
return new WriteStream(path, options);
19821873

1983-
if (options === undefined)
1984-
options = {};
1985-
else if (typeof options === 'string')
1986-
options = { encoding: options };
1987-
else if (options === null || typeof options !== 'object')
1988-
throw new TypeError('"options" argument must be a string or an object');
1989-
1990-
options = Object.create(options);
1874+
options = Object.create(getOptions(options, {}));
19911875

19921876
Writable.call(this, options);
19931877

0 commit comments

Comments
 (0)