[PATCH v3 0/2] MR8491: shell32: Implement IFileOperation_NewItem for directory creation.

[Juan P. Montiel (＠jpmontielr)]

Tests and implementation for IFileOpearation_NewItem, which a Windows file explorer that I really like needs.

--
  v3: shell32: Implement IFileOperation_NewItem for directory creation.
      shell32/tests: Add tests for IFileOperation_NewItem.

https://gitlab.winehq.org/wine/wine/-/merge_requests/8491

[PATCH v3 1/2] shell32/tests: Add tests for IFileOperation_NewItem.

[Juan P. Montiel]

From: "Juan P. Montiel"

---
 dlls/shell32/tests/shlfileop.c | 163 +++++++++++++++++++++++++++++++++
 1 file changed, 163 insertions(+)

diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c
index 6ed64b442ff..35b3d179517 100644
--- a/dlls/shell32/tests/shlfileop.c
+++ b/dlls/shell32/tests/shlfileop.c
@@ -96,6 +96,17 @@ static BOOL file_existsW(LPCWSTR name)
   return GetFileAttributesW(name) != INVALID_FILE_ATTRIBUTES;
 }

+static BOOL dir_existsW(const WCHAR *name)
+{
+    DWORD attr;
+    BOOL dir;
+
+    attr = GetFileAttributesW(name);
+    dir = ((attr & FILE_ATTRIBUTE_DIRECTORY) == FILE_ATTRIBUTE_DIRECTORY);
+
+    return ((attr != INVALID_FILE_ATTRIBUTES) && dir);
+}
+
 static BOOL file_has_content(const CHAR *name, const CHAR *content)
 {
     CHAR buf[MAX_PATH];
@@ -3060,6 +3071,158 @@ static void test_file_operation(void)
     ok(!refcount, "got %ld.\n", refcount);
     refcount = IFileOperationProgressSink_Release(progress2);
     ok(!refcount, "got %ld.\n", refcount);
+
+    /* IFileOperation_NewItem tests */
+
+    hr = CoCreateInstance(&CLSID_FileOperation, NULL, CLSCTX_INPROC_SERVER, &IID_IFileOperation, (void **)&operation);
+    ok(hr == S_OK, "got %#lx.\n", hr);
+    hr = SHCreateItemFromParsingName(dirpath, NULL, &IID_IShellItem, (void**)&folder);
+    ok(hr == S_OK, "got %#lx.\n", hr);
+
+    /* NewItem then PerformOperations, NewItem then PerformOperations */
+    hr = IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
+    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    hr = IFileOperation_PerformOperations(operation);
+    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    PathCombineW(path, dirpath, L"test_dir");
+    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+
+    hr = IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
+    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    hr = IFileOperation_PerformOperations(operation);
+    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    PathCombineW(path, dirpath, L"test_dir (2)");
+    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+
+    /* NewItem, NewItem, then PerformOperations */
+    hr = IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
+    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    hr = IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
+    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    hr = IFileOperation_PerformOperations(operation);
+    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    PathCombineW(path, dirpath, L"test_dir (3)");
+    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+    PathCombineW(path, dirpath, L"test_dir (4)");
+    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+
+    /* In-between dir suffix */
+    IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
+    IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir (7)", NULL, NULL);
+    IFileOperation_PerformOperations(operation);
+    IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
+    IFileOperation_PerformOperations(operation);
+    PathCombineW(path, dirpath, L"test_dir (5)");
+    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+    PathCombineW(path, dirpath, L"test_dir (6)");
+    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+    PathCombineW(path, dirpath, L"test_dir (7)");
+    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+
+    /* Intermediate folder deletion but creation through its IShellItem * */
+    PathCombineW(path, dirpath, L"test_dir");
+    hr = SHCreateItemFromParsingName(path, NULL, &IID_IShellItem, (void**)&item);
+    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    RemoveDirectoryW(path);
+
+    hr = IFileOperation_NewItem(operation, item, FILE_ATTRIBUTE_DIRECTORY, L"nested_test_dir", NULL, NULL);
+    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    hr = IFileOperation_PerformOperations(operation);
+    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+    PathCombineW(path, dirpath, L"test_dir\nested_test_dir");
+    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+
+    /* Native NewItem passes with a NULL IShellIem pointer, but then PerformOperations crashes */
+    if (0)
+    {
+        hr = IFileOperation_NewItem(operation, NULL, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
+        todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+        IFileOperation_PerformOperations(operation);
+    }
+
+    /* Valid but bogus (not a dir) IShellItem * */
+    PathCombineW(tmpfile, dirpath, L"testfile");
+    createTestFileW(tmpfile);
+    hr = SHCreateItemFromParsingName(tmpfile, NULL, &IID_IShellItem, (void**)&item2);
+    ok(hr == S_OK, "got %#lx.\n", hr);
+    hr = IFileOperation_NewItem(operation, item2, FILE_ATTRIBUTE_DIRECTORY, L"nested_test_dir", NULL, NULL);
+    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    hr = IFileOperation_PerformOperations(operation);
+    todo_wine ok(hr == E_FAIL, "got %#lx.\n", hr);
+
+    /* Bad IShellItem pointer Crashes native NewItem */
+    if (0)
+    {
+        folder = (IShellItem *)&cookie;
+            IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
+    }
+
+    /* Empty name for the new item */
+    hr = IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"", NULL, NULL);
+    todo_wine ok(hr == S_OK /* Windows 7 */ || hr == E_INVALIDARG, "got %#lx.\n", hr);
+    if (hr == E_INVALIDARG)
+    {
+        /* Crashes w7 */
+        hr = IFileOperation_PerformOperations(operation);
+        ok(hr == E_UNEXPECTED, "got %#lx.\n", hr);
+    }
+    else if (hr == S_OK)
+    {
+        /* Because in this case we skip the Release() called by PerformOperations */
+        IShellItem_Release(item);
+    }
+
+    /* NULL name crashes native */
+    if (0)
+    {
+        IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, NULL, NULL, NULL);
+    }
+
+    /* The breaking point for the name length is >= 256 wide chars  */
+    hr = IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY,
+        L"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
+        L"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
+        L"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
+        L"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
+        NULL, NULL);
+    todo_wine ok(hr == S_OK /* Windows 8.1, 10 v1507 */ ||
+        hr == HRESULT_FROM_WIN32(ERROR_INSUFFICIENT_BUFFER) /* 7, newer 10, 11 */, "got %#lx.\n", hr);
+    if (hr == HRESULT_FROM_WIN32(ERROR_INSUFFICIENT_BUFFER))
+    {
+        hr = IFileOperation_PerformOperations(operation);
+        ok(hr == E_UNEXPECTED, "got %#lx.\n", hr);
+    }
+
+    PathCombineW(path, dirpath, L"test_dir");
+    remove_directory(path);
+    PathCombineW(path, dirpath, L"test_dir (2)");
+    remove_directory(path);
+    PathCombineW(path, dirpath, L"test_dir (3)");
+    remove_directory(path);
+    PathCombineW(path, dirpath, L"test_dir (4)");
+    remove_directory(path);
+    PathCombineW(path, dirpath, L"test_dir (5)");
+    remove_directory(path);
+    PathCombineW(path, dirpath, L"test_dir (6)");
+    remove_directory(path);
+    PathCombineW(path, dirpath, L"test_dir (7)");
+    remove_directory(path);
+    ok(DeleteFileW(tmpfile), "expected testfile to exist\n");
+
+    IFileOperation_Release(operation);
+
+    refcount = IShellItem_Release(folder);
+    ok(!refcount, "got %ld.\n", refcount);
+
+    if (item)
+    {
+        refcount = IShellItem_Release(item);
+        todo_wine ok(!refcount, "got %ld.\n", refcount);
+    }
+
+    refcount = IShellItem_Release(item2);
+    ok(!refcount, "got %ld.\n", refcount);
 }

