std: Fix error handling in walk()

std/fs/walk.ts

@@ -14,31 +14,21 @@ export interface WalkOptions {
   exts?: string[];
   match?: RegExp[];
   skip?: RegExp[];
-  onError?: (err: Error) => void;
 }
 
-function patternTest(patterns: RegExp[], path: string): boolean {
-  // Forced to reset last index on regex while iterating for have
-  // consistent results.
-  // See: https://stackoverflow.com/a/1520853
-  return patterns.some((pattern): boolean => {
-    const r = pattern.test(path);
-    pattern.lastIndex = 0;
-    return r;
-  });
-}
-
-function include(filename: string, options: WalkOptions): boolean {
-  if (
-    options.exts &&
-    !options.exts.some((ext): boolean => filename.endsWith(ext))
-  ) {
+function include(
+  filename: string,
+  exts?: string[],
+  match?: RegExp[],
+  skip?: RegExp[]
+): boolean {
+  if (exts && !exts.some((ext): boolean => filename.endsWith(ext))) {
     return false;
   }
-  if (options.match && !patternTest(options.match, filename)) {
+  if (match && !match.some((pattern): boolean => !!filename.match(pattern))) {
     return false;
   }
-  if (options.skip && patternTest(options.skip, filename)) {
+  if (skip && skip.some((pattern): boolean => !!filename.match(pattern))) {
     return false;
   }
   return true;
@@ -62,7 +52,6 @@ export interface WalkInfo {
  * - exts?: string[];
  * - match?: RegExp[];
  * - skip?: RegExp[];
- * - onError?: (err: Error) => void;
  *
  *      for await (const { filename, info } of walk(".")) {
  *        console.log(filename);
@@ -71,38 +60,29 @@ export interface WalkInfo {
  */
 export async function* walk(
   root: string,
-  options: WalkOptions = {}
+  {
+    maxDepth = Infinity,
+    includeFiles = true,
+    includeDirs = true,
+    followSymlinks = false,
+    exts = null,
+    match = null,
+    skip = null
+  }: WalkOptions = {}
 ): AsyncIterableIterator<WalkInfo> {
-  const maxDepth = options.maxDepth != undefined ? options.maxDepth! : Infinity;
   if (maxDepth < 0) {
     return;
   }
-  if (options.includeDirs != false && include(root, options)) {
-    let rootInfo: FileInfo;
-    try {
-      rootInfo = await stat(root);
-    } catch (err) {
-      if (options.onError) {
-        options.onError(err);
-        return;
-      }
-    }
-    yield { filename: root, info: rootInfo! };
+  if (includeDirs && include(root, exts, match, skip)) {
+    yield { filename: root, info: await stat(root) };
   }
-  if (maxDepth < 1 || patternTest(options.skip || [], root)) {
+  if (maxDepth < 1 || !include(root, null, null, skip)) {
     return;
   }
-  let ls: FileInfo[] = [];
-  try {
-    ls = await readDir(root);
-  } catch (err) {
-    if (options.onError) {
-      options.onError(err);
-    }
-  }
+  const ls: FileInfo[] = await readDir(root);
   for (const info of ls) {
     if (info.isSymlink()) {
-      if (options.followSymlinks) {
+      if (followSymlinks) {
         // TODO(ry) Re-enable followSymlinks.
         unimplemented();
       } else {
@@ -113,50 +93,49 @@ export async function* walk(
     const filename = join(root, info.name!);
 
     if (info.isFile()) {
-      if (options.includeFiles != false && include(filename, options)) {
+      if (includeFiles && include(filename, exts, match, skip)) {
         yield { filename, info };
       }
     } else {
-      yield* walk(filename, { ...options, maxDepth: maxDepth - 1 });
+      yield* walk(filename, {
+        maxDepth: maxDepth - 1,
+        includeFiles,
+        includeDirs,
+        followSymlinks,
+        exts,
+        match,
+        skip
+      });
     }
   }
 }
 
 /** Same as walk() but uses synchronous ops */
 export function* walkSync(
   root: string,
-  options: WalkOptions = {}
+  {
+    maxDepth = Infinity,
+    includeFiles = true,
+    includeDirs = true,
+    followSymlinks = false,
+    exts = null,
+    match = null,
+    skip = null
+  }: WalkOptions = {}
 ): IterableIterator<WalkInfo> {
-  const maxDepth = options.maxDepth != undefined ? options.maxDepth! : Infinity;
   if (maxDepth < 0) {
     return;
   }
-  if (options.includeDirs != false && include(root, options)) {
-    let rootInfo: FileInfo;
-    try {
-      rootInfo = statSync(root);
-    } catch (err) {
-      if (options.onError) {
-        options.onError(err);
-        return;
-      }
-    }
-    yield { filename: root, info: rootInfo! };
+  if (includeDirs && include(root, exts, match, skip)) {
+    yield { filename: root, info: statSync(root) };
   }
-  if (maxDepth < 1 || patternTest(options.skip || [], root)) {
+  if (maxDepth < 1 || !include(root, null, null, skip)) {
     return;
   }
-  let ls: FileInfo[] = [];
-  try {
-    ls = readDirSync(root);
-  } catch (err) {
-    if (options.onError) {
-      options.onError(err);
-    }
-  }
+  const ls: FileInfo[] = readDirSync(root);
   for (const info of ls) {
     if (info.isSymlink()) {
-      if (options.followSymlinks) {
+      if (followSymlinks) {
         unimplemented();
       } else {
         continue;
@@ -166,11 +145,19 @@ export function* walkSync(
     const filename = join(root, info.name!);
 
     if (info.isFile()) {
-      if (options.includeFiles != false && include(filename, options)) {
+      if (includeFiles && include(filename, exts, match, skip)) {
         yield { filename, info };
       }
     } else {
-      yield* walkSync(filename, { ...options, maxDepth: maxDepth - 1 });
+      yield* walkSync(filename, {
+        maxDepth: maxDepth - 1,
+        includeFiles,
+        includeDirs,
+        followSymlinks,
+        exts,
+        match,
+        skip
+      });
     }
   }
 }

std/fs/walk_test.ts

@@ -1,7 +1,10 @@
-const { cwd, chdir, makeTempDir, mkdir, open, remove } = Deno;
+const { DenoError, ErrorKind, cwd, chdir, makeTempDir, mkdir, open } = Deno;
+const { remove } = Deno;
+type ErrorKind = Deno.ErrorKind;
+type DenoError = Deno.DenoError<ErrorKind>;
 import { walk, walkSync, WalkOptions, WalkInfo } from "./walk.ts";
 import { test, TestFunction, runIfMain } from "../testing/mod.ts";
-import { assertEquals } from "../testing/asserts.ts";
+import { assertEquals, assertThrowsAsync } from "../testing/asserts.ts";
 
 export async function testWalk(
   setup: (arg0: string) => void | Promise<void>,
@@ -232,14 +235,11 @@ testWalk(
 
 testWalk(
   async (_d: string): Promise<void> => {},
-  async function onError(): Promise<void> {
-    assertReady(1);
-    const ignored = await walkArray("missing");
-    assertEquals(ignored, ["missing"]);
-    let errors = 0;
-    await walkArray("missing", { onError: (_e): number => (errors += 1) });
-    // It's 2 since walkArray iterates over both sync and async.
-    assertEquals(errors, 2);
+  async function nonexistentRoot(): Promise<void> {
+    const error = (await assertThrowsAsync(async () => {
+      await walkArray("nonexistent");
+    }, DenoError)) as DenoError;
+    assertEquals(error.kind, ErrorKind.NotFound);
   }
 );
 

std/testing/asserts.ts

@@ -308,8 +308,9 @@ export function assertThrows(
   ErrorClass?: Constructor,
   msgIncludes = "",
   msg?: string
-): void {
+): Error {
   let doesThrow = false;
+  let error = null;
   try {
     fn();
   } catch (e) {
@@ -326,20 +327,23 @@ export function assertThrows(
       throw new AssertionError(msg);
     }
     doesThrow = true;
+    error = e;
   }
   if (!doesThrow) {
     msg = `Expected function to throw${msg ? `: ${msg}` : "."}`;
     throw new AssertionError(msg);
   }
+  return error;
 }
 
 export async function assertThrowsAsync(
   fn: () => Promise<void>,
   ErrorClass?: Constructor,
   msgIncludes = "",
   msg?: string
-): Promise<void> {
+): Promise<Error> {
   let doesThrow = false;
+  let error = null;
   try {
     await fn();
   } catch (e) {
@@ -356,11 +360,13 @@ export async function assertThrowsAsync(
       throw new AssertionError(msg);
     }
     doesThrow = true;
+    error = e;
   }
   if (!doesThrow) {
     msg = `Expected function to throw${msg ? `: ${msg}` : "."}`;
     throw new AssertionError(msg);
   }
+  return error;
 }
 
 /** Use this to stub out methods that will throw when invoked. */