Skip to content

Commit 169f485

Browse files
thefourtheyejasnell
thefourtheye
authored and
jasnell
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 <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: Nicu Micleușanu <[email protected]> Reviewed-By: Yorkie Liu <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 2c7019b commit 169f485

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
@@ -44,9 +44,24 @@ const isWindows = process.platform === 'win32';
4444
const DEBUG = process.env.NODE_DEBUG && /fs/.test(process.env.NODE_DEBUG);
4545
const errnoException = util._errnoException;
4646

47-
function throwOptionsError(options) {
48-
throw new TypeError('Expected options to be either an object or a string, ' +
49-
'but got ' + typeof options + ' instead');
47+
function getOptions(options, defaultOptions) {
48+
if (options === null || options === undefined ||
49+
typeof options === 'function') {
50+
return defaultOptions;
51+
}
52+
53+
if (typeof options === 'string') {
54+
defaultOptions = util._extend({}, defaultOptions);
55+
defaultOptions.encoding = options;
56+
options = defaultOptions;
57+
} else if (typeof options !== 'object') {
58+
throw new TypeError('"options" must be a string or an object, got ' +
59+
typeof options + ' instead.');
60+
}
61+
62+
if (options.encoding !== 'buffer')
63+
assertEncoding(options.encoding);
64+
return options;
5065
}
5166

5267
function rethrow() {
@@ -236,26 +251,14 @@ fs.existsSync = function(path) {
236251
}
237252
};
238253

239-
fs.readFile = function(path, options, callback_) {
240-
var callback = maybeCallback(arguments[arguments.length - 1]);
241-
242-
if (!options || typeof options === 'function') {
243-
options = { encoding: null, flag: 'r' };
244-
} else if (typeof options === 'string') {
245-
options = { encoding: options, flag: 'r' };
246-
} else if (typeof options !== 'object') {
247-
throwOptionsError(options);
248-
}
249-
250-
var encoding = options.encoding;
251-
assertEncoding(encoding);
252-
253-
var flag = options.flag || 'r';
254+
fs.readFile = function(path, options, callback) {
255+
callback = maybeCallback(arguments[arguments.length - 1]);
256+
options = getOptions(options, { flag: 'r' });
254257

255258
if (!nullCheck(path, callback))
256259
return;
257260

258-
var context = new ReadFileContext(callback, encoding);
261+
var context = new ReadFileContext(callback, options.encoding);
259262
context.isUserFd = isFd(path); // file descriptor ownership
260263
var req = new FSReqWrap();
261264
req.context = context;
@@ -269,7 +272,7 @@ fs.readFile = function(path, options, callback_) {
269272
}
270273

271274
binding.open(pathModule._makeLong(path),
272-
stringToFlags(flag),
275+
stringToFlags(options.flag || 'r'),
273276
0o666,
274277
req);
275278
};
@@ -460,20 +463,9 @@ function tryReadSync(fd, isUserFd, buffer, pos, len) {
460463
}
461464

462465
fs.readFileSync = function(path, options) {
463-
if (!options) {
464-
options = { encoding: null, flag: 'r' };
465-
} else if (typeof options === 'string') {
466-
options = { encoding: options, flag: 'r' };
467-
} else if (typeof options !== 'object') {
468-
throwOptionsError(options);
469-
}
470-
471-
var encoding = options.encoding;
472-
assertEncoding(encoding);
473-
474-
var flag = options.flag || 'r';
466+
options = getOptions(options, { flag: 'r' });
475467
var isUserFd = isFd(path); // file descriptor ownership
476-
var fd = isUserFd ? path : fs.openSync(path, flag, 0o666);
468+
var fd = isUserFd ? path : fs.openSync(path, options.flag || 'r', 0o666);
477469

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

520-
if (encoding) buffer = buffer.toString(encoding);
512+
if (options.encoding) buffer = buffer.toString(options.encoding);
521513
return buffer;
522514
};
523515

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

901893
fs.readdir = function(path, options, callback) {
902-
options = options || {};
903-
if (typeof options === 'function') {
904-
callback = options;
905-
options = {};
906-
} else if (typeof options === 'string') {
907-
options = {encoding: options};
908-
}
909-
if (typeof options !== 'object')
910-
throw new TypeError('"options" must be a string or an object');
911-
912-
callback = makeCallback(callback);
894+
callback = makeCallback(typeof options === 'function' ? options : callback);
895+
options = getOptions(options, {});
913896
if (!nullCheck(path, callback)) return;
914897
var req = new FSReqWrap();
915898
req.oncomplete = callback;
916899
binding.readdir(pathModule._makeLong(path), options.encoding, req);
917900
};
918901

919902
fs.readdirSync = function(path, options) {
920-
options = options || {};
921-
if (typeof options === 'string')
922-
options = {encoding: options};
923-
if (typeof options !== 'object')
924-
throw new TypeError('"options" must be a string or an object');
903+
options = getOptions(options, {});
925904
nullCheck(path);
926905
return binding.readdir(pathModule._makeLong(path), options.encoding);
927906
};
@@ -963,28 +942,16 @@ fs.statSync = function(path) {
963942
};
964943

965944
fs.readlink = function(path, options, callback) {
966-
options = options || {};
967-
if (typeof options === 'function') {
968-
callback = options;
969-
options = {};
970-
} else if (typeof options === 'string') {
971-
options = {encoding: options};
972-
}
973-
if (typeof options !== 'object')
974-
throw new TypeError('"options" must be a string or an object');
975-
callback = makeCallback(callback);
945+
callback = makeCallback(typeof options === 'function' ? options : callback);
946+
options = getOptions(options, {});
976947
if (!nullCheck(path, callback)) return;
977948
var req = new FSReqWrap();
978949
req.oncomplete = callback;
979950
binding.readlink(pathModule._makeLong(path), options.encoding, req);
980951
};
981952

982953
fs.readlinkSync = function(path, options) {
983-
options = options || {};
984-
if (typeof options === 'string')
985-
options = {encoding: options};
986-
if (typeof options !== 'object')
987-
throw new TypeError('"options" must be a string or an object');
954+
options = getOptions(options, {});
988955
nullCheck(path);
989956
return binding.readlink(pathModule._makeLong(path), options.encoding);
990957
};
@@ -1255,20 +1222,10 @@ function writeAll(fd, isUserFd, buffer, offset, length, position, callback_) {
12551222
});
12561223
}
12571224

