From ad97314418286b66a3c714b47559a7437b7cba85 Mon Sep 17 00:00:00 2001
From: Bryan English <bryan@bryanenglish.com>
Date: Fri, 27 Jul 2018 19:29:32 -0700
Subject: [PATCH] fs: readdir optionally returning type information

readdir and readdirSync now have a "withFileTypes" option, which, when
enabled, provides an array of DirectoryEntry objects, similar to Stats
objects, which have the filename and the type information.

Refs: https://github.com/nodejs/node/issues/15699

PR-URL: https://github.com/nodejs/node/pull/22020
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
---
 doc/api/fs.md                          | 101 +++++++++++-
 lib/fs.js                              |  23 ++-
 lib/internal/fs/promises.js            |  12 +-
 lib/internal/fs/utils.js               | 117 ++++++++++++-
 src/node_constants.cc                  |  10 ++
 src/node_file.cc                       | 217 ++++++++++++++++++++-----
 test/parallel/test-fs-readdir-types.js |  95 +++++++++++
 tools/doc/type-parser.js               |   1 +
 8 files changed, 524 insertions(+), 52 deletions(-)
 create mode 100644 test/parallel/test-fs-readdir-types.js

diff --git a/doc/api/fs.md b/doc/api/fs.md
index 57aabee7035fe7..4c00885117e4ca 100644
--- a/doc/api/fs.md
+++ b/doc/api/fs.md
@@ -283,6 +283,92 @@ synchronous use libuv's threadpool, which can have surprising and negative
 performance implications for some applications. See the
 [`UV_THREADPOOL_SIZE`][] documentation for more information.
 
+## Class: fs.Dirent
+<!-- YAML
+added: REPLACEME
+-->
+
+When [`fs.readdir()`][] or [`fs.readdirSync()`][] is called with the
+`withFileTypes` option set to `true`, the resulting array is filled with
+`fs.Dirent` objects, rather than strings or `Buffers`.
+
+### dirent.isBlockDevice()
+<!-- YAML
+added: REPLACEME
+-->
+
+* Returns: {boolean}
+
+Returns `true` if the `fs.Dirent` object describes a block device.
+
+### dirent.isCharacterDevice()
+<!-- YAML
+added: REPLACEME
+-->
+
+* Returns: {boolean}
+
+Returns `true` if the `fs.Dirent` object describes a character device.
+
+### dirent.isDirectory()
+<!-- YAML
+added: REPLACEME
+-->
+
+* Returns: {boolean}
+
+Returns `true` if the `fs.Dirent` object describes a file system
+directory.
+
+### dirent.isFIFO()
+<!-- YAML
+added: REPLACEME
+-->
+
+* Returns: {boolean}
+
+Returns `true` if the `fs.Dirent` object describes a first-in-first-out
+(FIFO) pipe.
+
+### dirent.isFile()
+<!-- YAML
+added: REPLACEME
+-->
+
+* Returns: {boolean}
+
+Returns `true` if the `fs.Dirent` object describes a regular file.
+
+### dirent.isSocket()
+<!-- YAML
+added: REPLACEME
+-->
+
+* Returns: {boolean}
+
+Returns `true` if the `fs.Dirent` object describes a socket.
+
+### dirent.isSymbolicLink()
+<!-- YAML
+added: REPLACEME
+-->
+
+* Returns: {boolean}
+
+Returns `true` if the `fs.Dirent` object describes a symbolic link.
+
+
+### dirent.name
+<!-- YAML
+added: REPLACEME
+-->
+
+* {string|Buffer}
+
+The file name that this `fs.Dirent` object refers to. The type of this
+value is determined by the `options.encoding` passed to [`fs.readdir()`][] or
+[`fs.readdirSync()`][].
+
 ## Class: fs.FSWatcher
 <!-- YAML
 added: v0.5.8
@@ -2299,9 +2385,10 @@ changes:
 * `path` {string|Buffer|URL}
 * `options` {string|Object}
   * `encoding` {string} **Default:** `'utf8'`
+  * `withFileTypes` {boolean} **Default:** `false`
 * `callback` {Function}
   * `err` {Error}
-  * `files` {string[]|Buffer[]}
+  * `files` {string[]|Buffer[]|fs.Dirent[]}
 
 Asynchronous readdir(3). Reads the contents of a directory.
 The callback gets two arguments `(err, files)` where `files` is an array of
