fs: remove fs.read's string interface

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:

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/);

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);

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));