Skip to content

Commit

Permalink
fs: default open/openSync flags argument to 'r'
Browse files Browse the repository at this point in the history
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 targos committed Nov 1, 2018
1 parent 0312d8b commit b4b101f
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 12 deletions.
8 changes: 5 additions & 3 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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}
Expand All @@ -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:
Expand All @@ -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}

Expand Down
19 changes: 12 additions & 7 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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 };
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
50 changes: 50 additions & 0 deletions test/parallel/test-fs-open.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}));
Expand All @@ -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()),
Expand All @@ -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 b4b101f

Please sign in to comment.