fs: change default value of position in read and readSync

doc/api/fs.md

@@ -2704,6 +2704,9 @@ directory and subsequent read operations.
 <!-- YAML
 added: v0.0.2
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/37101
+    description: Runtime description.
   - version: v10.10.0
     pr-url: https://github.com/nodejs/node/pull/22150
     description: The `buffer` parameter can now be any `TypedArray`, or a
@@ -2722,8 +2725,8 @@ changes:
 * `offset` {integer} The position in `buffer` to write the data to.
 * `length` {integer}  The number of bytes to read.
 * `position` {integer|bigint} Specifies where to begin reading from in the
-  file. If `position` is `null` or `-1 `, data will be read from the current
-  file position, and the file position will be updated. If `position` is an
+  file. If `position` is `-1 `, data will be read from the current file
+  position, and the file position will be updated. If `position` is any other
   integer, the file position will be unchanged.
 * `callback` {Function}
   * `err` {Error}
@@ -2746,6 +2749,9 @@ added:
  - v13.11.0
  - v12.17.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/37101
+    description: Runtime description.
   - version:
      - v13.11.0
      - v12.17.0
@@ -2758,7 +2764,7 @@ changes:
   * `buffer` {Buffer|TypedArray|DataView} **Default:** `Buffer.alloc(16384)`
   * `offset` {integer} **Default:** `0`
   * `length` {integer} **Default:** `buffer.length`
-  * `position` {integer|bigint} **Default:** `null`
+  * `position` {integer|bigint} **Default:** `-1`
 * `callback` {Function}
   * `err` {Error}
   * `bytesRead` {integer}

lib/fs.js

@@ -512,7 +512,7 @@ function openSync(path, flags, mode) {
 // fs.read(fd, buffer, offset, length, position, callback);
 // OR
 // fs.read(fd, {}, callback)
-function read(fd, buffer, offset, length, position, callback) {
+function read(fd, buffer, offset, length, position = -1, callback) {
   fd = getValidatedFd(fd);
 
   if (arguments.length <= 3) {
@@ -534,7 +534,7 @@ function read(fd, buffer, offset, length, position, callback) {
       buffer = Buffer.alloc(16384),
       offset = 0,
       length = buffer.length,
-      position
+      position = -1,
     } = options);
   }
 
@@ -562,8 +562,13 @@ function read(fd, buffer, offset, length, position, callback) {
 
   validateOffsetLengthRead(offset, length, buffer.byteLength);
 
-  if (position == null)
+  if (position === null) {
+    process.emitWarning(
+      `The provided ${position} is not a valid position, and is supported ` +
+      'in the fs module solely for compatibility.',
+      'DeprecationWarning', 'DEP01149');
     position = -1;
+  }
 
   validatePosition(position, 'position');
 
@@ -585,14 +590,18 @@ ObjectDefineProperty(read, internalUtil.customPromisifyArgs,
 // fs.readSync(fd, buffer, offset, length, position);
 // OR
 // fs.readSync(fd, buffer, {}) or fs.readSync(fd, buffer)
-function readSync(fd, buffer, offset, length, position) {
+function readSync(fd, buffer, offset, length, position = -1) {
   fd = getValidatedFd(fd);
 
   if (arguments.length <= 3) {
     // Assume fs.read(fd, buffer, options)
     const options = offset || {};
 
-    ({ offset = 0, length = buffer.length, position } = options);
+    ({
+      offset = 0,
+      length = buffer.length,
+      position = -1,
+    } = options);
   }
 
   validateBuffer(buffer);
@@ -616,8 +625,13 @@ function readSync(fd, buffer, offset, length, position) {
 
   validateOffsetLengthRead(offset, length, buffer.byteLength);
 
-  if (position == null)
+  if (position === null) {
+    process.emitWarning(
+      `The provided ${position} is not a valid position, and is supported ` +
+      'in the fs module solely for compatibility.',
+      'DeprecationWarning', 'DEP0149');
     position = -1;
+  }
 
   validatePosition(position, 'position');
 

test/parallel/test-fs-promisified.js

@@ -11,7 +11,7 @@ const exists = promisify(fs.exists);
 
 {
   const fd = fs.openSync(__filename, 'r');
-  read(fd, Buffer.alloc(1024), 0, 1024, null).then(common.mustCall((obj) => {
+  read(fd, Buffer.alloc(1024), 0, 1024, -1).then(common.mustCall((obj) => {
     assert.strictEqual(typeof obj.bytesRead, 'number');
     assert(obj.buffer instanceof Buffer);
     fs.closeSync(fd);

test/parallel/test-fs-read-type.js

@@ -8,7 +8,6 @@ const filepath = fixtures.path('x.txt');
 const fd = fs.openSync(filepath, 'r');
 const expected = 'xyz\n';
 
-
 // Error must be thrown with string
 assert.throws(
   () => fs.read(fd, expected.length, 0, 'utf-8', common.mustNotCall()),
@@ -90,6 +89,21 @@ assert.throws(() => {
   });
 });
 
+{
+  fs.read(fd,
+          Buffer.allocUnsafe(expected.length),
+          0,
+          expected.length,
+          null,
+          common.mustCall());
+  common.expectWarning(
+    'DeprecationWarning',
+    'The provided null is not a valid position, and is supported ' +
+    'in the fs module solely for compatibility.',
+    'DEP01149',
+  );
+}
+
 [0.5, 2 ** 53, 2n ** 63n].forEach((value) => {
   assert.throws(() => {
     fs.read(fd,
@@ -210,6 +224,20 @@ assert.throws(() => {
   });
 });
 
+{
+  fs.readSync(fd,
+              Buffer.allocUnsafe(expected.length),
+              0,
+              expected.length,
+              null);
+  common.expectWarning(
+    'DeprecationWarning',
+    'The provided null is not a valid position, and is supported ' +
+    'in the fs module solely for compatibility.',
+    'DEP01149',
+  );
+}
+
 [0.5, 2 ** 53, 2n ** 63n].forEach((value) => {
   assert.throws(() => {
     fs.readSync(fd,

test/parallel/test-fs-readfile-fd.js

@@ -79,7 +79,7 @@ function tempFdSync(callback) {
       const buf = Buffer.alloc(5);
 
       // Read only five bytes, so that the position moves to five.
-      fs.read(fd, buf, 0, 5, null, common.mustSucceed((bytes) => {
+      fs.read(fd, buf, 0, 5, -1, common.mustSucceed((bytes) => {
         assert.strictEqual(bytes, 5);
         assert.deepStrictEqual(buf.toString(), 'Hello');