@@ -2312,6 +2399,9 @@ object with an `encoding` property specifying the character encoding to use for
 the filenames passed to the callback. If the `encoding` is set to `'buffer'`,
 the filenames returned will be passed as `Buffer` objects.
 
+If `options.withFileTypes` is set to `true`, the `files` array will contain
+[`fs.Dirent`][] objects.
+
 ## fs.readdirSync(path[, options])
 <!-- YAML
 added: v0.1.21
@@ -2325,7 +2415,8 @@ changes:
 * `path` {string|Buffer|URL}
 * `options` {string|Object}
   * `encoding` {string} **Default:** `'utf8'`
-* Returns: {string[]} An array of filenames excluding `'.'` and `'..'`.
+  * `withFileTypes` {boolean} **Default:** `false`
+* Returns: {string[]|Buffer[]|fs.Dirent[]}
 
 Synchronous readdir(3).
 
@@ -2334,6 +2425,9 @@ object with an `encoding` property specifying the character encoding to use for
 the filenames returned. If the `encoding` is set to `'buffer'`,
 the filenames returned will be passed as `Buffer` objects.
 
+If `options.withFileTypes` is set to `true`, the result will contain
+[`fs.Dirent`][] objects.
+
 ## fs.readFile(path[, options], callback)
 <!-- YAML
 added: v0.1.29
@@ -4611,6 +4705,7 @@ the file contents.
 [`WriteStream`]: #fs_class_fs_writestream
 [`EventEmitter`]: events.html
 [`event ports`]: http://illumos.org/man/port_create
+[`fs.Dirent`]: #fs_class_fs_dirent
 [`fs.FSWatcher`]: #fs_class_fs_fswatcher
 [`fs.Stats`]: #fs_class_fs_stats
 [`fs.access()`]: #fs_fs_access_path_mode_callback
@@ -4626,6 +4721,8 @@ the file contents.
 [`fs.mkdtemp()`]: #fs_fs_mkdtemp_prefix_options_callback
 [`fs.open()`]: #fs_fs_open_path_flags_mode_callback
 [`fs.read()`]: #fs_fs_read_fd_buffer_offset_length_position_callback
+[`fs.readdir()`]: #fs_fs_readdir_path_options_callback
+[`fs.readdirSync()`]: #fs_fs_readdirsync_path_options
 [`fs.readFile()`]: #fs_fs_readfile_path_options_callback
 [`fs.readFileSync()`]: #fs_fs_readfilesync_path_options
 [`fs.realpath()`]: #fs_fs_realpath_path_options_callback
