BREAKING: Include limited metadata in 'DirEntry' objects (denoland#4941)

This change is to prevent needed a separate stat syscall for each file
when using readdir.

For consistency, this PR also modifies std's `WalkEntry` interface to
extend `DirEntry` with an additional `path` field.

cli/js/lib.deno.ns.d.ts

@@ -1386,8 +1386,11 @@ declare namespace Deno {
    * Requires `allow-read` permission. */
   export function realpath(path: string): Promise<string>;
 
-  export interface DirEntry extends FileInfo {
+  export interface DirEntry {
     name: string;
+    isFile: boolean;
+    isDirectory: boolean;
+    isSymlink: boolean;
   }
 
   /** Synchronously reads the directory given by `path` and returns an iterable

cli/js/ops/fs/read_dir.ts

@@ -1,21 +1,19 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
 import { sendSync, sendAsync } from "../dispatch_json.ts";
-import { FileInfo, StatResponse, parseFileInfo } from "./stat.ts";
 
-export interface DirEntry extends FileInfo {
+export interface DirEntry {
   name: string;
+  isFile: boolean;
+  isDirectory: boolean;
+  isSymlink: boolean;
 }
 
 interface ReadDirResponse {
-  entries: StatResponse[];
+  entries: DirEntry[];
 }
 
 function res(response: ReadDirResponse): DirEntry[] {
-  return response.entries.map(
-    (statRes: StatResponse): DirEntry => {
-      return { ...parseFileInfo(statRes), name: statRes.name! };
-    }
-  );
+  return response.entries;
 }
 
 export function readdirSync(path: string): Iterable<DirEntry> {

cli/js/ops/fs/stat.ts

@@ -29,8 +29,6 @@ export interface StatResponse {
   mtime: number | null;
   atime: number | null;
   birthtime: number | null;
-  // Null for stat(), but exists for readdir().
-  name: string | null;
   // Unix only members
   dev: number;
   ino: number;

cli/js/tests/read_dir_test.ts

@@ -4,19 +4,14 @@ import { unitTest, assert, assertEquals } from "./test_util.ts";
 function assertSameContent(files: Deno.DirEntry[]): void {
   let counter = 0;
 
-  for (const file of files) {
-    if (file.name === "subdir") {
-      assert(file.isDirectory);
-      counter++;
-    }
-
-    if (file.name === "002_hello.ts") {
-      assertEquals(file.mode!, Deno.statSync(`cli/tests/${file.name}`).mode!);
+  for (const entry of files) {
+    if (entry.name === "subdir") {
+      assert(entry.isDirectory);
       counter++;
     }
   }
 
-  assertEquals(counter, 2);
+  assertEquals(counter, 1);
 }
 
 unitTest({ perms: { read: true } }, function readdirSyncSuccess(): void {

cli/ops/fs.rs

@@ -460,10 +460,7 @@ fn to_msec(maybe_time: Result<SystemTime, io::Error>) -> serde_json::Value {
 }
 
 #[inline(always)]
-fn get_stat_json(
-  metadata: std::fs::Metadata,
-  maybe_name: Option<String>,
-) -> JsonResult {
+fn get_stat_json(metadata: std::fs::Metadata) -> JsonResult {
   // Unix stat member (number types only). 0 if not on unix.
   macro_rules! usm {
     ($member: ident) => {{
@@ -480,7 +477,7 @@ fn get_stat_json(
 
   #[cfg(unix)]
   use std::os::unix::fs::MetadataExt;
-  let mut json_val = json!({
+  let json_val = json!({
     "isFile": metadata.is_file(),
     "isDirectory": metadata.is_dir(),
     "isSymlink": metadata.file_type().is_symlink(),
@@ -502,14 +499,6 @@ fn get_stat_json(
     "blksize": usm!(blksize),
     "blocks": usm!(blocks),
   });
-
-  // "name" is an optional field by our design.
-  if let Some(name) = maybe_name {
-    if let serde_json::Value::Object(ref mut m) = json_val {
-      m.insert("name".to_owned(), json!(name));
-    }
-  }
-
   Ok(json_val)
 }
 
@@ -540,7 +529,7 @@ fn op_stat(
     } else {
       std::fs::metadata(&path)?
     };
-    get_stat_json(metadata, None)
+    get_stat_json(metadata)
   })
 }
 
@@ -599,10 +588,15 @@ fn op_read_dir(
     let entries: Vec<_> = std::fs::read_dir(path)?
       .filter_map(|entry| {
         let entry = entry.unwrap();
-        let metadata = entry.metadata().unwrap();
+        let file_type = entry.file_type().unwrap();
         // Not all filenames can be encoded as UTF-8. Skip those for now.
-        if let Ok(filename) = into_string(entry.file_name()) {
-          Some(get_stat_json(metadata, Some(filename)).unwrap())
+        if let Ok(name) = into_string(entry.file_name()) {
+          Some(json!({
+            "name": name,
+            "isFile": file_type.is_file(),
+            "isDirectory": file_type.is_dir(),
+            "isSymlink": file_type.is_symlink()
+          }))
         } else {
           None
         }

std/fs/README.md

@@ -156,8 +156,8 @@ for (const fileInfo of walkSync(".")) {
 
 // Async
 async function printFilesNames() {
-  for await (const fileInfo of walk()) {
-    console.log(fileInfo.filename);
+  for await (const entry of walk()) {
+    console.log(entry.path);
   }
 }
 

std/fs/copy.ts

@@ -157,15 +157,15 @@ async function copyDir(
     await Deno.utime(dest, srcStatInfo.atime, srcStatInfo.mtime);
   }
 
-  for await (const file of Deno.readdir(src)) {
-    const srcPath = path.join(src, file.name);
+  for await (const entry of Deno.readdir(src)) {
+    const srcPath = path.join(src, entry.name);
     const destPath = path.join(dest, path.basename(srcPath as string));
-    if (file.isDirectory) {
+    if (entry.isSymlink) {
+      await copySymLink(srcPath, destPath, options);
+    } else if (entry.isDirectory) {
       await copyDir(srcPath, destPath, options);
-    } else if (file.isFile) {
+    } else if (entry.isFile) {
       await copyFile(srcPath, destPath, options);
-    } else if (file.isSymlink) {
-      await copySymLink(srcPath, destPath, options);
     }
   }
 }
@@ -185,16 +185,16 @@ function copyDirSync(src: string, dest: string, options: CopyOptions): void {
     Deno.utimeSync(dest, srcStatInfo.atime, srcStatInfo.mtime);
   }
 
-  for (const file of Deno.readdirSync(src)) {
-    assert(file.name != null, "file.name must be set");
-    const srcPath = path.join(src, file.name);
+  for (const entry of Deno.readdirSync(src)) {
+    assert(entry.name != null, "file.name must be set");
+    const srcPath = path.join(src, entry.name);
     const destPath = path.join(dest, path.basename(srcPath as string));
-    if (file.isDirectory) {
+    if (entry.isSymlink) {
+      copySymlinkSync(srcPath, destPath, options);
+    } else if (entry.isDirectory) {
       copyDirSync(srcPath, destPath, options);
-    } else if (file.isFile) {
+    } else if (entry.isFile) {
       copyFileSync(srcPath, destPath, options);
-    } else if (file.isSymlink) {
-      copySymlinkSync(srcPath, destPath, options);
     }
   }
 }
@@ -229,12 +229,12 @@ export async function copy(
     );
   }
 
-  if (srcStat.isDirectory) {
+  if (srcStat.isSymlink) {
+    await copySymLink(src, dest, options);
+  } else if (srcStat.isDirectory) {
     await copyDir(src, dest, options);
   } else if (srcStat.isFile) {
     await copyFile(src, dest, options);
-  } else if (srcStat.isSymlink) {
-    await copySymLink(src, dest, options);
   }
 }
 
@@ -268,11 +268,11 @@ export function copySync(
     );
   }
 
-  if (srcStat.isDirectory) {
+  if (srcStat.isSymlink) {
+    copySymlinkSync(src, dest, options);
+  } else if (srcStat.isDirectory) {
     copyDirSync(src, dest, options);
   } else if (srcStat.isFile) {
     copyFileSync(src, dest, options);
-  } else if (srcStat.isSymlink) {
-    copySymlinkSync(src, dest, options);
   }
 }