fs: default open/openSync flags argument to 'r'

Make fs.open() and fs.openSync() more economic to use by making the flags argument optional. You can now write: fs.open(file, cb) Instead of the more verbose: fs.open(file, 'r', cb) This idiom is already supported by functions like fs.readFile(). PR-URL: #23767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
nodejs · Oct 26, 2018 · 482b56a · 482b56a
1 parent 6223236
commit 482b56a
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 12 deletions.
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -2309,7 +2309,7 @@ this API: [`fs.mkdtemp()`][].
 The optional `options` argument can be a string specifying an encoding, or an
 object with an `encoding` property specifying the character encoding to use.
 
-## fs.open(path, flags[, mode], callback)
+## fs.open(path[, flags[, mode]], callback)
 <!-- YAML
 added: v0.0.2
 changes:
@@ -2324,6 +2324,7 @@ changes:
 
 * `path` {string|Buffer|URL}
 * `flags` {string|number} See [support of file system `flags`][].
+  **Default:** `'r'`.
 * `mode` {integer} **Default:** `0o666` (readable and writable)
 * `callback` {Function}
   * `err` {Error}
@@ -2345,7 +2346,7 @@ a colon, Node.js will open a file system stream, as described by
 Functions based on `fs.open()` exhibit this behavior as well:
 `fs.writeFile()`, `fs.readFile()`, etc.
 
-## fs.openSync(path, flags[, mode])
+## fs.openSync(path[, flags, mode])
 <!-- YAML
 added: v0.1.21
 changes:
@@ -2356,7 +2357,8 @@ changes:
 -->
 
 * `path` {string|Buffer|URL}
-* `flags` {string|number} See [support of file system `flags`][].
+* `flags` {string|number} **Default:** `'r'`.
+   See [support of file system `flags`][].
 * `mode` {integer} **Default:** `0o666`
 * Returns: {number}
 

diff --git a/lib/fs.js b/lib/fs.js
@@ -345,7 +345,7 @@ function tryReadSync(fd, isUserFd, buffer, pos, len) {
 function readFileSync(path, options) {
   options = getOptions(options, { flag: 'r' });
   const isUserFd = isFd(path); // file descriptor ownership
-  const fd = isUserFd ? path : fs.openSync(path, options.flag || 'r', 0o666);
+  const fd = isUserFd ? path : fs.openSync(path, options.flag, 0o666);
 
   const stats = tryStatSync(fd, isUserFd);
   const size = isFileType(stats, S_IFREG) ? stats[8] : 0;
@@ -411,14 +411,19 @@ function closeSync(fd) {
 function open(path, flags, mode, callback) {
   path = toPathIfFileURL(path);
   validatePath(path);
-  const flagsNumber = stringToFlags(flags);
-  if (arguments.length < 4) {
-    callback = makeCallback(mode);
+  if (arguments.length < 3) {
+    callback = flags;
+    flags = 'r';
     mode = 0o666;
-  } else {
+  } else if (arguments.length === 3) {
+    callback = mode;
+    mode = 0o666;
+  }
+  const flagsNumber = stringToFlags(flags);
+  if (arguments.length >= 4) {
     mode = validateMode(mode, 'mode', 0o666);
-    callback = makeCallback(callback);
   }
+  callback = makeCallback(callback);
 
   const req = new FSReqCallback();
   req.oncomplete = callback;
@@ -433,7 +438,7 @@ function open(path, flags, mode, callback) {
 function openSync(path, flags, mode) {
   path = toPathIfFileURL(path);
   validatePath(path);
-  const flagsNumber = stringToFlags(flags);
+  const flagsNumber = stringToFlags(flags || 'r');
   mode = validateMode(mode, 'mode', 0o666);
 
   const ctx = { path };

diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
@@ -196,11 +196,12 @@ async function copyFile(src, dest, flags) {
 async function open(path, flags, mode) {
   path = toPathIfFileURL(path);
   validatePath(path);
+  if (arguments.length < 2) flags = 'r';
+  const flagsNumber = stringToFlags(flags);
   mode = validateMode(mode, 'mode', 0o666);
   return new FileHandle(
     await binding.openFileHandle(pathModule.toNamespacedPath(path),
-                                 stringToFlags(flags),
-                                 mode, kUsePromises));
+                                 flagsNumber, mode, kUsePromises));
 }
 
 async function read(handle, buffer, offset, length, position) {

diff --git a/test/parallel/test-fs-open.js b/test/parallel/test-fs-open.js
@@ -36,6 +36,12 @@ try {
 }
 assert.strictEqual(caughtException, true);
 
+fs.openSync(__filename);
+
+fs.open(__filename, common.mustCall((err) => {
+  assert.ifError(err);
+}));
+
 fs.open(__filename, 'r', common.mustCall((err) => {
   assert.ifError(err);
 }));
@@ -44,6 +50,39 @@ fs.open(__filename, 'rs', common.mustCall((err) => {
   assert.ifError(err);
 }));
 
+fs.open(__filename, 'r', 0, common.mustCall((err) => {
+  assert.ifError(err);
+}));
+
+fs.open(__filename, 'r', null, common.mustCall((err) => {
+  assert.ifError(err);
+}));
+
+async function promise() {
+  await fs.promises.open(__filename);
+  await fs.promises.open(__filename, 'r');
+}
+
+promise().then(common.mustCall()).catch(common.mustNotCall());
+
+common.expectsError(
+  () => fs.open(__filename, 'r', 'boom', common.mustNotCall()),
+  {
+    code: 'ERR_INVALID_ARG_VALUE',
+    type: TypeError
+  }
+);
+
+for (const extra of [[], ['r'], ['r', 0], ['r', 0, 'bad callback']]) {
+  common.expectsError(
+    () => fs.open(__filename, ...extra),
+    {
+      code: 'ERR_INVALID_CALLBACK',
+      type: TypeError
+    }
+  );
+}
+
 [false, 1, [], {}, null, undefined].forEach((i) => {
   common.expectsError(
     () => fs.open(i, 'r', common.mustNotCall()),
@@ -59,4 +98,15 @@ fs.open(__filename, 'rs', common.mustCall((err) => {
       type: TypeError
     }
   );
+  fs.promises.open(i, 'r')
+    .then(common.mustNotCall())
+    .catch(common.mustCall((err) => {
+      common.expectsError(
+        () => { throw err; },
+        {
+          code: 'ERR_INVALID_ARG_TYPE',
+          type: TypeError
+        }
+      );
+    }));
 });