1258-
fs.writeFile = function(path, data, options, callback_) {
1259-
var callback = maybeCallback(arguments[arguments.length - 1]);
1260-
1261-
if (!options || typeof options === 'function') {
1262-
options = { encoding: 'utf8', mode: 0o666, flag: 'w' };
1263-
} else if (typeof options === 'string') {
1264-
options = { encoding: options, mode: 0o666, flag: 'w' };
1265-
} else if (typeof options !== 'object') {
1266-
throwOptionsError(options);
1267-
}
1268-
1269-
assertEncoding(options.encoding);
1270-
1271-
var flag = options.flag || 'w';
1225+
fs.writeFile = function(path, data, options, callback) {
1226+
callback = maybeCallback(arguments[arguments.length - 1]);
1227+
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
1228+
const flag = options.flag || 'w';
12721229

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

12951252
fs.writeFileSync = function(path, data, options) {
1296-
if (!options) {
1297-
options = { encoding: 'utf8', mode: 0o666, flag: 'w' };
1298-
} else if (typeof options === 'string') {
1299-
options = { encoding: options, mode: 0o666, flag: 'w' };
1300-
} else if (typeof options !== 'object') {
1301-
throwOptionsError(options);
1302-
}
1303-
1304-
assertEncoding(options.encoding);
1253+
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
1254+
const flag = options.flag || 'w';
13051255

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

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

1330-
fs.appendFile = function(path, data, options, callback_) {
1331-
var callback = maybeCallback(arguments[arguments.length - 1]);
1332-
1333-
if (!options || typeof options === 'function') {
1334-
options = { encoding: 'utf8', mode: 0o666, flag: 'a' };
1335-
} else if (typeof options === 'string') {
1336-
options = { encoding: options, mode: 0o666, flag: 'a' };
1337-
} else if (typeof options !== 'object') {
1338-
throwOptionsError(options);
1339-
}
1279+
fs.appendFile = function(path, data, options, callback) {
1280+
callback = maybeCallback(arguments[arguments.length - 1]);
1281+
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' });
13401282

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

13511293
fs.appendFileSync = function(path, data, options) {
1352-
if (!options) {
1353-
options = { encoding: 'utf8', mode: 0o666, flag: 'a' };
1354-
} else if (typeof options === 'string') {
1355-
options = { encoding: options, mode: 0o666, flag: 'a' };
1356-
} else if (typeof options !== 'object') {
1357-
throwOptionsError(options);
1358-
}
1294+
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' });
13591295

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

1417-
options = options || {};
14181353
if (typeof options === 'function') {
14191354
listener = options;
1420-
options = {};
1421-
} else if (typeof options === 'string') {
1422-
options = {encoding: options};
14231355
}
1424-
if (typeof options !== 'object')
1425-
throw new TypeError('"options" must be a string or an object');
1356+
options = getOptions(options, {});
14261357

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

15711502
fs.realpathSync = function realpathSync(p, options) {
1572-
if (!options)
1573-
options = {};
1574-
else if (typeof options === 'string')
1575-
options = {encoding: options};
1576-
else if (typeof options !== 'object')
1577-
throw new TypeError('"options" must be a string or an object');
1503+
options = getOptions(options, {});
15781504
nullCheck(p);
15791505

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

16761602

16771603
fs.realpath = function realpath(p, options, callback) {
1678-
if (typeof callback !== 'function') {
1679-
callback = maybeCallback(options);
1680-
options = {};
1681-
}
1682-
1683-
if (!options) {
1684-
options = {};
1685-
} else if (typeof options === 'function') {
1686-
options = {};
1687-
} else if (typeof options === 'string') {
1688-
options = {encoding: options};
1689-
} else if (typeof options !== 'object') {
1690-
throw new TypeError('"options" must be a string or an object');
1691-
}
1604+
callback = maybeCallback(typeof options === 'function' ? options : callback);
1605+
options = getOptions(options, {});
16921606
if (!nullCheck(p, callback))
16931607
return;
16941608

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

17991713
fs.mkdtemp = function(prefix, options, callback) {
1714+
callback = makeCallback(typeof options === 'function' ? options : callback);
1715+
options = getOptions(options, {});
18001716
if (!prefix || typeof prefix !== 'string')
18011717
throw new TypeError('filename prefix is required');
1802-
1803-
options = options || {};
1804-
if (typeof options === 'function') {
1805-
callback = options;
1806-
options = {};
1807-
} else if (typeof options === 'string') {
1808-
options = {encoding: options};
1809-
}
1810-
if (typeof options !== 'object')
1811-
throw new TypeError('"options" must be a string or an object');
1812-
1813-
callback = makeCallback(callback);
18141718
if (!nullCheck(prefix, callback)) {
18151719
return;
18161720
}
@@ -1825,14 +1729,8 @@ fs.mkdtemp = function(prefix, options, callback) {
18251729
fs.mkdtempSync = function(prefix, options) {
18261730
if (!prefix || typeof prefix !== 'string')
18271731
throw new TypeError('filename prefix is required');
1828-
1829-
options = options || {};
1830-
if (typeof options === 'string')
1831-
options = {encoding: options};
1832-
if (typeof options !== 'object')
1833-
throw new TypeError('"options" must be a string or an object');
1732+
options = getOptions(options, {});
18341733
nullCheck(prefix);
1835-
18361734
return binding.mkdtemp(prefix + 'XXXXXX', options.encoding);
18371735
};
18381736

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

1859-
if (options === undefined)
1860-
options = {};
1861-
else if (typeof options === 'string')
1862-
options = { encoding: options };
1863-
else if (options === null || typeof options !== 'object')
1864-
throw new TypeError('"options" argument must be a string or an object');
1865-
18661757
// a little bit bigger buffer and water marks by default
1867-
options = Object.create(options);
1758+
options = Object.create(getOptions(options, {}));
18681759
if (options.highWaterMark === undefined)
18691760
options.highWaterMark = 64 * 1024;
18701761

@@ -2030,14 +1921,7 @@ function WriteStream(path, options) {
20301921
if (!(this instanceof WriteStream))
20311922
return new WriteStream(path, options);
20321923

2033-
if (options === undefined)
2034-
options = {};
2035-
else if (typeof options === 'string')
2036-
options = { encoding: options };
2037-
else if (options === null || typeof options !== 'object')
2038-
throw new TypeError('"options" argument must be a string or an object');
2039-
2040-
options = Object.create(options);
1924+
options = Object.create(getOptions(options, {}));
20411925

20421926
Writable.call(this, options);
20431927

0 commit comments

Comments
 (0)