Code Quality: Remove manual interop definitions and add agent instructions for this (#18449)

Signed-off-by: terat <62196528+0x5bfa@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: 0x5bfa

AGENTS.md

@@ -0,0 +1,48 @@
+# Repository Instructions
+
+This repository contains the Files Windows desktop app, a WinUI-based file manager for Windows. The codebase includes the main app, reusable controls, storage layers, Win32/CsWin32 interop, packaging support, background/server components, and UI/interaction tests.
+
+## Codebase Overview
+
+```text
+/src
+├── Files.App                  // Main WinUI desktop app: startup, DI, views, view models, actions, services, dialogs, styles, assets, strings, and app helpers.
+├── Files.App.CsWin32          // CsWin32 source-generated Win32 interop. Add APIs to NativeMethods.txt here.
+├── Files.App.Controls         // Reusable WinUI controls shared by the app.
+├── Files.App.Storage          // App-facing storage abstractions and storage implementation pieces.
+├── Files.App.BackgroundTasks  // Background task project.
+├── Files.App.Server           // App service/server behavior.
+├── Files.App.Launcher         // Launch-related entry points.
+├── Files.App.OpenDialog       // File open dialog-specific app project/folder.
+├── Files.App.SaveDialog       // File save dialog-specific app project/folder.
+├── Files.App (Package)        // Packaging-related app project assets.
+├── Files.Core.Storage         // Lower-level storage primitives that should not depend on the main WinUI app.
+├── Files.Core.SourceGenerator // Roslyn source generators used by the solution.
+└── Files.Shared               // Shared models, helpers, and code used by multiple projects.
+```
+
+```text
+/tests
+├── Files.App.UITests      // UI test assets and views.
+├── Files.InteractionTests // Interaction tests used by CI automation.
+└── Files.App.UnitTests    // Placeholder/stale in this checkout; verify project files before assuming unit tests exist here.
+```
+
+## When Dealing With Interop Code
+
+When the user asks to convert marshaled interop code into unmarshaled interop, or asks to remove trim-unsafe manual P/Invoke definitions, see [docs/interop-unmarshaled-conversion.md](docs/interop-unmarshaled-conversion.md).
+
+Prefer adding APIs and related generated types to `src/Files.App.CsWin32/NativeMethods.txt`, then update the callees to use CsWin32-generated `Windows.Win32.PInvoke` APIs directly. Do not leave manual `DllImport` definitions in place or replace them with local `LibraryImport` declarations when CsWin32 can generate the API.
+
+## When Building the App
+
+Use `.github/workflows/ci.yml` as the source of truth for building.
+For normal local verification, build with MSBuild restore and explicit configuration/platform; packaging is not required.
+
+```powershell
+msbuild -restore src\Files.App\Files.App.csproj /p:Configuration=Debug /p:Platform=x64
+```
+
+## When Packaging the App
+
+Use `.github/workflows/ci.yml` as the source of truth for packaging. Adjust `Configuration`, `Platform`, and `AppxBundlePlatforms` as needed.

docs/interop-unmarshaled-conversion.md

@@ -0,0 +1,128 @@
+# Converting Marshaled Interop to CsWin32 Unmarshaled Interop
+
+This repository is moving trim-unsafe manual interop toward source-generated CsWin32 interop.
+When a user asks to convert marshaled interop code into unmarshaled interop, prefer updating `NativeMethods.txt` and the call sites instead of preserving manual `DllImport`/`LibraryImport` declarations or wrapping them with more local P/Invoke code.
+
+Removing Vanara interop usage is also part of this direction since it internally has trim-unsafe interop code.
+
+## Goal
+
+Remove interop code that relies on runtime marshalling, especially declarations using `DllImport`, `ComImport` and the Vanara package.
+
+The target shape is:
+
+- Add the native API, COM interface, enum, or struct name to `src/Files.App.CsWin32/NativeMethods.txt`.
+- Use `Windows.Win32.PInvoke` and generated CsWin32 types at the call site.
+- Delete the manual declarations and Vanara references.
+- Keep `Win32PInvoke` only for definitions that are not yet converted or cannot be generated by CsWin32.
+
+## Workflow
+
+1. Locate manual interop:
+
+   ```powershell
+   git grep -n "DllImport\|MarshalAs\|StringBuilder" -- src
+   git grep -n "Win32PInvoke\." -- src/Files.App
+   git grep -n "using Vanara\|Vanara\.PInvoke\|Kernel32\.\|Shell32\.\|User32\." -- src/Files.App
+   ```
+
+2. Add the API and related generated types to `src/Files.App.CsWin32/NativeMethods.txt`.
+
+   Include both the function and any dependent structs, enums, or COM interfaces that the call site needs. For example:
+
+   ```text
+   RmStartSession
+   RmRegisterResources
+   RmGetList
+   RmEndSession
+   RM_PROCESS_INFO
+   SHBrowseForFolder
+   BROWSEINFOW
+   SHGetPathFromIDList
+   SHCreateItemFromParsingName
+   SHCreateStreamOnFileEx
+   ```
+
+3. Build the CsWin32 project or the app project to refresh generated signatures:
+
+   ```powershell
+   dotnet build src/Files.App.CsWin32/Files.App.CsWin32.csproj -c Debug -p:Platform=x64
+   ```
+
+4. Update callers to use generated APIs directly.
+
+   Prefer generated safe overloads when they exist, such as `Span<char>`, `SafeHandle`, `ComPtr<T>`, generated enums, and generated structs. Use unsafe raw overloads only when the generated API naturally exposes pointers or when COM pointer identity is required.
+
+5. Remove the manual definition only after all callers have moved.
+
+   Use targeted checks:
+
+   ```powershell
+   git grep -n "Win32PInvoke\.RmStartSession\|Win32PInvoke\.SHBrowseForFolder" -- src/Files.App
+   git grep -n "RM_PROCESS_INFO\|BROWSEINFO" -- src/Files.App
+   ```
+
+6. Build and confirm there are no C# errors.
+
+   In this repo, WinUI XAML compiler failures may appear independently of interop changes. Separate `error CS*` failures from `MSB3073` XAML compiler failures when reporting verification.
+
+## Manual Definition Conversion Notes
+
+- `StringBuilder` output buffers should usually become `Span<char>` or fixed `char*` buffers.
+- `IntPtr` handles should become `SafeFileHandle`, `SafeHandle`, `HANDLE`, or generated handle structs where practical.
+- Some APIs such as `CoCreateInstance` and `SHCreateItemFromParsingName` often require unsafe pointer overloads:
+
+  ```csharp
+  void* raw;
+  Guid iid = SomeInterfaceIid;
+  HRESULT hr = PInvoke.CoCreateInstance(&clsid, null, CLSCTX.CLSCTX_LOCAL_SERVER, &iid, &raw);
+  IntPtr instance = (IntPtr)raw;
+  ```
+
+- Shell APIs that return PIDLs or allocated strings still require explicit lifetime management, but the API declaration should come from CsWin32:
+
+  ```csharp
+  var pidl = PInvoke.SHBrowseForFolder(ref browseInfo);
+  Marshal.FreeCoTaskMem((nint)pidl);
+  ```
+
+- Restart Manager APIs can use generated `RM_PROCESS_INFO` and `Span<char>` session keys instead of local struct definitions.
+- Do not keep a local `LibraryImport` copy when the API can be represented in `NativeMethods.txt`. The requested direction is to update callees to CsWin32-generated interop.
+
+## Vanara Conversion Notes
+
+Treat Vanara removal the same way as manual P/Invoke removal when Vanara is only wrapping a native API. Add the API to `NativeMethods.txt`, inspect the generated CsWin32 signature, then update the call site to generated types.
+
+Example conversion:
+
+```csharp
+// Before
+var lib = Kernel32.LoadLibrary(file);
+StringBuilder result = new(2048);
+_ = User32.LoadString(lib, number, result, result.Capacity);
+Kernel32.FreeLibrary(lib);
+return result.ToString();
+
+// After
+using var lib = PInvoke.LoadLibrary(file);
+Span<char> result = stackalloc char[2048];
+int length = PInvoke.LoadString(lib, (uint)number, result, result.Length);
+return result[..length].ToString();
+```
+
+Useful heuristics:
+
+- Start with simple `Kernel32.*`, `User32.*`, or `Shell32.*` calls that map directly to one Win32 function.
+- Remove `using Vanara.PInvoke` only when no remaining types in the file depend on it.
+- Prefer generated `Span<T>` overloads over `StringBuilder` buffers when available.
+- Prefer generated handle types and `SafeHandle` overloads where CsWin32 provides them.
+- Watch for CsWin32 convenience overloads. Some APIs generate safer shapes than the underlying Win32 signature; for example, `LoadLibrary` returns a disposable `FreeLibrarySafeHandle`, so the call site can use `using var` instead of a separate `FreeLibrary` call.
+
+## Verification Checklist
+
+- `NativeMethods.txt` contains every newly used API/type.
+- No caller remains for the deleted manual method or struct.
+- The generated CsWin32 types are used at call sites.
+- `dotnet build src/Files.App.CsWin32/Files.App.CsWin32.csproj -c Debug -p:Platform=x64` succeeds.
+- `dotnet build src/Files.App/Files.App.csproj -c Debug -p:Platform=x64` has no new `error CS*` errors.
+- Any remaining build failure is called out explicitly, especially XAML compiler failures unrelated to interop.

src/Files.App.CsWin32/Extras.cs

@@ -22,11 +22,11 @@ namespace UI.WindowsAndMessaging
 
 	public static partial class PInvoke
 	{
-		[DllImport("User32", EntryPoint = "SetWindowLongW", ExactSpelling = true)]
-		static extern int _SetWindowLong(HWND hWnd, int nIndex, int dwNewLong);
+		[LibraryImport("User32", EntryPoint = "SetWindowLongW")]
+		private static partial int _SetWindowLong(nint hWnd, int nIndex, int dwNewLong);
 
-		[DllImport("User32", EntryPoint = "SetWindowLongPtrW", ExactSpelling = true)]
-		static extern nint _SetWindowLongPtr(HWND hWnd, int nIndex, nint dwNewLong);
+		[LibraryImport("User32", EntryPoint = "SetWindowLongPtrW")]
+		private static partial nint _SetWindowLongPtr(nint hWnd, int nIndex, nint dwNewLong);
 
 		// NOTE:
 		//  CsWin32 doesn't generate SetWindowLong on other than x86 and vice versa.

src/Files.App.CsWin32/NativeMethods.txt

@@ -50,7 +50,37 @@ WNetAddConnection3
 CREDENTIALW
 CredWrite
 WNetConnectionDialog1
+WNetGetConnection
 CONNECTDLGSTRUCTW
+RmRegisterResources
+RmStartSession
+RmEndSession
+RmGetList
+RM_UNIQUE_PROCESS
+RM_PROCESS_INFO
+CreateEvent
+SetEvent
+CancelIoEx
+WaitForSingleObjectEx
+ReadDirectoryChangesW
+CreateFileFromAppW
+GetFileAttributesExFromApp
+SetFileAttributesFromApp
+ReadFile
+WriteFile
+WriteFileEx
+GetFileTime
+SetFileTime
+GetFileInformationByHandleEx
+FILE_ID_BOTH_DIR_INFO
+FindFirstStreamW
+FindNextStreamW
+WIN32_FIND_STREAM_DATA
+RegisterApplicationRestart
+BROWSEINFOW
+SHBrowseForFolder
+SHGetPathFromIDList
+SHCreateStreamOnFileEx
 DwmSetWindowAttribute
 WIN32_ERROR
 CoCreateInstance
@@ -116,9 +146,13 @@ WindowsDeleteString
 IPreviewHandler
 AssocQueryString
 GetModuleHandle
+LoadLibrary
+FreeLibrary
+LoadString
 SHEmptyRecycleBin
 SHFileOperation
 SHGetFolderPath
+SHGetKnownFolderPath
 SHGFP_TYPE
 SHGetKnownFolderItem
 SHQUERYRBINFO

src/Files.App/App.xaml.cs

@@ -7,6 +7,7 @@
 using Microsoft.UI.Xaml;
 using Microsoft.UI.Xaml.Controls;
 using Microsoft.Windows.AppLifecycle;
+using Windows.Win32;
 using Windows.ApplicationModel;
 using Windows.ApplicationModel.DataTransfer;
 using Windows.Storage;
@@ -227,9 +228,8 @@ private async void Window_Closed(object sender, WindowEventArgs args)
 				var results = items.Select(x => x.ItemPath).ToList();
 				System.IO.File.WriteAllLines(OutputPath, results);
 
-				IntPtr eventHandle = Win32PInvoke.CreateEvent(IntPtr.Zero, false, false, "FILEDIALOG");
-				Win32PInvoke.SetEvent(eventHandle);
-				Win32PInvoke.CloseHandle(eventHandle);
+				using var eventHandle = PInvoke.CreateEvent(null, false, false, "FILEDIALOG");
+				PInvoke.SetEvent(eventHandle);
 			}
 
 			// Continue running the app on the background

src/Files.App/Helpers/Win32/Win32Helper.Process.cs

@@ -1,6 +1,10 @@
 // Copyright (c) 2024 Files Community
 // Licensed under the MIT License. See the LICENSE.
 
+using Windows.Win32;
+using Windows.Win32.Foundation;
+using Windows.Win32.System.RestartManager;
+
 namespace Files.App.Helpers
 {
 	/// <summary>
@@ -68,36 +72,37 @@ public static async Task<bool> InvokeWin32ComponentsAsync(IEnumerable<string> ap
 		/// </remarks>
 		public static List<Process> WhoIsLocking(string[] resources)
 		{
-			string key = Guid.NewGuid().ToString();
+			Span<char> key = stackalloc char[64];
+			Guid.NewGuid().TryFormat(key, out var charsWritten);
+			key = key[..charsWritten];
 			List<Process> processes = [];
 
-			int res = Win32PInvoke.RmStartSession(out uint handle, 0, key);
-			if (res != 0) throw new Exception("Could not begin restart session.  Unable to determine file locker.");
+			WIN32_ERROR res = PInvoke.RmStartSession(out uint handle, key);
+			if (res != WIN32_ERROR.NO_ERROR) throw new Exception("Could not begin restart session.  Unable to determine file locker.");
 
 			try
 			{
-				const int ERROR_MORE_DATA = 234;
 				uint pnProcInfo = 0;
-				uint lpdwRebootReasons = Win32PInvoke.RmRebootReasonNone;
+				uint lpdwRebootReasons;
 
-				res = Win32PInvoke.RmRegisterResources(handle, (uint)resources.Length, resources, 0, null, 0, null);
+				res = PInvoke.RmRegisterResources(handle, resources, [], []);
 
-				if (res != 0) throw new Exception("Could not register resource.");
+				if (res != WIN32_ERROR.NO_ERROR) throw new Exception("Could not register resource.");
 
 				// Note:
 				//  There's a race condition here -- the first call to RmGetList() returns the total number of process.
 				//  However, when we call RmGetList() again to get the actual processes this number may have increased.
-				res = Win32PInvoke.RmGetList(handle, out uint pnProcInfoNeeded, ref pnProcInfo, null, ref lpdwRebootReasons);
+				res = PInvoke.RmGetList(handle, out uint pnProcInfoNeeded, ref pnProcInfo, [], out lpdwRebootReasons);
 
-				if (res == ERROR_MORE_DATA)
+				if (res == WIN32_ERROR.ERROR_MORE_DATA)
 				{
 					// Create an array to store the process results
-					var processInfo = new Win32PInvoke.RM_PROCESS_INFO[pnProcInfoNeeded];
+					var processInfo = new RM_PROCESS_INFO[pnProcInfoNeeded];
 					pnProcInfo = pnProcInfoNeeded;
 
 					// Get the list
-					res = Win32PInvoke.RmGetList(handle, out pnProcInfoNeeded, ref pnProcInfo, processInfo, ref lpdwRebootReasons);
-					if (res == 0)
+					res = PInvoke.RmGetList(handle, out pnProcInfoNeeded, ref pnProcInfo, processInfo, out lpdwRebootReasons);
+					if (res == WIN32_ERROR.NO_ERROR)
 					{
 						processes = new List<Process>((int)pnProcInfo);
 
@@ -107,19 +112,19 @@ public static List<Process> WhoIsLocking(string[] resources)
 						{
 							try
 							{
-								processes.Add(Process.GetProcessById(processInfo[i].Process.dwProcessId));
+								processes.Add(Process.GetProcessById((int)processInfo[i].Process.dwProcessId));
 							}
 							// catch the error -- in case the process is no longer running
 							catch (ArgumentException) { }
 						}
 					}
 					else throw new Exception("Could not list processes locking resource.");
 				}
-				else if (res != 0) throw new Exception("Could not list processes locking resource. Failed to get size of result.");
+				else if (res != WIN32_ERROR.NO_ERROR) throw new Exception("Could not list processes locking resource. Failed to get size of result.");
 			}
 			finally
 			{
-				_ = Win32PInvoke.RmEndSession(handle);
+				_ = PInvoke.RmEndSession(handle);
 			}
 
 			return processes;

src/Files.App/Helpers/Win32/Win32Helper.Shell.cs

@@ -5,6 +5,9 @@
 using System.Runtime.InteropServices;
 using Vanara.PInvoke;
 using Vanara.Windows.Shell;
+using Windows.Win32;
+using Windows.Win32.Foundation;
+using Windows.Win32.UI.Shell;
 
 namespace Files.App.Helpers
 {
@@ -78,12 +81,12 @@ public static partial class Win32Helper
 			}, App.Logger);
 		}
 
-		public static string GetFolderFromKnownFolderGUID(Guid guid)
+		public static unsafe string GetFolderFromKnownFolderGUID(Guid guid)
 		{
-			nint pszPath;
-			Win32PInvoke.SHGetKnownFolderPath(guid, 0, nint.Zero, out pszPath);
-			string path = Marshal.PtrToStringUni(pszPath);
-			Marshal.FreeCoTaskMem(pszPath);
+			PWSTR pszPath;
+			PInvoke.SHGetKnownFolderPath(ref guid, (KNOWN_FOLDER_FLAG)0, null, out pszPath);
+			string path = pszPath.ToString();
+			Marshal.FreeCoTaskMem((nint)pszPath.Value);
 
 			return path;
 		}