START_TEST(shlfileop)

[PATCH v3 2/2] shell32: Implement IFileOperation_NewItem for directory creation.

[Juan P. Montiel]

From: "Juan P. Montiel"

---
 dlls/shell32/shlfileop.c       | 83 ++++++++++++++++++++++++++++++++--
 dlls/shell32/tests/shlfileop.c | 48 ++++++++++----------
 2 files changed, 102 insertions(+), 29 deletions(-)

diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c
index f24c7b7df23..53e7d0d1927 100644
--- a/dlls/shell32/shlfileop.c
+++ b/dlls/shell32/shlfileop.c
@@ -1836,6 +1836,7 @@ enum copy_engine_opcode
 {
     COPY_ENGINE_MOVE,
     COPY_ENGINE_REMOVE_DIRECTORY_SILENT,
+    COPY_ENGINE_NEW_ITEM_DIRECTORY,
 };

#define TSF_UNKNOWN_MEGRE_FLAG 0x1000
@@ -2180,6 +2181,57 @@ static HRESULT perform_file_operations(struct file_operation *operation)
                     IShellItem_Release(p.new_item);
                 break;
             }
+
+            case COPY_ENGINE_NEW_ITEM_DIRECTORY:
+            {
+                WCHAR *folder_path;
+                WCHAR name_path[MAX_PATH];
+                size_t name_len;
+                DWORD attrb;
+
+                /* Native crashes with a NULL IShellItem *, so do we */
+                IShellItem_GetDisplayName(op->folder, SIGDN_FILESYSPATH, &folder_path);
+                attrb = GetFileAttributesW(folder_path);
+                if(attrb == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_FILE_NOT_FOUND)
+                {
+                    if (!CreateDirectoryW(folder_path, NULL))
+                        return HRESULT_FROM_WIN32(GetLastError());
+                }
+                if(IsAttribFile(attrb))
+                {
+                    return E_FAIL;
+                }
+
+                name_len = wcslen(op->name);
+                if (name_len == 0 || name_len + 1 >= 256)
+                    return E_UNEXPECTED;
+
+                if (!PathCombineW(name_path, folder_path, op->name))
+                    return HRESULT_FROM_WIN32(GetLastError());
+
+                attrb = GetFileAttributesW(name_path);
+                if(attrb == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_FILE_NOT_FOUND)
+                {
+                    if (!CreateDirectoryW(name_path, NULL))
+                        return HRESULT_FROM_WIN32(GetLastError());
+                }
+                else if (attrb == FILE_ATTRIBUTE_DIRECTORY)
+                {
+                    unsigned int n = 2;
+                    WCHAR name_path_n[MAX_PATH];
+                    do {
+                        swprintf(name_path_n, ARRAY_SIZE(name_path_n), L"%s (%u)", name_path, n);
+                        attrb = GetFileAttributesW(name_path_n);
+                        ++n;
+                    } while (attrb == FILE_ATTRIBUTE_DIRECTORY);
+
+                    if (!CreateDirectoryW(name_path_n, NULL))
+                        return HRESULT_FROM_WIN32(GetLastError());
+                }
+
+                CoTaskMemFree(folder_path);
+                break;
+            }
         }
         set_file_operation_progress(operation, list_count(&operation->ops), operation->progress_sofar + 1);
         TRACE("op %d, hr %#lx.\n", op->opcode, hr);
