fs: walk() should not use deprecated FileInfo.path

The walk() interface is change to return a WalkInfo object which contains both the resolved filename and FileInfo object from stat. The followSymlinks feature is disabled for now.
denoland · May 13, 2019 · 360df28 · 360df28
1 parent 49ae943
commit 360df28
Show file tree

Hide file tree

Showing 5 changed files with 120 additions and 135 deletions.
diff --git a/fs/glob_test.ts b/fs/glob_test.ts
@@ -1,32 +1,11 @@
-const { mkdir, open } = Deno;
+const { mkdir } = Deno;
 type FileInfo = Deno.FileInfo;
-import { test } from "../testing/mod.ts";
+import { test, runIfMain } from "../testing/mod.ts";
 import { assertEquals } from "../testing/asserts.ts";
 import { glob } from "./glob.ts";
 import { join } from "./path.ts";
 import { testWalk } from "./walk_test.ts";
-import { walk, walkSync, WalkOptions } from "./walk.ts";
-
-async function touch(path: string): Promise<void> {
-  await open(path, "w");
-}
-
-async function walkArray(
-  dirname: string = ".",
-  options: WalkOptions = {}
-): Promise<string[]> {
-  const arr: string[] = [];
-  for await (const f of walk(dirname, { ...options })) {
-    arr.push(f.path.replace(/\\/g, "/"));
-  }
-  arr.sort();
-  const arrSync = Array.from(
-    walkSync(dirname, options),
-    (f: FileInfo): string => f.path.replace(/\\/g, "/")
-  ).sort();
-  assertEquals(arr, arrSync);
-  return arr;
-}
+import { touch, walkArray } from "./walk_test.ts";
 
 test({
   name: "glob: glob to regex",
@@ -77,7 +56,7 @@ testWalk(
   async function globInWalk(): Promise<void> {
     const arr = await walkArray(".", { match: [glob("*.ts")] });
     assertEquals(arr.length, 1);
-    assertEquals(arr[0], "./a/x.ts");
+    assertEquals(arr[0], "a/x.ts");
   }
 );
 
@@ -92,8 +71,8 @@ testWalk(
   async function globInWalkWildcardFiles(): Promise<void> {
     const arr = await walkArray(".", { match: [glob("*.ts")] });
     assertEquals(arr.length, 2);
-    assertEquals(arr[0], "./a/x.ts");
-    assertEquals(arr[1], "./b/z.ts");
+    assertEquals(arr[0], "a/x.ts");
+    assertEquals(arr[1], "b/z.ts");
   }
 );
 
@@ -113,7 +92,7 @@ testWalk(
       ]
     });
     assertEquals(arr.length, 1);
-    assertEquals(arr[0], "./a/yo/x.ts");
+    assertEquals(arr[0], "a/yo/x.ts");
   }
 );
 
@@ -137,8 +116,8 @@ testWalk(
       ]
     });
     assertEquals(arr.length, 2);
-    assertEquals(arr[0], "./a/deno/x.ts");
-    assertEquals(arr[1], "./a/raptor/x.ts");
+    assertEquals(arr[0], "a/deno/x.ts");
+    assertEquals(arr[1], "a/raptor/x.ts");
   }
 );
 
@@ -154,7 +133,9 @@ testWalk(
     });
     console.log(arr);
     assertEquals(arr.length, 2);
-    assertEquals(arr[0], "./x.js");
-    assertEquals(arr[1], "./x.ts");
+    assertEquals(arr[0], "x.js");
+    assertEquals(arr[1], "x.ts");
   }
 );
+
+runIfMain(import.meta);
diff --git a/fs/walk.ts b/fs/walk.ts
@@ -1,5 +1,10 @@
-const { readDir, readDirSync, readlink, readlinkSync, stat, statSync } = Deno;
+// 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 } = 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,114 @@ 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;
+export interface WalkInfo {
+  filename: string;
+  info: FileInfo;
 }
 
-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);
- *        assert(fileInfo.isFile());
+ *      for await (const { filename, info } of walk(".")) {
+ *        console.log(filename);
+ *        assert(info.isFile());
  *      };
  */
 export async function* walk(
-  dir: string = ".",
+  root: string,
   options: WalkOptions = {}
-): AsyncIterableIterator<FileInfo> {
+): AsyncIterableIterator<WalkInfo> {
   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 info of ls) {
+    if (info.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, info.name);
+
+    if (info.isFile()) {
+      if (include(filename, options)) {
+        yield { filename, info };
       }
     } 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<WalkInfo> {
   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 info of ls) {
+    if (info.isSymlink()) {
       if (options.followSymlinks) {
-        f = resolveSync(f);
+        unimplemented();
       } else {
         continue;
       }
     }
-    if (f.isFile()) {
-      if (include(f, options)) {
-        yield f;
+
+    const filename = join(root, info.name);
+
+    if (info.isFile()) {
+      if (include(filename, options)) {
+        yield { filename, info };
       }
     } else {
       if (!(options.maxDepth < 0)) {
-        yield* walkSync(f.path, options);
+        yield* walkSync(filename, options);
       }
     }
   }