fix: Only swallow NotFound errors in fs/expandGlob() (denoland/deno#3479)

fs/expand_glob.ts

@@ -9,7 +9,9 @@ import {
   normalize
 } from "../path/mod.ts";
 import { WalkInfo, walk, walkSync } from "./walk.ts";
-const { cwd, stat, statSync } = Deno;
+const { ErrorKind, cwd, stat, statSync } = Deno;
+type ErrorKind = Deno.ErrorKind;
+type DenoError = Deno.DenoError<ErrorKind>;
 type FileInfo = Deno.FileInfo;
 
 export interface ExpandGlobOptions extends GlobOptions {
@@ -41,13 +43,16 @@ function split(path: string): SplitPath {
   };
 }
 
+function throwUnlessNotFound(error: Error): void {
+  if ((error as DenoError).kind != ErrorKind.NotFound) {
+    throw error;
+  }
+}
+
 /**
  * Expand the glob string from the specified `root` directory and yield each
  * result as a `WalkInfo` object.
  */
-// TODO: Use a proper glob expansion algorithm.
-// This is a very incomplete solution. The whole directory tree from `root` is
-// walked and parent paths are not supported.
 export async function* expandGlob(
   glob: string,
   {
@@ -69,7 +74,7 @@ export async function* expandGlob(
   const excludePatterns = exclude
     .map(resolveFromRoot)
     .map((s: string): RegExp => globToRegExp(s, globOptions));
-  const shouldInclude = ({ filename }: WalkInfo): boolean =>
+  const shouldInclude = (filename: string): boolean =>
     !excludePatterns.some((p: RegExp): boolean => !!filename.match(p));
   const { segments, hasTrailingSep, winRoot } = split(resolveFromRoot(glob));
 
@@ -81,8 +86,8 @@ export async function* expandGlob(
   let fixedRootInfo: WalkInfo;
   try {
     fixedRootInfo = { filename: fixedRoot, info: await stat(fixedRoot) };
-  } catch {
-    return;
+  } catch (error) {
+    return throwUnlessNotFound(error);
   }
 
   async function* advanceMatch(
@@ -94,12 +99,13 @@ export async function* expandGlob(
     } else if (globSegment == "..") {
       const parentPath = joinGlobs([walkInfo.filename, ".."], globOptions);
       try {
-        return yield* [
-          { filename: parentPath, info: await stat(parentPath) }
-        ].filter(shouldInclude);
-      } catch {
-        return;
+        if (shouldInclude(parentPath)) {
+          return yield { filename: parentPath, info: await stat(parentPath) };
         }
+      } catch (error) {
+        throwUnlessNotFound(error);
+      }
+      return;
     } else if (globSegment == "**") {
       return yield* walk(walkInfo.filename, {
         includeFiles: false,
@@ -149,7 +155,6 @@ export async function* expandGlob(
 }
 
 /** Synchronous version of `expandGlob()`. */
-// TODO: As `expandGlob()`.
 export function* expandGlobSync(
   glob: string,
   {
@@ -171,7 +176,7 @@ export function* expandGlobSync(
   const excludePatterns = exclude
     .map(resolveFromRoot)
     .map((s: string): RegExp => globToRegExp(s, globOptions));
-  const shouldInclude = ({ filename }: WalkInfo): boolean =>
+  const shouldInclude = (filename: string): boolean =>
     !excludePatterns.some((p: RegExp): boolean => !!filename.match(p));
   const { segments, hasTrailingSep, winRoot } = split(resolveFromRoot(glob));
 
@@ -183,8 +188,8 @@ export function* expandGlobSync(
   let fixedRootInfo: WalkInfo;
   try {
     fixedRootInfo = { filename: fixedRoot, info: statSync(fixedRoot) };
-  } catch {
-    return;
+  } catch (error) {
+    return throwUnlessNotFound(error);
   }
 
   function* advanceMatch(
@@ -196,12 +201,13 @@ export function* expandGlobSync(
     } else if (globSegment == "..") {
       const parentPath = joinGlobs([walkInfo.filename, ".."], globOptions);
       try {
-        return yield* [
-          { filename: parentPath, info: statSync(parentPath) }
-        ].filter(shouldInclude);
-      } catch {
-        return;
+        if (shouldInclude(parentPath)) {
+          return yield { filename: parentPath, info: statSync(parentPath) };
         }
+      } catch (error) {
+        throwUnlessNotFound(error);
+      }
+      return;
     } else if (globSegment == "**") {
       return yield* walkSync(walkInfo.filename, {
         includeFiles: false,

fs/expand_glob_test.ts

@@ -1,6 +1,7 @@
-const { cwd } = Deno;
+const { cwd, execPath, run } = Deno;
+import { decode } from "../strings/mod.ts";
 import { test, runIfMain } from "../testing/mod.ts";
-import { assert, assertEquals } from "../testing/asserts.ts";
+import { assert, assertEquals, assertStrContains } from "../testing/asserts.ts";
 import {
   isWindows,
   join,
@@ -117,4 +118,20 @@ test(async function expandGlobIncludeDirs(): Promise<void> {
   assertEquals(await expandGlobArray("subdir", options), []);
 });
 
+test(async function expandGlobPermError(): Promise<void> {
+  const exampleUrl = new URL("testdata/expand_wildcard.js", import.meta.url);
+  const p = run({
+    args: [execPath(), exampleUrl.toString()],
+    stdin: "null",
+    stdout: "piped",
+    stderr: "piped"
+  });
+  assertEquals(await p.status(), { code: 1, success: false });
+  assertEquals(decode(await p.output()), "");
+  assertStrContains(
+    decode(await p.stderrOutput()),
+    "Uncaught PermissionDenied"
+  );
+});
+
 runIfMain(import.meta);

fs/testdata/expand_wildcard.js

@@ -0,0 +1,6 @@
+import { expandGlob } from "../expand_glob.ts";
+
+const glob = new URL("*", import.meta.url).pathname;
+for await (const { filename } of expandGlob(glob)) {
+  console.log(filename);
+}