fs: Fix default params for fs.write(Sync)

Add support for fs.write(fd, buffer, cb) and fs.write(fd, buffer, offset, cb)
as documented at
https://nodejs.org/api/fs.html#fs_fs_write_fd_data_position_encoding_callback
and equivalently for fs.writeSync

Update docs and code comments to reflect the implementation.

PR-URL: #7856
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Yorkie Liu <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>

Author: papandreou

doc/api/fs.md

@@ -1816,13 +1816,13 @@ _Note: [`fs.watch()`][] is more efficient than `fs.watchFile` and
 `fs.unwatchFile`. `fs.watch` should be used instead of `fs.watchFile` and
 `fs.unwatchFile` when possible._
 
-## fs.write(fd, buffer, offset, length[, position], callback)
+## fs.write(fd, buffer[, offset[, length[, position]]], callback)
 <!-- YAML
 added: v0.0.2
 -->
 
 * `fd` {Integer}
-* `buffer` {String | Buffer}
+* `buffer` {Buffer}
 * `offset` {Integer}
 * `length` {Integer}
 * `position` {Integer}
@@ -1847,19 +1847,19 @@ On Linux, positional writes don't work when the file is opened in append mode.
 The kernel ignores the position argument and always appends the data to
 the end of the file.
 
-## fs.write(fd, data[, position[, encoding]], callback)
+## fs.write(fd, string[, position[, encoding]], callback)
 <!-- YAML
 added: v0.11.5
 -->
 
 * `fd` {Integer}
-* `data` {String | Buffer}
+* `string` {String}
 * `position` {Integer}
 * `encoding` {String}
 * `callback` {Function}
 
-Write `data` to the file specified by `fd`.  If `data` is not a Buffer instance
-then the value will be coerced to a string.
+Write `string` to the file specified by `fd`.  If `string` is not a string, then
+the value will be coerced to one.
 
 `position` refers to the offset from the beginning of the file where this data
 should be written. If `typeof position !== 'number'` the data will be written at
@@ -1940,24 +1940,24 @@ added: v0.1.29
 
 The synchronous version of [`fs.writeFile()`][]. Returns `undefined`.
 
-## fs.writeSync(fd, buffer, offset, length[, position])
+## fs.writeSync(fd, buffer[, offset[, length[, position]]])
 <!-- YAML
 added: v0.1.21
 -->
 
 * `fd` {Integer}
-* `buffer` {String | Buffer}
+* `buffer` {Buffer}
 * `offset` {Integer}
 * `length` {Integer}
 * `position` {Integer}
 
-## fs.writeSync(fd, data[, position[, encoding]])
+## fs.writeSync(fd, string[, position[, encoding]])
 <!-- YAML
 added: v0.11.5
 -->
 
 * `fd` {Integer}
-* `data` {String | Buffer}
+* `string` {String}
 * `position` {Integer}
 * `encoding` {String}
 

lib/fs.js

@@ -662,7 +662,7 @@ fs.readSync = function(fd, buffer, offset, length, position) {
 };
 
 // usage:
