From 1b67eca6239f92a59d3af7cea68210f13d6707c4 Mon Sep 17 00:00:00 2001
From: Ryan Dahl <ry@tinyclouds.org>
Date: Mon, 13 May 2019 11:13:16 -0400
Subject: [PATCH] walk should not use fileInfo.path

---
 fs/walk.ts      | 105 +++++++++++++++++++++++-------------------------
 fs/walk_test.ts |  55 +++++++++++++++----------
 2 files changed, 84 insertions(+), 76 deletions(-)

diff --git a/fs/walk.ts b/fs/walk.ts
index b21044e72d62a..e69665fc10bb1 100644
--- a/fs/walk.ts
+++ b/fs/walk.ts
@@ -1,5 +1,10 @@
+// Documentation and interface for walk were adapted from Go
+// https://golang.org/pkg/path/filepath/#Walk
+// Copyright 2009 The Go Authors. All rights reserved. BSD license.
 const { readDir, readDirSync, readlink, readlinkSync, stat, statSync } = Deno;
 type FileInfo = Deno.FileInfo;
+import { unimplemented } from "../testing/asserts.ts";
+import { join } from "./path/mod.ts";
 
 export interface WalkOptions {
   maxDepth?: number;
@@ -23,121 +28,113 @@ function patternTest(patterns: RegExp[], path: string): boolean {
   );
 }
 