@@ -2402,13 +2454,34 @@ static HRESULT WINAPI file_operation_DeleteItems(IFileOperation *iface, IUnknown
     return E_NOTIMPL;
 }

-static HRESULT WINAPI file_operation_NewItem(IFileOperation *iface, IShellItem *folder, DWORD attributes,
-        LPCWSTR name, LPCWSTR template, IFileOperationProgressSink *sink)
+static HRESULT WINAPI file_operation_NewItem(IFileOperation *iface, IShellItem *folder,
+        DWORD attributes, LPCWSTR name, LPCWSTR template, IFileOperationProgressSink *sink)
 {
-    FIXME("(%p, %p, %lx, %s, %s, %p): stub.\n", iface, folder, attributes,
-          debugstr_w(name), debugstr_w(template), sink);
+    size_t name_len;

-    return E_NOTIMPL;
+    TRACE("(%p, %p, %lx, %s, %s, %p).\n", iface, folder, attributes,
+        debugstr_w(name), debugstr_w(template), sink);
+
+    /* Native crashes with a NULL name, so do we */
+    name_len = wcslen(name);
+
+    if (attributes == FILE_ATTRIBUTE_DIRECTORY)
+    {
+        add_operation(impl_from_IFileOperation(iface),
+            COPY_ENGINE_NEW_ITEM_DIRECTORY, NULL, folder, name, NULL, 0, NULL);
+    }
+    else
+    {
+        FIXME("Unsupported attributes %#lx.\n", attributes);
+        return E_NOTIMPL;
+    }
+
+    if (name_len == 0)
+        return E_INVALIDARG;
+    else if (name_len + 1 >= 256)
+        return HRESULT_FROM_WIN32(ERROR_INSUFFICIENT_BUFFER);
+
+    return S_OK;
 }

static HRESULT WINAPI file_operation_PerformOperations(IFileOperation *iface)
diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c
index 35b3d179517..67702f8ca93 100644
--- a/dlls/shell32/tests/shlfileop.c
+++ b/dlls/shell32/tests/shlfileop.c
@@ -3081,30 +3081,30 @@ static void test_file_operation(void)

/* NewItem then PerformOperations, NewItem then PerformOperations */
     hr = IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
-    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    ok(hr == S_OK, "got %#lx.\n", hr);
     hr = IFileOperation_PerformOperations(operation);
-    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    ok(hr == S_OK, "got %#lx.\n", hr);
     PathCombineW(path, dirpath, L"test_dir");
-    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+    ok(dir_existsW(path), "directory should exist.\n");

hr = IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
-    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    ok(hr == S_OK, "got %#lx.\n", hr);
     hr = IFileOperation_PerformOperations(operation);
-    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    ok(hr == S_OK, "got %#lx.\n", hr);
     PathCombineW(path, dirpath, L"test_dir (2)");
-    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+    ok(dir_existsW(path), "directory should exist.\n");