-//  fs.write(fd, buffer, offset, length[, position], callback);
+//  fs.write(fd, buffer[, offset[, length[, position]]], callback);
 // OR
 //  fs.write(fd, string[, position[, encoding]], callback);
 fs.write = function(fd, buffer, offset, length, position, callback) {
@@ -675,12 +675,16 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
   req.oncomplete = wrapper;
 
   if (buffer instanceof Buffer) {
-    // if no position is passed then assume null
-    if (typeof position === 'function') {
-      callback = position;
+    callback = maybeCallback(callback || position || length || offset);
+    if (typeof offset !== 'number') {
+      offset = 0;
+    }
+    if (typeof length !== 'number') {
+      length = buffer.length - offset;
+    }
+    if (typeof position !== 'number') {
       position = null;
     }
-    callback = maybeCallback(callback);
     return binding.writeBuffer(fd, buffer, offset, length, position, req);
   }
 
@@ -700,13 +704,17 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
 };
 
 // usage:
-//  fs.writeSync(fd, buffer, offset, length[, position]);
+//  fs.writeSync(fd, buffer[, offset[, length[, position]]]);
 // OR
 //  fs.writeSync(fd, string[, position[, encoding]]);
 fs.writeSync = function(fd, buffer, offset, length, position) {
   if (buffer instanceof Buffer) {
     if (position === undefined)
       position = null;
+    if (typeof offset !== 'number')
+      offset = 0;
+    if (typeof length !== 'number')
+      length = buffer.length - offset;
     return binding.writeBuffer(fd, buffer, offset, length, position);
   }
   if (typeof buffer !== 'string')

test/parallel/test-fs-write-buffer.js

@@ -2,29 +2,107 @@
 const common = require('../common');
 const assert = require('assert');
 const path = require('path');
-const Buffer = require('buffer').Buffer;
 const fs = require('fs');
-const filename = path.join(common.tmpDir, 'write.txt');
 const expected = Buffer.from('hello');
 
 common.refreshTmpDir();
 
-fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) {
-  if (err) throw err;
-
-  fs.write(fd,
-           expected,
-           0,
-           expected.length,
-           null,
-           common.mustCall(function(err, written) {
-             if (err) throw err;
-
-             assert.equal(expected.length, written);
-             fs.closeSync(fd);
-
-             var found = fs.readFileSync(filename, 'utf8');
-             assert.deepStrictEqual(expected.toString(), found);
-             fs.unlinkSync(filename);
-           }));
-}));
+// fs.write with all parameters provided:
+{
+  const filename = path.join(common.tmpDir, 'write1.txt');
+  fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) {
+    assert.ifError(err);
+
+    const cb = common.mustCall(function(err, written) {
+      assert.ifError(err);
+
+      assert.strictEqual(expected.length, written);
+      fs.closeSync(fd);
+
+      var found = fs.readFileSync(filename, 'utf8');
+      assert.deepStrictEqual(expected.toString(), found);
+    });
+
+    fs.write(fd, expected, 0, expected.length, null, cb);
+  }));
+}
+
+// fs.write with a buffer, without the length parameter:
+{
+  const filename = path.join(common.tmpDir, 'write2.txt');
+  fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) {
+    assert.ifError(err);
+
+    const cb = common.mustCall(function(err, written) {
+      assert.ifError(err);
+
+      assert.strictEqual(2, written);
+      fs.closeSync(fd);
+
+      const found = fs.readFileSync(filename, 'utf8');
+      assert.deepStrictEqual('lo', found);
+    });
+
+    fs.write(fd, Buffer.from('hello'), 3, cb);
+  }));
+}
+
+// fs.write with a buffer, without the offset and length parameters:
+{
+  const filename = path.join(common.tmpDir, 'write3.txt');
+  fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) {
+    assert.ifError(err);
+
+    const cb = common.mustCall(function(err, written) {
+      assert.ifError(err);
+
+      assert.strictEqual(expected.length, written);
+      fs.closeSync(fd);
+
+      const found = fs.readFileSync(filename, 'utf8');
+      assert.deepStrictEqual(expected.toString(), found);
+    });
+
+    fs.write(fd, expected, cb);
+  }));
+}
+
+// fs.write with the offset passed as undefined followed by the callback:
+{
+  const filename = path.join(common.tmpDir, 'write4.txt');
+  fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) {
+    assert.ifError(err);
+
+    const cb = common.mustCall(function(err, written) {
+      assert.ifError(err);
+
+      assert.strictEqual(expected.length, written);
+      fs.closeSync(fd);
+
+      const found = fs.readFileSync(filename, 'utf8');
+      assert.deepStrictEqual(expected.toString(), found);
+    });
+
+    fs.write(fd, expected, undefined, cb);
+  }));
+}
+
+// fs.write with offset and length passed as undefined followed by the callback:
+{
+  const filename = path.join(common.tmpDir, 'write5.txt');
+  fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) {
+    assert.ifError(err);
+
+    const cb = common.mustCall(function(err, written) {
+      assert.ifError(err);
+
+      assert.strictEqual(expected.length, written);
+      fs.closeSync(fd);
+
+      const found = fs.readFileSync(filename, 'utf8');
+      assert.deepStrictEqual(expected.toString(), found);
+    });
+
+    fs.write(fd, expected, undefined, undefined, cb);
+  }));
+}

test/parallel/test-fs-write-sync.js

@@ -1,23 +1,56 @@
 'use strict';
-var common = require('../common');
-var assert = require('assert');
-var path = require('path');
-var fs = require('fs');
-var fn = path.join(common.tmpDir, 'write.txt');
+const common = require('../common');
+const assert = require('assert');
+const path = require('path');
+const fs = require('fs');
+const filename = path.join(common.tmpDir, 'write.txt');
 
 common.refreshTmpDir();
 
-var foo = 'foo';
-var fd = fs.openSync(fn, 'w');
+// fs.writeSync with all parameters provided:
+{
+  const fd = fs.openSync(filename, 'w');
 
-var written = fs.writeSync(fd, '');
-assert.strictEqual(0, written);
+  let written = fs.writeSync(fd, '');
+  assert.strictEqual(0, written);
 
-fs.writeSync(fd, foo);
+  fs.writeSync(fd, 'foo');
 
-var bar = 'bár';
-written = fs.writeSync(fd, Buffer.from(bar), 0, Buffer.byteLength(bar));
-assert.ok(written > 3);
-fs.closeSync(fd);
+  written = fs.writeSync(fd, Buffer.from('bár'), 0, Buffer.byteLength('bár'));
+  assert.ok(written > 3);
+  fs.closeSync(fd);
 
-assert.equal(fs.readFileSync(fn), 'foobár');
+  assert.strictEqual(fs.readFileSync(filename, 'utf-8'), 'foobár');
+}
+
+// fs.writeSync with a buffer, without the length parameter:
+{
+  const fd = fs.openSync(filename, 'w');
+
+  let written = fs.writeSync(fd, '');
+  assert.strictEqual(0, written);
+
+  fs.writeSync(fd, 'foo');
+
+  written = fs.writeSync(fd, Buffer.from('bár'), 0);
+  assert.ok(written > 3);
+  fs.closeSync(fd);
+
+  assert.strictEqual(fs.readFileSync(filename, 'utf-8'), 'foobár');
+}
+
+// fs.writeSync with a buffer, without the offset and length parameters:
+{
+  const fd = fs.openSync(filename, 'w');
+
+  let written = fs.writeSync(fd, '');
+  assert.strictEqual(0, written);
+
+  fs.writeSync(fd, 'foo');
+
+  written = fs.writeSync(fd, Buffer.from('bár'));
+  assert.ok(written > 3);
+  fs.closeSync(fd);
+
+  assert.strictEqual(fs.readFileSync(filename, 'utf-8'), 'foobár');
+}