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]>
Signed-off-by: Beth Griggs <[email protected]>

Author: bnoordhuis

doc/api/fs.md

@@ -2304,7 +2304,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:
@@ -2322,6 +2322,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}
@@ -2343,7 +2344,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:
@@ -2360,7 +2361,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}
 

lib/fs.js

@@ -340,7 +340,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;
@@ -406,14 +406,19 @@ function closeSync(fd) {
 function open(path, flags, mode, callback) {
   path = toPathIfFileURL(path);
   validatePath(path);
-  const flagsNumber = stringToFlags(flags);
-  if (typeof mode === 'function') {
-    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 FSReqWrap();
   req.oncomplete = callback;
@@ -428,7 +433,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 };

lib/internal/fs/promises.js

@@ -195,11 +195,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) {

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
+        }
+      );
+    }));
 });