/* NewItem, NewItem, then PerformOperations */
     hr = IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
-    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    ok(hr == S_OK, "got %#lx.\n", hr);
     hr = IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
-    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    ok(hr == S_OK, "got %#lx.\n", hr);
     hr = IFileOperation_PerformOperations(operation);
-    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    ok(hr == S_OK, "got %#lx.\n", hr);
     PathCombineW(path, dirpath, L"test_dir (3)");
-    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+    ok(dir_existsW(path), "directory should exist.\n");
     PathCombineW(path, dirpath, L"test_dir (4)");
-    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+    ok(dir_existsW(path), "directory should exist.\n");

/* In-between dir suffix */
     IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
@@ -3113,25 +3113,25 @@ static void test_file_operation(void)
     IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"test_dir", NULL, NULL);
     IFileOperation_PerformOperations(operation);
     PathCombineW(path, dirpath, L"test_dir (5)");
-    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+    ok(dir_existsW(path), "directory should exist.\n");
     PathCombineW(path, dirpath, L"test_dir (6)");
-    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+    ok(dir_existsW(path), "directory should exist.\n");
     PathCombineW(path, dirpath, L"test_dir (7)");
-    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+    ok(dir_existsW(path), "directory should exist.\n");

/* Intermediate folder deletion but creation through its IShellItem * */
     PathCombineW(path, dirpath, L"test_dir");
     hr = SHCreateItemFromParsingName(path, NULL, &IID_IShellItem, (void**)&item);
-    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    ok(hr == S_OK, "got %#lx.\n", hr);
     RemoveDirectoryW(path);

hr = IFileOperation_NewItem(operation, item, FILE_ATTRIBUTE_DIRECTORY, L"nested_test_dir", NULL, NULL);
-    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    ok(hr == S_OK, "got %#lx.\n", hr);
     hr = IFileOperation_PerformOperations(operation);
-    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
-    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+    ok(hr == S_OK, "got %#lx.\n", hr);
+    ok(dir_existsW(path), "directory should exist.\n");
     PathCombineW(path, dirpath, L"test_dir\nested_test_dir");
-    todo_wine ok(dir_existsW(path), "directory should exist.\n");
+    ok(dir_existsW(path), "directory should exist.\n");

/* Native NewItem passes with a NULL IShellIem pointer, but then PerformOperations crashes */
     if (0)
@@ -3147,9 +3147,9 @@ static void test_file_operation(void)
     hr = SHCreateItemFromParsingName(tmpfile, NULL, &IID_IShellItem, (void**)&item2);
     ok(hr == S_OK, "got %#lx.\n", hr);
     hr = IFileOperation_NewItem(operation, item2, FILE_ATTRIBUTE_DIRECTORY, L"nested_test_dir", NULL, NULL);
-    todo_wine ok(hr == S_OK, "got %#lx.\n", hr);
+    ok(hr == S_OK, "got %#lx.\n", hr);
     hr = IFileOperation_PerformOperations(operation);
-    todo_wine ok(hr == E_FAIL, "got %#lx.\n", hr);
+    ok(hr == E_FAIL, "got %#lx.\n", hr);

/* Bad IShellItem pointer Crashes native NewItem */
     if (0)
@@ -3160,7 +3160,7 @@ static void test_file_operation(void)

/* Empty name for the new item */
     hr = IFileOperation_NewItem(operation, folder, FILE_ATTRIBUTE_DIRECTORY, L"", NULL, NULL);
-    todo_wine ok(hr == S_OK /* Windows 7 */ || hr == E_INVALIDARG, "got %#lx.\n", hr);
+    ok(hr == S_OK /* Windows 7 */ || hr == E_INVALIDARG, "got %#lx.\n", hr);
     if (hr == E_INVALIDARG)
     {
         /* Crashes w7 */
@@ -3186,7 +3186,7 @@ static void test_file_operation(void)
         L"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
         L"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
         NULL, NULL);
-    todo_wine ok(hr == S_OK /* Windows 8.1, 10 v1507 */ ||
+    ok(hr == S_OK /* Windows 8.1, 10 v1507 */ ||
         hr == HRESULT_FROM_WIN32(ERROR_INSUFFICIENT_BUFFER) /* 7, newer 10, 11 */, "got %#lx.\n", hr);
     if (hr == HRESULT_FROM_WIN32(ERROR_INSUFFICIENT_BUFFER))
     {
@@ -3218,7 +3218,7 @@ static void test_file_operation(void)
     if (item)
     {
         refcount = IShellItem_Release(item);
-        todo_wine ok(!refcount, "got %ld.\n", refcount);
+        ok(!refcount, "got %ld.\n", refcount);
     }

refcount = IShellItem_Release(item2);