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