Only call directory removal method on actual dirs

This prevents all symbolic link following, and also cuts down on the
system calls quite considerably, while still preventing the "unlinked
directory" issue on SunOS, with only the cost of a single lstat at the
start of the process.

Also added test of mid-stream AbortSignal triggering in sync rimrafs,
because this is possible if called from a filter method.

Fix: #259

src/bin.ts

@@ -91,10 +91,9 @@ const interactiveRimraf = async (
       return false
     }
     while (!allRemaining) {
-      const a = (await prompt(
-        rl,
-        `rm? ${relative(cwd, path)}\n[(Yes)/No/All/Quit] > `
-      )).trim()
+      const a = (
+        await prompt(rl, `rm? ${relative(cwd, path)}\n[(Yes)/No/All/Quit] > `)
+      ).trim()
       if (/^n/i.test(a)) {
         return false
       } else if (/^a/i.test(a)) {

src/fs.ts

@@ -10,6 +10,7 @@ export {
   rmdirSync,
   rmSync,
   statSync,
+  lstatSync,
   unlinkSync,
 } from 'fs'
 
@@ -66,6 +67,11 @@ const stat = (path: fs.PathLike): Promise<fs.Stats> =>
     fs.stat(path, (er, data) => (er ? rej(er) : res(data)))
   )
 
+const lstat = (path: fs.PathLike): Promise<fs.Stats> =>
+  new Promise((res, rej) =>
+    fs.lstat(path, (er, data) => (er ? rej(er) : res(data)))
+  )
+
 const unlink = (path: fs.PathLike): Promise<void> =>
   new Promise((res, rej) =>
     fs.unlink(path, (er, ...d: any[]) => (er ? rej(er) : res(...d)))
@@ -79,5 +85,6 @@ export const promises = {
   rm,
   rmdir,
   stat,
+  lstat,
   unlink,
 }

src/ignore-enoent.ts

@@ -1,4 +1,3 @@
-
 export const ignoreENOENT = async (p: Promise<any>) =>
   p.catch(er => {
     if (er.code !== 'ENOENT') {

src/readdir-or-error.ts

@@ -1,6 +1,6 @@
 // returns an array of entries if readdir() works,
 // or the error that readdir() raised if not.
-import {  promises, readdirSync } from './fs.js'
+import { promises, readdirSync } from './fs.js'
 const { readdir } = promises
 export const readdirOrError = (path: string) =>
   readdir(path).catch(er => er as NodeJS.ErrnoException)

src/rimraf-move-remove.ts

@@ -18,13 +18,15 @@ import { ignoreENOENT, ignoreENOENTSync } from './ignore-enoent.js'
 
 import {
   chmodSync,
+  lstatSync,
   promises as fsPromises,
   renameSync,
   rmdirSync,
   unlinkSync,
 } from './fs.js'
-const { rename, unlink, rmdir, chmod } = fsPromises
+const { lstat, rename, unlink, rmdir, chmod } = fsPromises
 
+import { Dirent, Stats } from 'fs'
 import { RimrafAsyncOptions, RimrafSyncOptions } from '.'
 import { readdirOrError, readdirOrErrorSync } from './readdir-or-error.js'
 
@@ -72,25 +74,51 @@ const unlinkFixEPERMSync = (path: string) => {
 export const rimrafMoveRemove = async (
   path: string,
   opt: RimrafAsyncOptions
+) => {
+  if (opt?.signal?.aborted) {
+    throw opt.signal.reason
+  }
+  try {
+    return await rimrafMoveRemoveDir(path, opt, await lstat(path))
+  } catch (er) {
+    if ((er as NodeJS.ErrnoException)?.code === 'ENOENT') return true
+    throw er
+  }
+}
+
+const rimrafMoveRemoveDir = async (
+  path: string,
+  opt: RimrafAsyncOptions,
+  ent: Dirent | Stats
 ): Promise<boolean> => {
   if (opt?.signal?.aborted) {
     throw opt.signal.reason
   }
   if (!opt.tmp) {
-    return rimrafMoveRemove(path, { ...opt, tmp: await defaultTmp(path) })
+    return rimrafMoveRemoveDir(
+      path,
+      { ...opt, tmp: await defaultTmp(path) },
+      ent
+    )
   }
   if (path === opt.tmp && parse(path).root !== path) {
     throw new Error('cannot delete temp directory used for deletion')
   }
 
-  const entries = await readdirOrError(path)
+  const entries = ent.isDirectory() ? await readdirOrError(path) : null
   if (!Array.isArray(entries)) {
-    if (entries.code === 'ENOENT') {
-      return true
-    }
-    if (entries.code !== 'ENOTDIR') {
-      throw entries
+    // this can only happen if lstat/readdir lied, or if the dir was
+    // swapped out with a file at just the right moment.
+    /* c8 ignore start */
+    if (entries) {
+      if (entries.code === 'ENOENT') {
+        return true
+      }
+      if (entries.code !== 'ENOTDIR') {
+        throw entries
+      }
     }
+    /* c8 ignore stop */
     if (opt.filter && !(await opt.filter(path))) {
       return false
     }
@@ -100,7 +128,7 @@ export const rimrafMoveRemove = async (
 
   const removedAll = (
     await Promise.all(
-      entries.map(entry => rimrafMoveRemove(resolve(path, entry.name), opt))
+      entries.map(ent => rimrafMoveRemoveDir(resolve(path, ent.name), opt, ent))
     )
   ).reduce((a, b) => a && b, true)
   if (!removedAll) {
@@ -130,30 +158,53 @@ const tmpUnlink = async (
   return await rm(tmpFile)
 }
 
-export const rimrafMoveRemoveSync = (
+export const rimrafMoveRemoveSync = (path: string, opt: RimrafSyncOptions) => {
+  if (opt?.signal?.aborted) {
+    throw opt.signal.reason
+  }
+  try {
+    return rimrafMoveRemoveDirSync(path, opt, lstatSync(path))
+  } catch (er) {
+    if ((er as NodeJS.ErrnoException)?.code === 'ENOENT') return true
+    throw er
+  }
+}
+
+const rimrafMoveRemoveDirSync = (
   path: string,
-  opt: RimrafSyncOptions
+  opt: RimrafSyncOptions,
+  ent: Dirent | Stats
 ): boolean => {
   if (opt?.signal?.aborted) {
     throw opt.signal.reason
   }
   if (!opt.tmp) {
-    return rimrafMoveRemoveSync(path, { ...opt, tmp: defaultTmpSync(path) })
+    return rimrafMoveRemoveDirSync(
+      path,
+      { ...opt, tmp: defaultTmpSync(path) },
+      ent
+    )
   }
   const tmp: string = opt.tmp
 
   if (path === opt.tmp && parse(path).root !== path) {
     throw new Error('cannot delete temp directory used for deletion')
   }
 
-  const entries = readdirOrErrorSync(path)
+  const entries = ent.isDirectory() ? readdirOrErrorSync(path) : null
   if (!Array.isArray(entries)) {
-    if (entries.code === 'ENOENT') {
-      return true
-    }
-    if (entries.code !== 'ENOTDIR') {
-      throw entries
+    // this can only happen if lstat/readdir lied, or if the dir was
+    // swapped out with a file at just the right moment.
+    /* c8 ignore start */
+    if (entries) {
+      if (entries.code === 'ENOENT') {
+        return true
+      }
+      if (entries.code !== 'ENOTDIR') {
+        throw entries
+      }
     }
+    /* c8 ignore stop */
     if (opt.filter && !opt.filter(path)) {
       return false
     }
@@ -162,9 +213,9 @@ export const rimrafMoveRemoveSync = (
   }
 
   let removedAll = true
-  for (const entry of entries) {
-    removedAll =
-      rimrafMoveRemoveSync(resolve(path, entry.name), opt) && removedAll
+  for (const ent of entries) {
+    const p = resolve(path, ent.name)
+    removedAll = rimrafMoveRemoveDirSync(p, opt, ent) && removedAll
   }
   if (!removedAll) {
     return false

src/rimraf-posix.ts

@@ -1,35 +1,66 @@
 // the simple recursive removal, where unlink and rmdir are atomic
 // Note that this approach does NOT work on Windows!
-// We rmdir before unlink even though that is arguably less efficient
-// (since the average folder contains >1 file, it means more system
-// calls), because sunos will let root unlink a directory, and some
+// We stat first and only unlink if the Dirent isn't a directory,
+// because sunos will let root unlink a directory, and some
 // SUPER weird breakage happens as a result.
 
-import { promises, rmdirSync, unlinkSync } from './fs.js'
-const { rmdir, unlink } = promises
+import { lstatSync, promises, rmdirSync, unlinkSync } from './fs.js'
+const { lstat, rmdir, unlink } = promises
 
 import { parse, resolve } from 'path'
 
 import { readdirOrError, readdirOrErrorSync } from './readdir-or-error.js'
 
+import { Dirent, Stats } from 'fs'
 import { RimrafAsyncOptions, RimrafSyncOptions } from '.'
 import { ignoreENOENT, ignoreENOENTSync } from './ignore-enoent.js'
 
-export const rimrafPosix = async (
+export const rimrafPosix = async (path: string, opt: RimrafAsyncOptions) => {
+  if (opt?.signal?.aborted) {
+    throw opt.signal.reason
+  }
+  try {
+    return await rimrafPosixDir(path, opt, await lstat(path))
+  } catch (er) {
+    if ((er as NodeJS.ErrnoException)?.code === 'ENOENT') return true
+    throw er
+  }
+}
+
+export const rimrafPosixSync = (path: string, opt: RimrafSyncOptions) => {
+  if (opt?.signal?.aborted) {
+    throw opt.signal.reason
+  }
+  try {
+    return rimrafPosixDirSync(path, opt, lstatSync(path))
+  } catch (er) {
+    if ((er as NodeJS.ErrnoException)?.code === 'ENOENT') return true
+    throw er
+  }
+}
+
+const rimrafPosixDir = async (
   path: string,
-  opt: RimrafAsyncOptions
+  opt: RimrafAsyncOptions,
+  ent: Dirent | Stats
 ): Promise<boolean> => {
   if (opt?.signal?.aborted) {
     throw opt.signal.reason
   }
-  const entries = await readdirOrError(path)
+  const entries = ent.isDirectory() ? await readdirOrError(path) : null
   if (!Array.isArray(entries)) {
-    if (entries.code === 'ENOENT') {
-      return true
-    }
-    if (entries.code !== 'ENOTDIR') {
-      throw entries
+    // this can only happen if lstat/readdir lied, or if the dir was
+    // swapped out with a file at just the right moment.
+    /* c8 ignore start */
+    if (entries) {
+      if (entries.code === 'ENOENT') {
+        return true
+      }
+      if (entries.code !== 'ENOTDIR') {
+        throw entries
+      }
     }
+    /* c8 ignore stop */
     if (opt.filter && !(await opt.filter(path))) {
       return false
     }
@@ -39,7 +70,7 @@ export const rimrafPosix = async (
 
   const removedAll = (
     await Promise.all(
-      entries.map(entry => rimrafPosix(resolve(path, entry.name), opt))
+      entries.map(ent => rimrafPosixDir(resolve(path, ent.name), opt, ent))
     )
   ).reduce((a, b) => a && b, true)
 
@@ -62,30 +93,38 @@ export const rimrafPosix = async (
   return true
 }
 
-export const rimrafPosixSync = (
+const rimrafPosixDirSync = (
   path: string,
-  opt: RimrafSyncOptions
+  opt: RimrafSyncOptions,
+  ent: Dirent | Stats
 ): boolean => {
   if (opt?.signal?.aborted) {
     throw opt.signal.reason
   }
-  const entries = readdirOrErrorSync(path)
+  const entries = ent.isDirectory() ? readdirOrErrorSync(path) : null
   if (!Array.isArray(entries)) {
-    if (entries.code === 'ENOENT') {
-      return true
-    }
-    if (entries.code !== 'ENOTDIR') {
-      throw entries
+    // this can only happen if lstat/readdir lied, or if the dir was
+    // swapped out with a file at just the right moment.
+    /* c8 ignore start */
+    if (entries) {
+      if (entries.code === 'ENOENT') {
+        return true
+      }
+      if (entries.code !== 'ENOTDIR') {
+        throw entries
+      }
     }
+    /* c8 ignore stop */
     if (opt.filter && !opt.filter(path)) {
       return false
     }
     ignoreENOENTSync(() => unlinkSync(path))
     return true
   }
   let removedAll: boolean = true
-  for (const entry of entries) {
-    removedAll = rimrafPosixSync(resolve(path, entry.name), opt) && removedAll
+  for (const ent of entries) {
+    const p = resolve(path, ent.name)
+    removedAll = rimrafPosixDirSync(p, opt, ent) && removedAll
   }
   if (opt.preserveRoot === false && path === parse(path).root) {
     return false