diff --git a/lib/fs.js b/lib/fs.js
index e0781161dfccbb..4a9476049d03b7 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -58,6 +58,8 @@ const { getPathFromURL } = require('internal/url');
 const internalUtil = require('internal/util');
 const {
   copyObject,
+  Dirent,
+  getDirents,
   getOptions,
   nullCheck,
   preprocessSymlinkDestination,
@@ -736,8 +738,19 @@ function readdir(path, options, callback) {
   validatePath(path);
 
   const req = new FSReqWrap();
-  req.oncomplete = callback;
-  binding.readdir(pathModule.toNamespacedPath(path), options.encoding, req);
+  if (!options.withFileTypes) {
+    req.oncomplete = callback;
+  } else {
+    req.oncomplete = (err, result) => {
+      if (err) {
+        callback(err);
+        return;
+      }
+      getDirents(path, result, callback);
+    };
+  }
+  binding.readdir(pathModule.toNamespacedPath(path), options.encoding,
+                  !!options.withFileTypes, req);
 }
 
 function readdirSync(path, options) {
@@ -746,9 +759,10 @@ function readdirSync(path, options) {
   validatePath(path);
   const ctx = { path };
   const result = binding.readdir(pathModule.toNamespacedPath(path),
-                                 options.encoding, undefined, ctx);
+                                 options.encoding, !!options.withFileTypes,
+                                 undefined, ctx);
   handleErrorFromBinding(ctx);
-  return result;
+  return options.withFileTypes ? getDirents(path, result) : result;
 }
 
 function fstat(fd, options, callback) {
@@ -1773,6 +1787,7 @@ module.exports = fs = {
   writeFileSync,
   write,
   writeSync,
+  Dirent,
   Stats,
 
   // Stream constructors
diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
index a8d94a739e2bee..f9750aa55a8b11 100644
--- a/lib/internal/fs/promises.js
+++ b/lib/internal/fs/promises.js
@@ -18,6 +18,7 @@ const { getPathFromURL } = require('internal/url');
 const { isUint8Array } = require('internal/util/types');
 const {
   copyObject,
+  getDirents,
   getOptions,
   getStatsFromBinding,
   nullCheck,
@@ -36,10 +37,13 @@ const {
   validateUint32
 } = require('internal/validators');
 const pathModule = require('path');
+const { promisify } = require('internal/util');
 
 const kHandle = Symbol('handle');
 const { kUsePromises } = binding;
 
+const getDirectoryEntriesPromise = promisify(getDirents);
+
 class FileHandle {
   constructor(filehandle) {
     this[kHandle] = filehandle;
@@ -294,8 +298,12 @@ async function readdir(path, options) {
   options = getOptions(options, {});
   path = getPathFromURL(path);
   validatePath(path);
-  return binding.readdir(pathModule.toNamespacedPath(path),
-                         options.encoding, kUsePromises);
+  const result = await binding.readdir(pathModule.toNamespacedPath(path),
+                                       options.encoding, !!options.withTypes,
+                                       kUsePromises);
+  return options.withFileTypes ?
+    getDirectoryEntriesPromise(path, result) :
+    result;
 }
 
 async function readlink(path, options) {
diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js
index 7aa19605e92a6a..befb06047312fb 100644
--- a/lib/internal/fs/utils.js
+++ b/lib/internal/fs/utils.js
@@ -12,6 +12,8 @@ const {
 const { isUint8Array } = require('internal/util/types');
 const pathModule = require('path');
 const util = require('util');
+const kType = Symbol('type');
+const kStats = Symbol('stats');
 
 const {
   O_APPEND,
@@ -31,17 +33,84 @@ const {
   S_IFREG,
   S_IFSOCK,
   UV_FS_SYMLINK_DIR,
-  UV_FS_SYMLINK_JUNCTION
+  UV_FS_SYMLINK_JUNCTION,
+  UV_DIRENT_UNKNOWN,
+  UV_DIRENT_FILE,
+  UV_DIRENT_DIR,
+  UV_DIRENT_LINK,
+  UV_DIRENT_FIFO,
+  UV_DIRENT_SOCKET,
+  UV_DIRENT_CHAR,
+  UV_DIRENT_BLOCK
 } = process.binding('constants').fs;
 
 const isWindows = process.platform === 'win32';
 
+let fs;
+function lazyLoadFs() {
+  if (!fs) {
+    fs = require('fs');
+  }
+  return fs;
+}
+
 function assertEncoding(encoding) {
   if (encoding && !Buffer.isEncoding(encoding)) {
     throw new ERR_INVALID_OPT_VALUE_ENCODING(encoding);
   }
 }
 
+class Dirent {
+  constructor(name, type) {
+    this.name = name;
+    this[kType] = type;
+  }
+
+  isDirectory() {
+    return this[kType] === UV_DIRENT_DIR;
+  }
+
+  isFile() {
+    return this[kType] === UV_DIRENT_FILE;
+  }
+
+  isBlockDevice() {
+    return this[kType] === UV_DIRENT_BLOCK;
+  }
+
+  isCharacterDevice() {
+    return this[kType] === UV_DIRENT_CHAR;
+  }
+
+  isSymbolicLink() {
+    return this[kType] === UV_DIRENT_LINK;
+  }
+
+  isFIFO() {
+    return this[kType] === UV_DIRENT_FIFO;
+  }
+
+  isSocket() {
+    return this[kType] === UV_DIRENT_SOCKET;
+  }
+}
+
+class DirentFromStats extends Dirent {
+  constructor(name, stats) {
+    super(name, null);
+    this[kStats] = stats;
+  }
+}
+
+for (const name of Reflect.ownKeys(Dirent.prototype)) {
+  if (name === 'constructor') {
+    continue;
+  }
+  DirentFromStats.prototype[name] = function() {
+    return this[kStats][name]();
+  };
+}
+
 function copyObject(source) {
   var target = {};
   for (var key in source)
@@ -49,6 +118,50 @@ function copyObject(source) {
   return target;
 }
 
+function getDirents(path, [names, types], callback) {
+  var i;
+  if (typeof callback == 'function') {
+    const len = names.length;
+    let toFinish = 0;
+    for (i = 0; i < len; i++) {
+      const type = types[i];
+      if (type === UV_DIRENT_UNKNOWN) {
+        const name = names[i];
+        const idx = i;
+        toFinish++;
+        lazyLoadFs().stat(pathModule.resolve(path, name), (err, stats) => {
+          if (err) {
+            callback(err);
+            return;
+          }
+          names[idx] = new DirentFromStats(name, stats);
+          if (--toFinish === 0) {
+            callback(null, names);
+          }
+        });
+      } else {
+        names[i] = new Dirent(names[i], types[i]);
+      }
+    }
+    if (toFinish === 0) {
+      callback(null, names);
+    }
+  } else {
+    const len = names.length;
+    for (i = 0; i < len; i++) {
+      const type = types[i];
+      if (type === UV_DIRENT_UNKNOWN) {
+        const name = names[i];
+        const stats = lazyLoadFs().statSync(pathModule.resolve(path, name));
+        names[i] = new DirentFromStats(name, stats);
+      } else {
+        names[i] = new Dirent(names[i], types[i]);
+      }
+    }
+    return names;
+  }
+}
+
 function getOptions(options, defaultOptions) {
   if (options === null || options === undefined ||
       typeof options === 'function') {
@@ -341,6 +454,8 @@ function validatePath(path, propName = 'path') {
 module.exports = {
   assertEncoding,
   copyObject,
+  Dirent,
+  getDirents,
   getOptions,
   nullCheck,
   preprocessSymlinkDestination,
diff --git a/src/node_constants.cc b/src/node_constants.cc
index 61aa42a8efb6b2..28d5a9ca4edd38 100644
--- a/src/node_constants.cc
+++ b/src/node_constants.cc
@@ -1022,6 +1022,16 @@ void DefineSystemConstants(Local<Object> target) {
   NODE_DEFINE_CONSTANT(target, O_WRONLY);
   NODE_DEFINE_CONSTANT(target, O_RDWR);
 
+  // file types from readdir
+  NODE_DEFINE_CONSTANT(target, UV_DIRENT_UNKNOWN);
+  NODE_DEFINE_CONSTANT(target, UV_DIRENT_FILE);
+  NODE_DEFINE_CONSTANT(target, UV_DIRENT_DIR);
+  NODE_DEFINE_CONSTANT(target, UV_DIRENT_LINK);
+  NODE_DEFINE_CONSTANT(target, UV_DIRENT_FIFO);
+  NODE_DEFINE_CONSTANT(target, UV_DIRENT_SOCKET);
+  NODE_DEFINE_CONSTANT(target, UV_DIRENT_CHAR);
+  NODE_DEFINE_CONSTANT(target, UV_DIRENT_BLOCK);
+
   NODE_DEFINE_CONSTANT(target, S_IFMT);
   NODE_DEFINE_CONSTANT(target, S_IFREG);
   NODE_DEFINE_CONSTANT(target, S_IFDIR);
diff --git a/src/node_file.cc b/src/node_file.cc
index 16f2506d1f2e2b..2d53e231bff4b8 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -551,51 +551,139 @@ void AfterScanDir(uv_fs_t* req) {
   FSReqBase* req_wrap = FSReqBase::from_req(req);
   FSReqAfterScope after(req_wrap, req);
 
-  if (after.Proceed()) {
-    Environment* env = req_wrap->env();
-    Local<Value> error;
-    int r;
-    Local<Array> names = Array::New(env->isolate(), 0);
-    Local<Function> fn = env->push_values_to_array_function();
-    Local<Value> name_argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
-    size_t name_idx = 0;
+  if (!after.Proceed()) {
+    return;
+  }
+  Environment* env = req_wrap->env();
+  Local<Value> error;
+  int r;
+  Local<Array> names = Array::New(env->isolate(), 0);
+  Local<Function> fn = env->push_values_to_array_function();
+  Local<Value> name_argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
+  size_t name_idx = 0;
+
+  for (int i = 0; ; i++) {
+    uv_dirent_t ent;
+
+    r = uv_fs_scandir_next(req, &ent);
+    if (r == UV_EOF)
+      break;
+    if (r != 0) {
+      return req_wrap->Reject(
+          UVException(r, nullptr, req_wrap->syscall(),
+            static_cast<const char*>(req->path)));
+    }
 
-    for (int i = 0; ; i++) {
-      uv_dirent_t ent;
+    MaybeLocal<Value> filename =
+      StringBytes::Encode(env->isolate(),
+          ent.name,
+          req_wrap->encoding(),
+          &error);
+    if (filename.IsEmpty())
+      return req_wrap->Reject(error);
 
-      r = uv_fs_scandir_next(req, &ent);
-      if (r == UV_EOF)
-        break;
-      if (r != 0) {
-        return req_wrap->Reject(
-            UVException(r, nullptr, req_wrap->syscall(),
-                        static_cast<const char*>(req->path)));
+    name_argv[name_idx++] = filename.ToLocalChecked();
+
+    if (name_idx >= arraysize(name_argv)) {
+      MaybeLocal<Value> ret = fn->Call(env->context(), names, name_idx,
+                                       name_argv);
+      if (ret.IsEmpty()) {
+        return;
       }
+      name_idx = 0;
+    }
+  }
 
-      MaybeLocal<Value> filename =
-          StringBytes::Encode(env->isolate(),
-                              ent.name,
-                              req_wrap->encoding(),
-                              &error);
-      if (filename.IsEmpty())
-        return req_wrap->Reject(error);
+  if (name_idx > 0) {
+    fn->Call(env->context(), names, name_idx, name_argv)
+      .ToLocalChecked();
+  }
 
-      name_argv[name_idx++] = filename.ToLocalChecked();
+  req_wrap->Resolve(names);
+}
 
-      if (name_idx >= arraysize(name_argv)) {
-        fn->Call(env->context(), names, name_idx, name_argv)
-            .ToLocalChecked();
-        name_idx = 0;
+void AfterScanDirWithTypes(uv_fs_t* req) {
+  FSReqBase* req_wrap = FSReqBase::from_req(req);
+  FSReqAfterScope after(req_wrap, req);
+
+  if (!after.Proceed()) {
+    return;
+  }
+
+  Environment* env = req_wrap->env();
+  Local<Value> error;
+  int r;
+  Local<Array> names = Array::New(env->isolate(), 0);
+  Local<Function> fn = env->push_values_to_array_function();
+  Local<Value> name_argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
+  size_t name_idx = 0;
+  Local<Value> types = Array::New(env->isolate(), 0);
+  Local<Value> type_argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
+  size_t type_idx = 0;
+
+  for (int i = 0; ; i++) {
+    uv_dirent_t ent;
+
+    r = uv_fs_scandir_next(req, &ent);
+    if (r == UV_EOF)
+      break;
+    if (r != 0) {
+      return req_wrap->Reject(
+          UVException(r, nullptr, req_wrap->syscall(),
+            static_cast<const char*>(req->path)));
+    }
+
+    MaybeLocal<Value> filename =
+      StringBytes::Encode(env->isolate(),
+          ent.name,
+          req_wrap->encoding(),
+          &error);
+    if (filename.IsEmpty())
+      return req_wrap->Reject(error);
+
+    name_argv[name_idx++] = filename.ToLocalChecked();
+
+    if (name_idx >= arraysize(name_argv)) {
+      MaybeLocal<Value> ret = fn->Call(env->context(), names, name_idx,
+                                       name_argv);
+      if (ret.IsEmpty()) {
+        return;
       }
+      name_idx = 0;
     }
 
-    if (name_idx > 0) {
-      fn->Call(env->context(), names, name_idx, name_argv)
-          .ToLocalChecked();
+    type_argv[type_idx++] = Integer::New(env->isolate(), ent.type);
+
+    if (type_idx >= arraysize(type_argv)) {
+      MaybeLocal<Value> ret = fn->Call(env->context(), types, type_idx,
+          type_argv);
+      if (ret.IsEmpty()) {
+        return;
+      }
+      type_idx = 0;
+    }
+  }
+
+  if (name_idx > 0) {
+    MaybeLocal<Value> ret = fn->Call(env->context(), names, name_idx,
+        name_argv);
+    if (ret.IsEmpty()) {
+      return;
     }
+  }
 
-    req_wrap->Resolve(names);
+  if (type_idx > 0) {
+    MaybeLocal<Value> ret = fn->Call(env->context(), types, type_idx,
+        type_argv);
+    if (ret.IsEmpty()) {
+      return;
+    }
   }
+
+  Local<Array> result = Array::New(env->isolate(), 2);
+  result->Set(0, names);
+  result->Set(1, types);
+  req_wrap->Resolve(result);
 }
 
 
@@ -1228,15 +1316,22 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
 
   const enum encoding encoding = ParseEncoding(env->isolate(), args[1], UTF8);
 
-  FSReqBase* req_wrap_async = GetReqWrap(env, args[2]);
-  if (req_wrap_async != nullptr) {  // readdir(path, encoding, req)
-    AsyncCall(env, req_wrap_async, args, "scandir", encoding, AfterScanDir,
-              uv_fs_scandir, *path, 0 /*flags*/);
-  } else {  // readdir(path, encoding, undefined, ctx)
-    CHECK_EQ(argc, 4);
+  bool with_types = args[2]->BooleanValue();
+
+  FSReqBase* req_wrap_async = GetReqWrap(env, args[3]);
+  if (req_wrap_async != nullptr) {  // readdir(path, encoding, withTypes, req)
+    if (with_types) {
+      AsyncCall(env, req_wrap_async, args, "scandir", encoding,
+                AfterScanDirWithTypes, uv_fs_scandir, *path, 0 /*flags*/);
+    } else {
+      AsyncCall(env, req_wrap_async, args, "scandir", encoding,
+                AfterScanDir, uv_fs_scandir, *path, 0 /*flags*/);
+    }
+  } else {  // readdir(path, encoding, withTypes, undefined, ctx)
+    CHECK_EQ(argc, 5);
     FSReqWrapSync req_wrap_sync;
     FS_SYNC_TRACE_BEGIN(readdir);
-    int err = SyncCall(env, args[3], &req_wrap_sync, "scandir",
+    int err = SyncCall(env, args[4], &req_wrap_sync, "scandir",
                        uv_fs_scandir, *path, 0 /*flags*/);
     FS_SYNC_TRACE_END(readdir);
     if (err < 0) {
@@ -1250,6 +1345,14 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
     Local<Value> name_v[NODE_PUSH_VAL_TO_ARRAY_MAX];
     size_t name_idx = 0;
 
+    Local<Value> types;
+    Local<Value> type_v[NODE_PUSH_VAL_TO_ARRAY_MAX];
+    size_t type_idx;
+    if (with_types) {
+      types = Array::New(env->isolate(), 0);
+      type_idx = 0;
+    }
+
     for (int i = 0; ; i++) {
       uv_dirent_t ent;
 
@@ -1257,7 +1360,7 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
       if (r == UV_EOF)
         break;
       if (r != 0) {
-        Local<Object> ctx = args[3].As<Object>();
+        Local<Object> ctx = args[4].As<Object>();
         ctx->Set(env->context(), env->errno_string(),
                  Integer::New(env->isolate(), r)).FromJust();
         ctx->Set(env->context(), env->syscall_string(),
@@ -1270,8 +1373,9 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
                                                        ent.name,
                                                        encoding,
                                                        &error);
+
       if (filename.IsEmpty()) {
-        Local<Object> ctx = args[3].As<Object>();
+        Local<Object> ctx = args[4].As<Object>();
         ctx->Set(env->context(), env->error_string(), error).FromJust();
         return;
       }
@@ -1286,6 +1390,19 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
         }
         name_idx = 0;
       }
+
+      if (with_types) {
+        type_v[type_idx++] = Integer::New(env->isolate(), ent.type);
+
+        if (type_idx >= arraysize(type_v)) {
+          MaybeLocal<Value> ret = fn->Call(env->context(), types, type_idx,
+              type_v);
+          if (ret.IsEmpty()) {
+            return;
+          }
+          type_idx = 0;
+        }
+      }
     }
 
     if (name_idx > 0) {
@@ -1295,7 +1412,21 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
       }
     }
 
-    args.GetReturnValue().Set(names);
+    if (with_types && type_idx > 0) {
+      MaybeLocal<Value> ret = fn->Call(env->context(), types, type_idx, type_v);
+      if (ret.IsEmpty()) {
+        return;
+      }
+    }
+
+    if (with_types) {
+      Local<Array> result = Array::New(env->isolate(), 2);
+      result->Set(0, names);
+      result->Set(1, types);
+      args.GetReturnValue().Set(result);
+    } else {
+      args.GetReturnValue().Set(names);
+    }
   }
 }
 
diff --git a/test/parallel/test-fs-readdir-types.js b/test/parallel/test-fs-readdir-types.js
new file mode 100644
index 00000000000000..75452895cc178e
--- /dev/null
+++ b/test/parallel/test-fs-readdir-types.js
@@ -0,0 +1,95 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const fs = require('fs');
+
+const tmpdir = require('../common/tmpdir');
+
+const binding = process.binding('fs');
+
+const readdirDir = tmpdir.path;
+const files = ['empty', 'files', 'for', 'just', 'testing'];
+const constants = process.binding('constants').fs;
+const types = {
+  isDirectory: constants.UV_DIRENT_DIR,
+  isFile: constants.UV_DIRENT_FILE,
+  isBlockDevice: constants.UV_DIRENT_BLOCK,
+  isCharacterDevice: constants.UV_DIRENT_CHAR,
+  isSymbolicLink: constants.UV_DIRENT_LINK,
+  isFIFO: constants.UV_DIRENT_FIFO,
+  isSocket: constants.UV_DIRENT_SOCKET
+};
+const typeMethods = Object.keys(types);
+
+// Make sure tmp directory is clean
+tmpdir.refresh();
+
+// Create the necessary files
+files.forEach(function(currentFile) {
+  fs.closeSync(fs.openSync(`${readdirDir}/${currentFile}`, 'w'));
+});
+
+
+function assertDirents(dirents) {
+  assert.strictEqual(files.length, dirents.length);
+  for (const [i, dirent] of dirents.entries()) {
+    assert(dirent instanceof fs.Dirent);
+    assert.strictEqual(dirent.name, files[i]);
+    assert.strictEqual(dirent.isFile(), true);
+    assert.strictEqual(dirent.isDirectory(), false);
+    assert.strictEqual(dirent.isSocket(), false);
+    assert.strictEqual(dirent.isBlockDevice(), false);
+    assert.strictEqual(dirent.isCharacterDevice(), false);
+    assert.strictEqual(dirent.isFIFO(), false);
+    assert.strictEqual(dirent.isSymbolicLink(), false);
+  }
+}
+
+// Check the readdir Sync version
+assertDirents(fs.readdirSync(readdirDir, { withFileTypes: true }));
+
+// Check the readdir async version
+fs.readdir(readdirDir, {
+  withFileTypes: true
+}, common.mustCall((err, dirents) => {
+  assert.ifError(err);
+  assertDirents(dirents);
+}));
+
+// Check for correct types when the binding returns unknowns
+const UNKNOWN = constants.UV_DIRENT_UNKNOWN;
+const oldReaddir = binding.readdir;
+binding.readdir = common.mustCall((path, encoding, types, req, ctx) => {
+  if (req) {
+    const oldCb = req.oncomplete;
+    req.oncomplete = (err, results) => {
+      if (err) {
+        oldCb(err);
+        return;
+      }
+      results[1] = results[1].map(() => UNKNOWN);
+      oldCb(null, results);
+    };
+    oldReaddir(path, encoding, types, req);
+  } else {
+    const results = oldReaddir(path, encoding, types, req, ctx);
+    results[1] = results[1].map(() => UNKNOWN);
+    return results;
+  }
+}, 2);
+assertDirents(fs.readdirSync(readdirDir, { withFileTypes: true }));
+fs.readdir(readdirDir, {
+  withFileTypes: true
+}, common.mustCall((err, dirents) => {
+  assert.ifError(err);
+  assertDirents(dirents);
+}));
+
+// Dirent types
+for (const method of typeMethods) {
+  const dirent = new fs.Dirent('foo', types[method]);
+  for (const testMethod of typeMethods) {
+    assert.strictEqual(dirent[testMethod](), testMethod === method);
+  }
+}
diff --git a/tools/doc/type-parser.js b/tools/doc/type-parser.js
index 42cc27f19c5261..2a63f97c798075 100644
--- a/tools/doc/type-parser.js
+++ b/tools/doc/type-parser.js
@@ -58,6 +58,7 @@ const customTypesMap = {
   'EventEmitter': 'events.html#events_class_eventemitter',
 
   'FileHandle': 'fs.html#fs_class_filehandle',
+  'fs.Dirent': 'fs.html#fs_class_fs_dirent',
   'fs.FSWatcher': 'fs.html#fs_class_fs_fswatcher',
   'fs.ReadStream': 'fs.html#fs_class_fs_readstream',
   'fs.Stats': 'fs.html#fs_class_fs_stats',