fs: remove fs.read's string interface

It is a maintenance burden that was removed from the docs in 2010
(c93e0aaf062081db3ec40ac45b3e2c979d5759d6) and runtime-deprecated in v6.0
(1124de2d76ad7118267d91a08485aa928a5f0865).

PR-URL: https://github.com/nodejs/node/pull/9683
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

7 files changed, 57 insertions, 218 deletions

diff --git a/lib/fs.js b/lib/fs.js
index f9223f5ca8..6362fab54a 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -555,40 +555,7 @@ fs.openSync = function(path, flags, mode) {
   return binding.open(pathModule._makeLong(path), stringToFlags(flags), mode);
 };
 
-var readWarned = false;
 fs.read = function(fd, buffer, offset, length, position, callback) {
-  if (!isUint8Array(buffer)) {
-    // legacy string interface (fd, length, position, encoding, callback)
-    if (!readWarned) {
-      readWarned = true;
-      process.emitWarning(
-        'fs.read\'s legacy String interface is deprecated. Use the Buffer ' +
-        'API as mentioned in the documentation instead.',
-        'DeprecationWarning');
-    }
-
-    const cb = arguments[4];
-    const encoding = arguments[3];
-
-    assertEncoding(encoding);
-
-    position = arguments[2];
-    length = arguments[1];
-    buffer = Buffer.allocUnsafe(length);
-    offset = 0;
-
-    callback = function(err, bytesRead) {
-      if (!cb) return;
-      if (err) return cb(err);
-
-      if (bytesRead > 0) {
-        tryToStringWithEnd(buffer, encoding, bytesRead, cb);
-      } else {
-        (cb)(err, '', bytesRead);
-      }
-    };
-  }
-
   if (length === 0) {
     return process.nextTick(function() {
       callback && callback(null, 0, buffer);
@@ -606,57 +573,12 @@ fs.read = function(fd, buffer, offset, length, position, callback) {
   binding.read(fd, buffer, offset, length, position, req);
 };
 
-function tryToStringWithEnd(buf, encoding, end, callback) {
-  var e;
-  try {
-    buf = buf.toString(encoding, 0, end);
-  } catch (err) {
-    e = err;
-  }
-  callback(e, buf, end);
-}
-
-var readSyncWarned = false;
 fs.readSync = function(fd, buffer, offset, length, position) {
-  var legacy = false;
-  var encoding;
-
-  if (!isUint8Array(buffer)) {
-    // legacy string interface (fd, length, position, encoding, callback)
-    if (!readSyncWarned) {
-      readSyncWarned = true;
-      process.emitWarning(
-        'fs.readSync\'s legacy String interface is deprecated. Use the ' +
-        'Buffer API as mentioned in the documentation instead.',
-        'DeprecationWarning');
-    }
-    legacy = true;
-    encoding = arguments[3];
-
-    assertEncoding(encoding);
-
-    position = arguments[2];
-    length = arguments[1];
-    buffer = Buffer.allocUnsafe(length);
-
-    offset = 0;
-  }
-
   if (length === 0) {
-    if (legacy) {
-      return ['', 0];
-    } else {
-      return 0;
-    }
-  }
-
-  var r = binding.read(fd, buffer, offset, length, position);
-  if (!legacy) {
-    return r;
+    return 0;
   }
 
-  var str = (r > 0) ? buffer.toString(encoding, 0, r) : '';
-  return [str, r];
+  return binding.read(fd, buffer, offset, length, position);
 };
 
 // usage:
diff --git a/test/parallel/test-fs-read-buffer-tostring-fail.js b/test/parallel/test-fs-read-buffer-tostring-fail.js
deleted file mode 100644
index cce33edf4e..0000000000
--- a/test/parallel/test-fs-read-buffer-tostring-fail.js
+++ /dev/null
@@ -1,64 +0,0 @@
-'use strict';
-
-const common = require('../common');
-
-if (!common.enoughTestMem) {
-  const skipMessage = 'intensive toString tests due to memory confinements';
-  common.skip(skipMessage);
-  return;
-}
-
-const assert = require('assert');
-const fs = require('fs');
-const path = require('path');
-const Buffer = require('buffer').Buffer;
-const kStringMaxLength = process.binding('buffer').kStringMaxLength;
-
-var fd;
-
-common.refreshTmpDir();
-
-const file = path.join(common.tmpDir, 'toobig2.txt');
-const stream = fs.createWriteStream(file, {
-  flags: 'a'
-});
-
-const size = kStringMaxLength / 200;
-const a = Buffer.alloc(size, 'a');
-
-for (var i = 0; i < 201; i++) {
-  stream.write(a);
-}
-
-stream.end();
-stream.on('finish', common.mustCall(function() {
-  fd = fs.openSync(file, 'r');
-  fs.read(fd, kStringMaxLength + 1, 0, 'utf8', common.mustCall(function(err) {
-    assert.ok(err instanceof Error);
-    assert.strictEqual('"toString()" failed', err.message);
-  }));
-}));
-
-function destroy() {
-  try {
-    // Make sure we close fd and unlink the file
-    fs.closeSync(fd);
-    fs.unlinkSync(file);
-  } catch (err) {
-    // it may not exist
-  }
-}
-
-process.on('exit', destroy);
-
-process.on('SIGINT', function() {
-  destroy();
-  process.exit();
-});
-
-// To make sure we don't leave a very large file
-// on test machines in the event this test fails.
-process.on('uncaughtException', function(err) {
-  destroy();
-  throw err;
-});
diff --git a/test/parallel/test-fs-read-buffer-zero-length.js b/test/parallel/test-fs-read-buffer-zero-length.js
deleted file mode 100644
index 35b1f7b566..0000000000
--- a/test/parallel/test-fs-read-buffer-zero-length.js
+++ /dev/null
@@ -1,19 +0,0 @@
-'use strict';
-const common = require('../common');
-const assert = require('assert');
-const path = require('path');
-const Buffer = require('buffer').Buffer;
-const fs = require('fs');
-const filepath = path.join(common.fixturesDir, 'x.txt');
-const fd = fs.openSync(filepath, 'r');
-const bufferAsync = Buffer.alloc(0);
-const bufferSync = Buffer.alloc(0);
-
-fs.read(fd, bufferAsync, 0, 0, 0, common.mustCall(function(err, bytesRead) {
-  assert.equal(bytesRead, 0);
-  assert.deepStrictEqual(bufferAsync, Buffer.alloc(0));
-}));
-
-const r = fs.readSync(fd, bufferSync, 0, 0, 0);
-assert.deepStrictEqual(bufferSync, Buffer.alloc(0));
-assert.equal(r, 0);
diff --git a/test/parallel/test-fs-read-buffer.js b/test/parallel/test-fs-read-buffer.js
deleted file mode 100644
index 733be5ba0d..0000000000
--- a/test/parallel/test-fs-read-buffer.js
+++ /dev/null
@@ -1,35 +0,0 @@
-'use strict';
-const common = require('../common');
-const assert = require('assert');
-const path = require('path');
-const Buffer = require('buffer').Buffer;
-const fs = require('fs');
-const filepath = path.join(common.fixturesDir, 'x.txt');
-const fd = fs.openSync(filepath, 'r');
-
-const expected = Buffer.from('xyz\n');
-
-function test(bufferAsync, bufferSync, expected) {
-  fs.read(fd,
-          bufferAsync,
-          0,
-          expected.length,
-          0,
-          common.mustCall((err, bytesRead) => {
-            assert.ifError(err);
-            assert.strictEqual(bytesRead, expected.length);
-            assert.deepStrictEqual(bufferAsync, Buffer.from(expected));
-          }));
-
-  const r = fs.readSync(fd, bufferSync, 0, expected.length, 0);
-  assert.deepStrictEqual(bufferSync, Buffer.from(expected));
-  assert.strictEqual(r, expected.length);
-}
-
-test(Buffer.allocUnsafe(expected.length),
-     Buffer.allocUnsafe(expected.length),
-     expected);
-
-test(new Uint8Array(expected.length),
-     new Uint8Array(expected.length),
-     Uint8Array.from(expected));
diff --git a/test/parallel/test-fs-read-type.js b/test/parallel/test-fs-read-type.js
new file mode 100644
index 0000000000..2b600b0487
--- /dev/null
+++ b/test/parallel/test-fs-read-type.js
@@ -0,0 +1,21 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const path = require('path');
+const fs = require('fs');
+const filepath = path.join(common.fixturesDir, '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',
+          () => {});
+}, /Second argument needs to be a buffer/);
+
+assert.throws(() => {
+  fs.readSync(fd, expected.length, 0, 'utf-8');
+}, /Second argument needs to be a buffer/);
diff --git a/test/parallel/test-fs-read-zero-length.js b/test/parallel/test-fs-read-zero-length.js
index 9c4cde5236..35b1f7b566 100644
--- a/test/parallel/test-fs-read-zero-length.js
+++ b/test/parallel/test-fs-read-zero-length.js
@@ -2,17 +2,18 @@
 const common = require('../common');
 const assert = require('assert');
 const path = require('path');
+const Buffer = require('buffer').Buffer;
 const fs = require('fs');
 const filepath = path.join(common.fixturesDir, 'x.txt');
 const fd = fs.openSync(filepath, 'r');
-const expected = '';
+const bufferAsync = Buffer.alloc(0);
+const bufferSync = Buffer.alloc(0);
 
-fs.read(fd, 0, 0, 'utf-8', common.mustCall(function(err, str, bytesRead) {
-  assert.ok(!err);
-  assert.equal(str, expected);
+fs.read(fd, bufferAsync, 0, 0, 0, common.mustCall(function(err, bytesRead) {
   assert.equal(bytesRead, 0);
+  assert.deepStrictEqual(bufferAsync, Buffer.alloc(0));
 }));
 
-const r = fs.readSync(fd, 0, 0, 'utf-8');
-assert.equal(r[0], expected);
-assert.equal(r[1], 0);
+const r = fs.readSync(fd, bufferSync, 0, 0, 0);
+assert.deepStrictEqual(bufferSync, Buffer.alloc(0));
+assert.equal(r, 0);
diff --git a/test/parallel/test-fs-read.js b/test/parallel/test-fs-read.js
index 41957c228e..733be5ba0d 100644
--- a/test/parallel/test-fs-read.js
+++ b/test/parallel/test-fs-read.js
@@ -2,21 +2,34 @@
 const common = require('../common');
 const assert = require('assert');
 const path = require('path');
+const Buffer = require('buffer').Buffer;
 const fs = require('fs');
 const filepath = path.join(common.fixturesDir, 'x.txt');
 const fd = fs.openSync(filepath, 'r');
-const expected = 'xyz\n';
 
-fs.read(fd,
-        expected.length,
-        0,
-        'utf-8',
-        common.mustCall((err, str, bytesRead) => {
-          assert.ifError(err);
-          assert.strictEqual(str, expected);
-          assert.strictEqual(bytesRead, expected.length);
-        }));
+const expected = Buffer.from('xyz\n');
 
-var r = fs.readSync(fd, expected.length, 0, 'utf-8');
-assert.strictEqual(r[0], expected);
-assert.strictEqual(r[1], expected.length);
+function test(bufferAsync, bufferSync, expected) {
+  fs.read(fd,
+          bufferAsync,
+          0,
+          expected.length,
+          0,
+          common.mustCall((err, bytesRead) => {
+            assert.ifError(err);
+            assert.strictEqual(bytesRead, expected.length);
+            assert.deepStrictEqual(bufferAsync, Buffer.from(expected));
+          }));
+
+  const r = fs.readSync(fd, bufferSync, 0, expected.length, 0);
+  assert.deepStrictEqual(bufferSync, Buffer.from(expected));
+  assert.strictEqual(r, expected.length);
+}
+
+test(Buffer.allocUnsafe(expected.length),
+     Buffer.allocUnsafe(expected.length),
+     expected);
+
+test(new Uint8Array(expected.length),
+     new Uint8Array(expected.length),
+     Uint8Array.from(expected));