-function include(f: FileInfo, options: WalkOptions): boolean {
+function include(filename: string, options: WalkOptions): boolean {
   if (
     options.exts &&
-    !options.exts.some((ext): boolean => f.path.endsWith(ext))
+    !options.exts.some((ext): boolean => filename.endsWith(ext))
   ) {
     return false;
   }
-  if (options.match && !patternTest(options.match, f.path)) {
+  if (options.match && !patternTest(options.match, filename)) {
     return false;
   }
-  if (options.skip && patternTest(options.skip, f.path)) {
+  if (options.skip && patternTest(options.skip, filename)) {
     return false;
   }
   return true;
 }
 
-async function resolve(f: FileInfo): Promise<FileInfo> {
-  // This is the full path, unfortunately if we were to make it relative
-  // it could resolve to a symlink and cause an infinite loop.
-  const fpath = await readlink(f.path);
-  f = await stat(fpath);
-  // workaround path not being returned by stat
-  f.path = fpath;
-  return f;
-}
 
-function resolveSync(f: FileInfo): FileInfo {
-  // This is the full path, unfortunately if we were to make it relative
-  // it could resolve to a symlink and cause an infinite loop.
-  const fpath = readlinkSync(f.path);
-  f = statSync(fpath);
-  // workaround path not being returned by stat
-  f.path = fpath;
-  return f;
-}
 
-/** Generate all files in a directory recursively.
+/** Walks the file tree rooted at root, calling walkFn for each file or
+ * directory in the tree, including root. The files are walked in lexical
+ * order, which makes the output deterministic but means that for very large
+ * directories walk() can be inefficient.
+ *
+ * Options:
+ * - maxDepth?: number;
+ * - exts?: string[];
+ * - match?: RegExp[];
+ * - skip?: RegExp[];
+ * - onError?: (err: Error) => void;
+ * - followSymlinks?: boolean;
  *
- *      for await (const fileInfo of walk()) {
- *        console.log(fileInfo.path);
+ *      for await (const [path, fileInfo] of walk(".")) {
+ *        console.log(path);
  *        assert(fileInfo.isFile());
  *      };
  */
 export async function* walk(
-  dir: string = ".",
+  root: string,
   options: WalkOptions = {}
-): AsyncIterableIterator<FileInfo> {
+): AsyncIterableIterator<[string, FileInfo]> {
   options.maxDepth -= 1;
   let ls: FileInfo[] = [];
   try {
-    ls = await readDir(dir);
+    ls = await readDir(root);
   } catch (err) {
     if (options.onError) {
       options.onError(err);
     }
   }
   const length = ls.length;
-  for (var i = 0; i < length; i++) {
-    let f = ls[i];
-    if (f.isSymlink()) {
+  for (let fileInfo of ls) {
+    if (fileInfo.isSymlink()) {
       if (options.followSymlinks) {
-        f = await resolve(f);
+        // TODO(ry) Re-enable followSymlinks.
+        unimplemented();
       } else {
         continue;
       }
     }
-    if (f.isFile()) {
-      if (include(f, options)) {
-        yield f;
+
+    const filename = join(root, fileInfo.name);
+
+    if (fileInfo.isFile()) {
+      if (include(filename, options)) {
+        yield [filename, fileInfo];
       }
     } else {
       if (!(options.maxDepth < 0)) {
-        yield* walk(f.path, options);
+        yield* walk(filename, options);
       }
     }
   }
 }
 
-/** Generate all files in a directory recursively.
- *
- *      for (const fileInfo of walkSync()) {
- *        console.log(fileInfo.path);
- *        assert(fileInfo.isFile());
- *      };
- */
+/** Same as walk() but uses synchronous ops */
 export function* walkSync(
-  dir: string = ".",
+  root: string = ".",
   options: WalkOptions = {}
-): IterableIterator<FileInfo> {
+): IterableIterator<[string, FileInfo]> {
   options.maxDepth -= 1;
   let ls: FileInfo[] = [];
   try {
-    ls = readDirSync(dir);
+    ls = readDirSync(root);
   } catch (err) {
     if (options.onError) {
       options.onError(err);
     }
   }
   const length = ls.length;
-  for (var i = 0; i < length; i++) {
-    let f = ls[i];
-    if (f.isSymlink()) {
+  for (let fileInfo of ls) {
+    if (fileInfo.isSymlink()) {
       if (options.followSymlinks) {
-        f = resolveSync(f);
+        unimplemented();
       } else {
         continue;
       }
     }
-    if (f.isFile()) {
-      if (include(f, options)) {
-        yield f;
+
+    const filename = join(root, fileInfo.name);
+
+    if (fileInfo.isFile()) {
+      if (include(filename, options)) {
+        yield [filename, fileInfo];
       }
     } else {
       if (!(options.maxDepth < 0)) {
-        yield* walkSync(f.path, options);
+        yield* walkSync(filename, options);
       }
     }
   }
diff --git a/fs/walk_test.ts b/fs/walk_test.ts
index e94a653c6be54..63a6741f99dea 100644
--- a/fs/walk_test.ts
+++ b/fs/walk_test.ts
@@ -1,7 +1,7 @@
 const { cwd, chdir, makeTempDir, mkdir, open, build, remove, symlink } = Deno;
 type FileInfo = Deno.FileInfo;
 import { walk, walkSync, WalkOptions } from "./walk.ts";
-import { test, TestFunction } from "../testing/mod.ts";
+import { test, TestFunction, runIfMain } from "../testing/mod.ts";
 import { assert, assertEquals } from "../testing/asserts.ts";
 
 const isWindows = build.os === "win";
@@ -26,19 +26,24 @@ export async function testWalk(
   test({ name, fn });
 }
 
+function normalize([filename, _info]: [string, FileInfo]): string {
+  return filename.replace(/\\/g, "/")
+}
+
 async function walkArray(
-  dirname: string = ".",
+  root: string = ".",
   options: WalkOptions = {}
 ): Promise<string[]> {
   const arr: string[] = [];
-  for await (const f of walk(dirname, { ...options })) {
-    arr.push(f.path.replace(/\\/g, "/"));
+  for await (const w of walk(root, { ...options })) {
+    arr.push(normalize(w));
   }
-  arr.sort();
+  arr.sort(); // TODO(ry) Remove sort. The order should be deterministic.
   const arrSync = Array.from(
-    walkSync(dirname, options),
-    (f: FileInfo): string => f.path.replace(/\\/g, "/")
-  ).sort();
+    walkSync(root, options),
+    normalize
+  );
+  arrSync.sort(); // TODO(ry) Remove sort. The order should be deterministic.
   assertEquals(arr, arrSync);
   return arr;
 }
@@ -46,8 +51,10 @@ async function walkArray(
 async function touch(path: string): Promise<void> {
   await open(path, "w");
 }
+
 function assertReady(expectedLength: number): void {
-  const arr = Array.from(walkSync(), (f: FileInfo): string => f.path);
+  const arr = Array.from(walkSync(), normalize);
+
   assertEquals(arr.length, expectedLength);
 }
 
@@ -68,7 +75,7 @@ testWalk(
   async function singleFile(): Promise<void> {
     const arr = await walkArray();
     assertEquals(arr.length, 1);
-    assertEquals(arr[0], "./x");
+    assertEquals(arr[0], "x");
   }
 );
 
@@ -78,11 +85,11 @@ testWalk(
   },
   async function iteratable(): Promise<void> {
     let count = 0;
-    for (const _ of walkSync()) {
+    for (const _ of walkSync(".")) {
       count += 1;
     }
     assertEquals(count, 1);
-    for await (const _ of walk()) {
+    for await (const _ of walk(".")) {
       count += 1;
     }
     assertEquals(count, 2);
@@ -97,7 +104,7 @@ testWalk(
   async function nestedSingleFile(): Promise<void> {
     const arr = await walkArray();
     assertEquals(arr.length, 1);
-    assertEquals(arr[0], "./a/x");
+    assertEquals(arr[0], "a/x");
   }
 );
 
@@ -112,7 +119,7 @@ testWalk(
     assertEquals(arr3.length, 0);
     const arr5 = await walkArray(".", { maxDepth: 5 });
     assertEquals(arr5.length, 1);
-    assertEquals(arr5[0], "./a/b/c/d/x");
+    assertEquals(arr5[0], "a/b/c/d/x");
   }
 );
 
@@ -125,7 +132,7 @@ testWalk(
     assertReady(2);
     const arr = await walkArray(".", { exts: [".ts"] });
     assertEquals(arr.length, 1);
-    assertEquals(arr[0], "./x.ts");
+    assertEquals(arr[0], "x.ts");
   }
 );
 
@@ -139,8 +146,8 @@ testWalk(
     assertReady(3);
     const arr = await walkArray(".", { exts: [".rs", ".ts"] });
     assertEquals(arr.length, 2);
-    assertEquals(arr[0], "./x.ts");
-    assertEquals(arr[1], "./y.rs");
+    assertEquals(arr[0], "x.ts");
+    assertEquals(arr[1], "y.rs");
   }
 );
 
@@ -153,7 +160,7 @@ testWalk(
     assertReady(2);
     const arr = await walkArray(".", { match: [/x/] });
     assertEquals(arr.length, 1);
-    assertEquals(arr[0], "./x");
+    assertEquals(arr[0], "x");
   }
 );
 
@@ -167,8 +174,8 @@ testWalk(
     assertReady(3);
     const arr = await walkArray(".", { match: [/x/, /y/] });
     assertEquals(arr.length, 2);
-    assertEquals(arr[0], "./x");
-    assertEquals(arr[1], "./y");
+    assertEquals(arr[0], "x");
+    assertEquals(arr[1], "y");
   }
 );
 
@@ -181,7 +188,7 @@ testWalk(
     assertReady(2);
     const arr = await walkArray(".", { skip: [/x/] });
     assertEquals(arr.length, 1);
-    assertEquals(arr[0], "./y");
+    assertEquals(arr[0], "y");
   }
 );
 
@@ -195,7 +202,7 @@ testWalk(
     assertReady(3);
     const arr = await walkArray(".", { skip: [/x/, /y/] });
     assertEquals(arr.length, 1);
-    assertEquals(arr[0], "./z");
+    assertEquals(arr[0], "z");
   }
 );
 
@@ -228,6 +235,7 @@ testWalk(
   }
 );
 
+/* TODO(ry) Re-enable followSymlinks
 testWalk(
   async (d: string): Promise<void> => {
     await mkdir(d + "/a");
@@ -258,3 +266,6 @@ testWalk(
     assert(arr.some((f): boolean => f.endsWith("/b/z")));
   }
 );
+*/
+
+runIfMain(import.meta);