From 07ca4f9629769a75bad11bb1059a7809d7f90db8 Mon Sep 17 00:00:00 2001
From: Ryan Dahl <ry@tinyclouds.org>
Date: Tue, 14 May 2019 17:14:08 -0400
Subject: [PATCH] walk() should not use deprecated FileInfo.path (#398)

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.
---
 fs/glob_test.ts       |  45 +++++------------
 fs/walk.ts            | 114 +++++++++++++++++++++---------------------
 fs/walk_test.ts       |  66 +++++++++++++-----------
 prettier/main.ts      |  19 ++++---
 prettier/main_test.ts |  12 ++---
 5 files changed, 120 insertions(+), 136 deletions(-)

diff --git a/fs/glob_test.ts b/fs/glob_test.ts
index 5772373dd49d..3139cb7f4694 100644
--- 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
index b21044e72d62..5cfd7e63dbef 100644
--- 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);
       }
     }
   }
diff --git a/fs/walk_test.ts b/fs/walk_test.ts
index e94a653c6be5..20e105a52641 100644
--- a/fs/walk_test.ts
+++ b/fs/walk_test.ts
@@ -1,10 +1,8 @@
-const { cwd, chdir, makeTempDir, mkdir, open, build, remove, symlink } = Deno;
+const { cwd, chdir, makeTempDir, mkdir, open, remove } = Deno;
 type FileInfo = Deno.FileInfo;
-import { walk, walkSync, WalkOptions } from "./walk.ts";
-import { test, TestFunction } from "../testing/mod.ts";
-import { assert, assertEquals } from "../testing/asserts.ts";
-
-const isWindows = build.os === "win";
+import { walk, walkSync, WalkOptions, WalkInfo } from "./walk.ts";
+import { test, TestFunction, runIfMain } from "../testing/mod.ts";
+import { assertEquals } from "../testing/asserts.ts";
 
 export async function testWalk(
   setup: (string) => void | Promise<void>,
@@ -26,28 +24,32 @@ export async function testWalk(
   test({ name, fn });
 }
 
-async function walkArray(
-  dirname: string = ".",
+function normalize({ filename }: WalkInfo): string {
+  return filename.replace(/\\/g, "/");
+}
+
+export async function walkArray(
+  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();
-  const arrSync = Array.from(
-    walkSync(dirname, options),
-    (f: FileInfo): string => f.path.replace(/\\/g, "/")
-  ).sort();
+  arr.sort(); // TODO(ry) Remove sort. The order should be deterministic.
+  const arrSync = Array.from(walkSync(root, options), normalize);
+  arrSync.sort(); // TODO(ry) Remove sort. The order should be deterministic.
   assertEquals(arr, arrSync);
   return arr;
 }
 
-async function touch(path: string): Promise<void> {
+export 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 +70,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 +80,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 +99,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 +114,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 +127,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 +141,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 +155,7 @@ testWalk(
     assertReady(2);
     const arr = await walkArray(".", { match: [/x/] });
     assertEquals(arr.length, 1);
-    assertEquals(arr[0], "./x");
+    assertEquals(arr[0], "x");
   }
 );
 
@@ -167,8 +169,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 +183,7 @@ testWalk(
     assertReady(2);
     const arr = await walkArray(".", { skip: [/x/] });
     assertEquals(arr.length, 1);
-    assertEquals(arr[0], "./y");
+    assertEquals(arr[0], "y");
   }
 );
 
@@ -195,7 +197,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 +230,7 @@ testWalk(
   }
 );
 
+/* TODO(ry) Re-enable followSymlinks
 testWalk(
   async (d: string): Promise<void> => {
     await mkdir(d + "/a");
@@ -258,3 +261,6 @@ testWalk(
     assert(arr.some((f): boolean => f.endsWith("/b/z")));
   }
 );
+*/
+
+runIfMain(import.meta);
diff --git a/prettier/main.ts b/prettier/main.ts
index a11501de13fc..0b5034c5fd3f 100755
--- a/prettier/main.ts
+++ b/prettier/main.ts
@@ -12,9 +12,8 @@
 // This script formats the given source files. If the files are omitted, it
 // formats the all files in the repository.
 const { args, exit, readFile, writeFile } = Deno;
-type FileInfo = Deno.FileInfo;
 import { glob } from "../fs/glob.ts";
-import { walk } from "../fs/walk.ts";
+import { walk, WalkInfo } from "../fs/walk.ts";
 import { parse } from "../flags/mod.ts";
 import { prettier, prettierPlugins } from "./prettier.ts";
 
@@ -174,15 +173,15 @@ function selectParser(path: string): ParserLabel | null {
  * If paths are empty, then checks all the files.
  */
 async function checkSourceFiles(
-  files: AsyncIterableIterator<FileInfo>,
+  files: AsyncIterableIterator<WalkInfo>,
   prettierOpts: PrettierOptions
 ): Promise<void> {
   const checks: Array<Promise<boolean>> = [];
 
-  for await (const file of files) {
-    const parser = selectParser(file.path);
+  for await (const { filename } of files) {
+    const parser = selectParser(filename);
     if (parser) {
-      checks.push(checkFile(file.path, parser, prettierOpts));
+      checks.push(checkFile(filename, parser, prettierOpts));
     }
   }
 
@@ -202,15 +201,15 @@ async function checkSourceFiles(
  * If paths are empty, then formats all the files.
  */
 async function formatSourceFiles(
-  files: AsyncIterableIterator<FileInfo>,
+  files: AsyncIterableIterator<WalkInfo>,
   prettierOpts: PrettierOptions
 ): Promise<void> {
   const formats: Array<Promise<void>> = [];
 
-  for await (const file of files) {
-    const parser = selectParser(file.path);
+  for await (const { filename } of files) {
+    const parser = selectParser(filename);
     if (parser) {
-      formats.push(formatFile(file.path, parser, prettierOpts));
+      formats.push(formatFile(filename, parser, prettierOpts));
     }
   }
 
diff --git a/prettier/main_test.ts b/prettier/main_test.ts
index 0c6653a3cf04..5a507afe750e 100644
--- a/prettier/main_test.ts
+++ b/prettier/main_test.ts
@@ -63,8 +63,8 @@ test(async function testPrettierCheckAndFormatFiles(): Promise<void> {
   assertEquals(code, 0);
   assertEquals(
     normalizeOutput(stdout),
-    `Formatting ./prettier/testdata/0.ts
-Formatting ./prettier/testdata/1.js`
+    `Formatting prettier/testdata/0.ts
+Formatting prettier/testdata/1.js`
   );
 
   var { code, stdout } = await run([...cmd, "--check", ...files]);
@@ -87,10 +87,10 @@ test(async function testPrettierCheckAndFormatDirs(): Promise<void> {
   assertEquals(code, 0);
   assertEquals(
     normalizeOutput(stdout),
-    `Formatting ./prettier/testdata/bar/0.ts
-Formatting ./prettier/testdata/bar/1.js
-Formatting ./prettier/testdata/foo/0.ts
-Formatting ./prettier/testdata/foo/1.js`
+    `Formatting prettier/testdata/bar/0.ts
+Formatting prettier/testdata/bar/1.js
+Formatting prettier/testdata/foo/0.ts
+Formatting prettier/testdata/foo/1.js`
   );
 
   var { code, stdout } = await run([...cmd, "--check", ...dirs]);