FoundationEssentials: tweak removeFile(_:with:) on Windows (#659)

Ensure that we call the callbacks at the appropriate time. Change the
indentation and encapsulating scope to avoid a double callback.

Author: compnerd

Sources/FoundationEssentials/FileManager/FileOperations.swift

@@ -317,6 +317,12 @@ enum _FileOperations {
         try path.withNTPathRepresentation {
             var faAttributes: WIN32_FILE_ATTRIBUTE_DATA = .init()
             guard GetFileAttributesExW($0, GetFileExInfoStandard, &faAttributes) else {
+                // NOTE: in the case that the 'stat' failed, we want to ensure
+                // that we query if the item should be removed. If the item
+                // should not be removed, we can continue, else we should check
+                // if we should proceed after the error.
+                guard filemanager?._shouldRemoveItemAtPath(path) ?? true else { return }
+
                 let error = CocoaError.removeFileError(GetLastError(), path)
                 guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: path) ?? false) else {
                     throw error
@@ -342,47 +348,55 @@ enum _FileOperations {
                     return
                 }
             }
-        }
 
-        var stack = [path]
-        while let directory = stack.popLast() {
-            guard filemanager?._shouldRemoveItemAtPath(directory) ?? true else { continue }
-            try directory.withNTPathRepresentation {
-                if RemoveDirectoryW($0) { return }
-                let dwError: DWORD = GetLastError()
-                guard dwError == ERROR_DIR_NOT_EMPTY else {
-                    let error = CocoaError.removeFileError(dwError, directory)
-                    guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: directory) ?? false) else {
-                        throw error
+            var stack = [(path, false)]
+            while let (directory, checked) = stack.popLast() {
+                try directory.withNTPathRepresentation {
+                    let ntpath = String(decodingCString: $0, as: UTF16.self)
+
+                    guard checked || filemanager?._shouldRemoveItemAtPath(ntpath) ?? true else { return }
+
+                    if RemoveDirectoryW($0) { return }
+                    var dwError: DWORD = GetLastError()
+                    guard dwError == ERROR_DIR_NOT_EMPTY else {
+                        let error = CocoaError.removeFileError(dwError, directory)
+                        guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: ntpath) ?? false) else {
+                            throw error
+                        }
+                        return
                     }
-                    return
-                }
-                stack.append(directory)
+                    stack.append((directory, true))
 
-                for entry in _Win32DirectoryContentsSequence(path: directory, appendSlashForDirectory: false, prefix: [directory]) {
-                    try entry.fileNameWithPrefix.withNTPathRepresentation {
-                        if entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY == FILE_ATTRIBUTE_DIRECTORY,
-                                entry.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT != FILE_ATTRIBUTE_REPARSE_POINT {
-                            stack.append(entry.fileNameWithPrefix)
-                        } else {
-                            if entry.dwFileAttributes & FILE_ATTRIBUTE_READONLY == FILE_ATTRIBUTE_READONLY {
-                                guard SetFileAttributesW($0, entry.dwFileAttributes & ~FILE_ATTRIBUTE_READONLY) else {
-                                    throw CocoaError.removeFileError(GetLastError(), path)
+                    for entry in _Win32DirectoryContentsSequence(path: directory, appendSlashForDirectory: false, prefix: [directory]) {
+                        try entry.fileNameWithPrefix.withNTPathRepresentation {
+                            let ntpath = String(decodingCString: $0, as: UTF16.self)
+
+                            if entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY == FILE_ATTRIBUTE_DIRECTORY,
+                                    entry.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT != FILE_ATTRIBUTE_REPARSE_POINT {
+                                if filemanager?._shouldRemoveItemAtPath(ntpath) ?? true {
+                                    stack.append((ntpath, true))
                                 }
-                            }
-                            guard filemanager?._shouldRemoveItemAtPath(entry.fileNameWithPrefix) ?? true else { return }
-                            if entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY == FILE_ATTRIBUTE_DIRECTORY {
-                                if !RemoveDirectoryW($0) {
-                                    let error = CocoaError.removeFileError(GetLastError(), entry.fileName)
-                                    guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: entry.fileNameWithPrefix) ?? false) else {
-                                        throw error
+                            } else {
+                                if entry.dwFileAttributes & FILE_ATTRIBUTE_READONLY == FILE_ATTRIBUTE_READONLY {
+                                    guard SetFileAttributesW($0, entry.dwFileAttributes & ~FILE_ATTRIBUTE_READONLY) else {
+                                        throw CocoaError.removeFileError(GetLastError(), ntpath)
                                     }
                                 }
-                            } else {
-                                if !DeleteFileW($0) {
-                                    let error = CocoaError.removeFileError(GetLastError(), entry.fileName)
-                                    guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: entry.fileNameWithPrefix) ?? false) else {
-                                        throw error
+                                if entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY == FILE_ATTRIBUTE_DIRECTORY {
+                                    guard filemanager?._shouldRemoveItemAtPath(ntpath) ?? true else { return }
+                                    if !RemoveDirectoryW($0) {
+                                        let error = CocoaError.removeFileError(GetLastError(), entry.fileName)
+                                        guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: entry.fileNameWithPrefix) ?? false) else {
+                                            throw error
+                                        }
+                                    }
+                                } else {
+                                    guard filemanager?._shouldRemoveItemAtPath(ntpath) ?? true else { return }
+                                    if !DeleteFileW($0) {
+                                        let error = CocoaError.removeFileError(GetLastError(), entry.fileName)
+                                        guard (filemanager?._shouldProceedAfter(error: error, removingItemAtPath: entry.fileNameWithPrefix) ?? false) else {
+                                            throw error
+                                        }
                                     }
                                 }
                             }

Tests/FoundationEssentialsTests/FileManager/FileManagerTests.swift

@@ -426,8 +426,8 @@ final class FileManagerTests : XCTestCase {
             XCTAssertEqual(try $0.subpathsOfDirectory(atPath: ".").sorted(), ["dir", "dir/foo", "other"])
             XCTAssertEqual($0.delegateCaptures.shouldRemove, [.init("dir/bar")])
             XCTAssertEqual($0.delegateCaptures.shouldProceedAfterRemoveError, [])
-            
-            let rootDir = $0.currentDirectoryPath
+
+            let rootDir = URL(fileURLWithPath: $0.currentDirectoryPath).path
             try $0.removeItem(atPath: "dir")
             XCTAssertEqual(try $0.subpathsOfDirectory(atPath: ".").sorted(), ["other"])
             XCTAssertEqual($0.delegateCaptures.shouldRemove, [.init("dir/bar"), .init("\(rootDir)/dir"), .init("\(rootDir)/dir/foo")])
@@ -443,6 +443,22 @@ final class FileManagerTests : XCTestCase {
             XCTAssertEqual($0.delegateCaptures.shouldProceedAfterRemoveError, [.init("does_not_exist", code: .fileNoSuchFile)])
         }
 
+        try FileManagerPlayground {
+            Directory("dir") {
+                Directory("dir2") {
+                    "file"
+                }
+            }
+        }.test(captureDelegateCalls: true) {
+            let rootDir = URL(fileURLWithPath: $0.currentDirectoryPath).path
+
+            XCTAssertTrue($0.delegateCaptures.isEmpty)
+            try $0.removeItem(atPath: "dir")
+            XCTAssertEqual(try $0.subpathsOfDirectory(atPath: ".").sorted(), [])
+            XCTAssertEqual($0.delegateCaptures.shouldRemove, [.init("\(rootDir)/dir"), .init("\(rootDir)/dir/dir2"), .init("\(rootDir)/dir/dir2/file")])
+            XCTAssertEqual($0.delegateCaptures.shouldProceedAfterRemoveError, [])
+        }
+
         #if canImport(Darwin)
         // not supported on linux as the test depends on FileManager.removeItem calling removefile(3)
         // not supported on older versions of Darwin where removefile would return ENOENT instead of ENAMETOOLONG