BREAKING: Include limited metadata in 'DirEntry' objects (denoland/de…

…no#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.
caspervonb · Feb 1, 2021 · 5cc4e45 · 5cc4e45
1 parent 1204b3d
commit 5cc4e45
Show file tree

Hide file tree

Showing 9 changed files with 166 additions and 167 deletions.
diff --git a/fs/README.md b/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);
   }
 }
 

diff --git a/fs/copy.ts b/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);
   }
 }
diff --git a/fs/expand_glob.ts b/fs/expand_glob.ts
@@ -8,9 +8,15 @@ import {
   joinGlobs,
   normalize,
 } from "../path/mod.ts";
-import { WalkEntry, walk, walkSync } from "./walk.ts";
+import {
+  WalkEntry,
+  createWalkEntry,
+  createWalkEntrySync,
+  walk,
+  walkSync,
+} from "./walk.ts";
 import { assert } from "../testing/asserts.ts";
-const { cwd, stat, statSync } = Deno;
+const { cwd } = Deno;
 type FileInfo = Deno.FileInfo;
 
 export interface ExpandGlobOptions extends GlobOptions {
@@ -48,6 +54,12 @@ function throwUnlessNotFound(error: Error): void {
   }
 }
 
+function comparePath(a: WalkEntry, b: WalkEntry): number {
+  if (a.path < b.path) return -1;
+  if (a.path > b.path) return 1;
+  return 0;
+}
+
 /**
  * Expand the glob string from the specified `root` directory and yield each
  * result as a `WalkEntry` object.
@@ -73,8 +85,8 @@ export async function* expandGlob(
   const excludePatterns = exclude
     .map(resolveFromRoot)
     .map((s: string): RegExp => globToRegExp(s, globOptions));
-  const shouldInclude = (filename: string): boolean =>
-    !excludePatterns.some((p: RegExp): boolean => !!filename.match(p));
+  const shouldInclude = (path: string): boolean =>
+    !excludePatterns.some((p: RegExp): boolean => !!path.match(p));
   const { segments, hasTrailingSep, winRoot } = split(resolveFromRoot(glob));
 
   let fixedRoot = winRoot != undefined ? winRoot : "/";
@@ -86,7 +98,7 @@ export async function* expandGlob(
 
   let fixedRootInfo: WalkEntry;
   try {
-    fixedRootInfo = { filename: fixedRoot, info: await stat(fixedRoot) };
+    fixedRootInfo = await createWalkEntry(fixedRoot);
   } catch (error) {
     return throwUnlessNotFound(error);
   }
@@ -95,29 +107,29 @@ export async function* expandGlob(
     walkInfo: WalkEntry,
     globSegment: string
   ): AsyncIterableIterator<WalkEntry> {
-    if (!walkInfo.info.isDirectory) {
+    if (!walkInfo.isDirectory) {
       return;
     } else if (globSegment == "..") {
-      const parentPath = joinGlobs([walkInfo.filename, ".."], globOptions);
+      const parentPath = joinGlobs([walkInfo.path, ".."], globOptions);
       try {
         if (shouldInclude(parentPath)) {
-          return yield { filename: parentPath, info: await stat(parentPath) };
+          return yield await createWalkEntry(parentPath);
         }
       } catch (error) {
         throwUnlessNotFound(error);
       }
       return;
     } else if (globSegment == "**") {
-      return yield* walk(walkInfo.filename, {
+      return yield* walk(walkInfo.path, {
         includeFiles: false,
         skip: excludePatterns,
       });
     }
-    yield* walk(walkInfo.filename, {
+    yield* walk(walkInfo.path, {
       maxDepth: 1,
       match: [
         globToRegExp(
-          joinGlobs([walkInfo.filename, globSegment], globOptions),
+          joinGlobs([walkInfo.path, globSegment], globOptions),
           globOptions
         ),
       ],
@@ -129,27 +141,22 @@ export async function* expandGlob(
   for (const segment of segments) {
     // Advancing the list of current matches may introduce duplicates, so we
     // pass everything through this Map.
-    const nextMatchMap: Map<string, FileInfo> = new Map();
+    const nextMatchMap: Map<string, WalkEntry> = new Map();
     for (const currentMatch of currentMatches) {
       for await (const nextMatch of advanceMatch(currentMatch, segment)) {
-        nextMatchMap.set(nextMatch.filename, nextMatch.info);
+        nextMatchMap.set(nextMatch.path, nextMatch);
       }
     }
-    currentMatches = [...nextMatchMap].sort().map(
-      ([filename, info]): WalkEntry => ({
-        filename,
-        info,
-      })
-    );
+    currentMatches = [...nextMatchMap.values()].sort(comparePath);
   }
   if (hasTrailingSep) {
     currentMatches = currentMatches.filter(
-      ({ info }): boolean => info.isDirectory
+      (entry: WalkEntry): boolean => entry.isDirectory
     );
   }
   if (!includeDirs) {
     currentMatches = currentMatches.filter(
-      ({ info }): boolean => !info.isDirectory
+      (entry: WalkEntry): boolean => !entry.isDirectory
     );
   }
   yield* currentMatches;
@@ -177,8 +184,8 @@ export function* expandGlobSync(
   const excludePatterns = exclude
     .map(resolveFromRoot)
     .map((s: string): RegExp => globToRegExp(s, globOptions));
-  const shouldInclude = (filename: string): boolean =>
-    !excludePatterns.some((p: RegExp): boolean => !!filename.match(p));
+  const shouldInclude = (path: string): boolean =>
+    !excludePatterns.some((p: RegExp): boolean => !!path.match(p));
   const { segments, hasTrailingSep, winRoot } = split(resolveFromRoot(glob));
 
   let fixedRoot = winRoot != undefined ? winRoot : "/";
@@ -190,7 +197,7 @@ export function* expandGlobSync(
 
   let fixedRootInfo: WalkEntry;
   try {
-    fixedRootInfo = { filename: fixedRoot, info: statSync(fixedRoot) };
+    fixedRootInfo = createWalkEntrySync(fixedRoot);
   } catch (error) {
     return throwUnlessNotFound(error);
   }
@@ -199,29 +206,29 @@ export function* expandGlobSync(
     walkInfo: WalkEntry,
     globSegment: string
   ): IterableIterator<WalkEntry> {
-    if (!walkInfo.info.isDirectory) {
+    if (!walkInfo.isDirectory) {
       return;
     } else if (globSegment == "..") {
-      const parentPath = joinGlobs([walkInfo.filename, ".."], globOptions);
+      const parentPath = joinGlobs([walkInfo.path, ".."], globOptions);
       try {
         if (shouldInclude(parentPath)) {
-          return yield { filename: parentPath, info: statSync(parentPath) };
+          return yield createWalkEntrySync(parentPath);
         }
       } catch (error) {
         throwUnlessNotFound(error);
       }
       return;
     } else if (globSegment == "**") {
-      return yield* walkSync(walkInfo.filename, {
+      return yield* walkSync(walkInfo.path, {
         includeFiles: false,
         skip: excludePatterns,
       });
     }
-    yield* walkSync(walkInfo.filename, {
+    yield* walkSync(walkInfo.path, {
       maxDepth: 1,
       match: [
         globToRegExp(
-          joinGlobs([walkInfo.filename, globSegment], globOptions),
+          joinGlobs([walkInfo.path, globSegment], globOptions),
           globOptions
         ),
       ],
@@ -233,27 +240,22 @@ export function* expandGlobSync(
   for (const segment of segments) {
     // Advancing the list of current matches may introduce duplicates, so we
     // pass everything through this Map.
-    const nextMatchMap: Map<string, FileInfo> = new Map();
+    const nextMatchMap: Map<string, WalkEntry> = new Map();
     for (const currentMatch of currentMatches) {
       for (const nextMatch of advanceMatch(currentMatch, segment)) {
-        nextMatchMap.set(nextMatch.filename, nextMatch.info);
+        nextMatchMap.set(nextMatch.path, nextMatch);
       }
     }
-    currentMatches = [...nextMatchMap].sort().map(
-      ([filename, info]): WalkEntry => ({
-        filename,
-        info,
-      })
-    );
+    currentMatches = [...nextMatchMap.values()].sort(comparePath);
   }
   if (hasTrailingSep) {
     currentMatches = currentMatches.filter(
-      ({ info }): boolean => info.isDirectory
+      (entry: WalkEntry): boolean => entry.isDirectory
     );
   }
   if (!includeDirs) {
     currentMatches = currentMatches.filter(
-      ({ info }): boolean => !info.isDirectory
+      (entry: WalkEntry): boolean => !entry.isDirectory
     );
   }
   yield* currentMatches;

diff --git a/fs/expand_glob_test.ts b/fs/expand_glob_test.ts
@@ -19,12 +19,12 @@ async function expandGlobArray(
   options: ExpandGlobOptions
 ): Promise<string[]> {
   const paths: string[] = [];
-  for await (const { filename } of expandGlob(globString, options)) {
-    paths.push(filename);
+  for await (const { path } of expandGlob(globString, options)) {
+    paths.push(path);
   }
   paths.sort();
   const pathsSync = [...expandGlobSync(globString, options)].map(
-    ({ filename }): string => filename
+    ({ path }): string => path
   );
   pathsSync.sort();
   assertEquals(paths, pathsSync);