Skip to content
Permalink
Browse files

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 <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
  • Loading branch information...
bnoordhuis authored and Trott committed Oct 23, 2018
1 parent 6223236 commit 482b56ae60bb14588ee57dd92b48158d5f7c2f23
Showing with 70 additions and 12 deletions.
  1. +5 −3 doc/api/fs.md
  2. +12 −7 lib/fs.js
  3. +3 −2 lib/internal/fs/promises.js
  4. +50 −0 test/parallel/test-fs-open.js
@@ -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}

@@ -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 };
@@ -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) {
@@ -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
}
);
}));
});

0 comments on commit 482b56a

Please sign in to comment.
You can’t perform that action at this time.